feat(html): implement atob and btoa - #272
Conversation
V8 provides no atob/btoa (they are Web APIs, not ECMAScript). Implemented natively on WindowOrWorkerGlobalScope with correct binary-string semantics and forgiving-base64 decoding.
WalkthroughThis PR adds 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
| "outside of the Latin1 range.") | ||
| } | ||
| bytes = append(bytes, byte(r)) | ||
| } |
There was a problem hiding this comment.
The implementation threw me off, but after checking the MDN documentation; it makes sense.
However, I think it would make sense to create a dedicated decoder for this, e.g., DecodeBinaryString, allowing this implementation to be reduced to
bytes, err := js.ConsumeArgument(cbCtx, "data", nil, codec.DecodeBinaryString)
if err != nil {
return nil, err
}
return cbCtx.NewString(base64.StdEncoding.EncodeToString(bytes)), nilIt would only be used this once, so far, so not crucial; just seem to be separate concerns better.
There was a problem hiding this comment.
Sure! I added codec.DecodeBinaryString, and btoa's argument decodes straight to []byte. As part of this it's all generated from the Web IDL now: native Btoa([]byte) string / Atob(string) ([]byte, error) live on windowOrWorkerGlobalScope in package html.
| eq("round-trip binary", `atob(btoa("\x00\x01\xfe\xff")) === "\x00\x01\xfe\xff"`, true) | ||
| // Implemented natively, so Function.prototype.toString reports native code. | ||
| eq("atob is native", `/\[native code\]/.test(atob.toString())`, true) | ||
| eq("btoa is native", `/\[native code\]/.test(btoa.toString())`, true) |
There was a problem hiding this comment.
Testing that toString() on the functions doesn't seem to make sense.
|
Hey. I actually ran into issues, wishing I had this par implemented :D However, there are some thing I'd like to move around ;) Separate implementation from JS bindingsMany of the types align with web IDL specifications, and the two functions are defined on This is also where e.g., In So by removing these two lines; it'd almost work, but there's probably some customization necessary to tell it to decode strings to The generator also takes care of exposing the functions in global scope. Worker support out of the box.I have an experimental branch with Worker support. By moving the implementation to I can do it later ...IF you feel up to it; you are welcome to give it a shot; if not, I'll gladly take it as is, and refactor accordingly later. The web-idl -> go codenerator isn't the ... cleanest part of the code base. |
| s = strings.Map(func(r rune) rune { | ||
| switch r { | ||
| case ' ', '\t', '\n', '\r', '\f': | ||
| return -1 |
There was a problem hiding this comment.
I assume that -1 filters out the value. I wonder. Would it make sense to use unicode.IsSpace()? https://pkg.go.dev/unicode#IsSpace
There was a problem hiding this comment.
Well the -1 makes strings.Map drop the rune. I kept the explicit set rather than unicode.IsSpace, though: WHATWG forgiving-base64 decode strips ASCII whitespace only (tab/LF/FF/CR/space), whereas unicode.IsSpace also matches non-ASCII whitespace (NBSP, the Unicode separators…), which the algorithm requires to be treated as invalid input rather than stripped. Added a comment + spec link. (Moved into forgivingBase64Decode in package html with the restructure.)
Move the native atob/btoa implementation onto windowOrWorkerGlobalScope in package html (alongside setTimeout and friends) and generate the JavaScript bindings from the Web IDL specification, rather than hand-wiring them in the scripting layer. To support the binary-string conversions the bindings need, add a []byte GoType and a matching DecodeBinaryString/EncodeBinaryString codec pair: btoa takes its argument as []byte and atob returns its result as []byte (ByteString). The forgiving-base64 decode helper keeps the explicit ASCII whitespace set required by the Infra standard. Also fix the interface generator so a GoType in an empty (local) package renders correctly as an argument type. Move the atob/btoa test into the shared script-engine suite so it runs against both the V8 and Sobek engines, and drop the Function.prototype.toString checks.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31412e72-1ee8-4a9a-af55-e373cb01c161
📒 Files selected for processing (13)
html/window_or_worker_global_scope.gointernal/code-gen/customrules/html_rules.gointernal/code-gen/gotypes/gotypes.gointernal/code-gen/interfaces/idl_interface.gointernal/code-gen/scripting/codecs.gointernal/code-gen/scripting/model/encoder.gointernal/code-gen/scripting/model/es_operation.gointernal/interfaces/html-interfaces/window_or_worker_global_scope_generated.goscripting/internal/codec/decoders.goscripting/internal/codec/encoders.goscripting/internal/html/window_or_worker_global_scope_generated.goscripting/internal/scripttests/base64_suite.goscripting/internal/scripttests/suites.go
| if decoded, err := base64.StdEncoding.DecodeString(s); err == nil { | ||
| return decoded, nil | ||
| } | ||
| // Fall back to unpadded decoding for inputs missing trailing '='. | ||
| return base64.RawStdEncoding.DecodeString(strings.TrimRight(s, "=")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject malformed padded input before the raw fallback.
If the padded decode fails, Line 50 strips all trailing = and retries as unpadded base64. That makes malformed inputs like Zg=== and == decode successfully instead of failing.
Suggested fix
- if decoded, err := base64.StdEncoding.DecodeString(s); err == nil {
+ if decoded, err := base64.StdEncoding.DecodeString(s); err == nil {
return decoded, nil
+ } else if strings.ContainsRune(s, '=') {
+ return nil, err
}
- // Fall back to unpadded decoding for inputs missing trailing '='.
- return base64.RawStdEncoding.DecodeString(strings.TrimRight(s, "="))
+ // Fall back only for genuinely unpadded input.
+ return base64.RawStdEncoding.DecodeString(s)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if decoded, err := base64.StdEncoding.DecodeString(s); err == nil { | |
| return decoded, nil | |
| } | |
| // Fall back to unpadded decoding for inputs missing trailing '='. | |
| return base64.RawStdEncoding.DecodeString(strings.TrimRight(s, "=")) | |
| if decoded, err := base64.StdEncoding.DecodeString(s); err == nil { | |
| return decoded, nil | |
| } else if strings.ContainsRune(s, '=') { | |
| return nil, err | |
| } | |
| // Fall back only for genuinely unpadded input. | |
| return base64.RawStdEncoding.DecodeString(s) |
| for _, r := range str { | ||
| if r > 0xFF { | ||
| return nil, fmt.Errorf( | ||
| "codec: binary string contains a character outside the Latin-1 range") | ||
| } | ||
| bytes = append(bytes, byte(r)) | ||
| } | ||
| return bytes, nil | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
G115 (gosec) is a false positive here, but may still break CI.
The r > 0xFF guard bounds r to [0, 0xFF] before the byte(r) conversion (runes are non-negative; invalid UTF-8 decodes to 0xFFFD, which is also caught), so the conversion is safe. gosec doesn't perform this range analysis. If gosec is enforced in the pipeline, add a //nolint:gosec // bounded by the r > 0xFF guard to avoid a failing build.
🧰 Tools
🪛 golangci-lint (2.12.2)
[high] 46-46: G115: integer overflow conversion rune -> byte
(gosec)
Source: Linters/SAST tools
2 participants
V8 does not provide
atob/btoa(they are Web APIs, not part of ECMAScript), so they were missing. This implements them natively onWindowOrWorkerGlobalScopewith correct binary-string semantics and WHATWG forgiving-base64 decoding. Implemented natively (rather than as a JS polyfill) soFunction.prototype.toStringreports[native code], matching browsers.Testing: added
TestBase64(round-trips, binary strings, native-code check).AI disclosure: This change was developed with the help of an AI coding assistant. I've reviewed and tested it myself; it follows the existing conventions and the full test suite (main module,
v8engine,sobekengine) passes locally.