Conversation
Contributor
Reviewer's Guide为 Goja VM 引入一个完整兼容 Web Crypto API 的 crypto.subtle.encrypt 调用流程时序图sequenceDiagram
participant JS as JS_Script
participant VM as Goja_Runtime
participant Crypto as CryptoModule
participant KeyHelpers as CryptoKeyHelpers
JS->>VM: call crypto.subtle.encrypt(algorithm, key, data)
VM->>Crypto: subtleEncrypt(rt, call)
Crypto->>Crypto: parseAlgorithmIdentifier(rt, algorithm)
Crypto->>KeyHelpers: extractCryptoKeyHandle(rt, key)
KeyHelpers-->>Crypto: *CryptoKeyHandle
Crypto->>Crypto: bufferSourceBytes(rt, data)
Crypto->>Crypto: encryptData(rt, algorithmName, algObj, handle, bytes)
Crypto-->>VM: Promise_resolved(ArrayBuffer(cipherText))
VM-->>JS: resolved Promise with cipherText
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideIntroduce a full Web Crypto API-compatible Sequence diagram for crypto.subtle.encrypt call flowsequenceDiagram
participant JS as JS_Script
participant VM as Goja_Runtime
participant Crypto as CryptoModule
participant KeyHelpers as CryptoKeyHelpers
JS->>VM: call crypto.subtle.encrypt(algorithm, key, data)
VM->>Crypto: subtleEncrypt(rt, call)
Crypto->>Crypto: parseAlgorithmIdentifier(rt, algorithm)
Crypto->>KeyHelpers: extractCryptoKeyHandle(rt, key)
KeyHelpers-->>Crypto: *CryptoKeyHandle
Crypto->>Crypto: bufferSourceBytes(rt, data)
Crypto->>Crypto: encryptData(rt, algorithmName, algObj, handle, bytes)
Crypto-->>VM: Promise_resolved(ArrayBuffer(cipherText))
VM-->>JS: resolved Promise with cipherText
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出一些总体反馈：
crypto.go的实现已经变得非常庞大（大约 3.7k 行），同时混合了算法解析、JWK 处理、RSA 数学运算、AES 封装等内容；建议将其拆分成多个更小的文件（例如：digest.go、aes.go、rsa.go、jwk.go、derive.go），以便未来的修改和代码审查更容易管理。- 像 MD5、SHA-1、DES/3DES 和
RSAES-PKCS1-v1_5这样的弱/传统算法目前通过 API 完全暴露；即使文档中已经标注为已弃用，最好还是通过显式的选择加入标志或运行时选项来加以限制，这样就不会在普通脚本中被意外使用。
面向 AI 代理的提示词
Please address the comments from this code review:
## Overall Comments
- The `crypto.go` implementation has grown very large (~3.7k lines) and mixes algorithm parsing, JWK handling, RSA math, AES wrappers, etc.; consider splitting it into smaller files (e.g., `digest.go`, `aes.go`, `rsa.go`, `jwk.go`, `derive.go`) to make future changes and reviews more manageable.
- Weak/legacy algorithms like MD5, SHA-1, DES/3DES, and `RSAES-PKCS1-v1_5` are fully exposed through the API; even though they are documented as deprecated, it might be safer to gate them behind an explicit opt-in flag or runtime option so they cannot be used accidentally in normal scripts.帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- The
crypto.goimplementation has grown very large (~3.7k lines) and mixes algorithm parsing, JWK handling, RSA math, AES wrappers, etc.; consider splitting it into smaller files (e.g.,digest.go,aes.go,rsa.go,jwk.go,derive.go) to make future changes and reviews more manageable. - Weak/legacy algorithms like MD5, SHA-1, DES/3DES, and
RSAES-PKCS1-v1_5are fully exposed through the API; even though they are documented as deprecated, it might be safer to gate them behind an explicit opt-in flag or runtime option so they cannot be used accidentally in normal scripts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `crypto.go` implementation has grown very large (~3.7k lines) and mixes algorithm parsing, JWK handling, RSA math, AES wrappers, etc.; consider splitting it into smaller files (e.g., `digest.go`, `aes.go`, `rsa.go`, `jwk.go`, `derive.go`) to make future changes and reviews more manageable.
- Weak/legacy algorithms like MD5, SHA-1, DES/3DES, and `RSAES-PKCS1-v1_5` are fully exposed through the API; even though they are documented as deprecated, it might be safer to gate them behind an explicit opt-in flag or runtime option so they cannot be used accidentally in normal scripts.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
|
既然做了 crypto 相关的 package，或许把文件拆得小一些，做成 package 内不同的文件或许会更好一些 |
Szzrain
reviewed
Feb 24, 2026
Szzrain
left a comment
Szzrain
left a comment
Contributor
There was a problem hiding this comment.
关于此pr我思索了许久，我认为这部分功能不太适合放在海豹之中，而更合适塞进 goja_nodejs 或仿照其实现方式另起一个 repo
Contributor
认同。 另外，木落老师有透露未来 2.0 版本不排除去除 goja 继而引入其他库与架构对插件系统进行重构的可能，虽然确认即将到来的 1.6.0 版本已经距离 1.5.1 版本很久，但私以为此修改的未来可迁移性有些太低，可能在那时会有隐性的技术债务。 |
Contributor
Author
|
为什么我觉得不影响未来可能的迁移呢。 |
Contributor
目前的 crypto helper 是基于 goja 做的 bridge 实现，但是未来我们如果希望引入例如 wazero 库作为 2.0 可能的插件系统底层，那么这一套就行不通了，届时，如果要重构，肯定要考虑 像这样子既非 goja 亦非海豹原生实现的插件功能是否要搬运出去作为一个 repo 或是直接 remove再或是重写为符合对应底层实现的文件。无论哪一条，迁移难度都不低。 |
Member
我了解了一下这个方向上的大致用途，感觉还是有用的，不过考虑到兼容性问题，需要确认api与标准api一致，未来可以无缝迁移。简单说就是看看js测试脚本在quickjs中能否运行 我认为可以这样：如果无法做到完全支持或者大部分支持，可以缩小一点接触面，也就是把常用的那几个算法支持掉 |
Contributor
我想将该实现挪动到sealdice/goja_ext中，请问您的意见？ |
Member
|
暂时无法完成验证，等1.6.0发布后考虑合并，核心验证点是和常规webapi的一致性 |
Contributor
|
已将对应代码拷贝到sealdice_ext内。本PR若考虑修改，则修改对应引入sealdice_ext代码；否则将考虑关闭。 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1603
Summary by Sourcery
在 Goja JS 运行时中新增一个兼容 Web Crypto API 的
crypto模块，并将其集成到 Dice JS 环境中。新功能：
crypto对象，提供getRandomValues、randomUUID，以及符合 Web Crypto API 的subtle接口。文档：
crypto模块撰写文档，说明暴露的 JS Web Crypto API 接口、所支持的算法、密钥格式以及使用示例。测试：
Original summary in English
Summary by Sourcery
Add a Web Crypto API-compatible
cryptomodule to the Goja JS runtime and integrate it into the Dice JS environment.New Features:
cryptoobject withgetRandomValues,randomUUID, and asubtleWeb Crypto API surface in the Goja environment.Documentation:
cryptomodule.Tests: