fix(ts-sdk): decode base64 data URLs with media-type parameters - #247
fix(ts-sdk): decode base64 data URLs with media-type parameters#247rohitsux wants to merge 2 commits into
Conversation
toImageBytes()'s regex ^data:[^;]+;base64, requires a ;-free media type, so valid RFC 2397 data URLs with a media-type parameter (e.g. data:image/svg+xml;charset=utf-8;base64,...) or an omitted media type (data:;base64,...) don't match and fall through, handing the entire data URL to the base64 decoder. That decoder then throws InvalidCharacterError under atob (browser) or silently corrupts bytes under Buffer.from (Node). Fix matches up to the ;base64, marker ([^,]*) instead. Base64 payloads never contain a comma, so the payload is still captured correctly. No change in behavior for existing inputs. Added two regression tests covering a media type with a parameter and an omitted media type.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughChangesImage data URL decoding
Merge Risk: ⚪ Minimal · up to This localized change correctly handles base64 data URLs with media-type parameters or omitted media types, with regression coverage and passing checks. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
mamayer19
left a comment
mamayer19
left a comment
There was a problem hiding this comment.
Thanks for the contribution, this reproduces the original issue clearly, and both new cases pass locally. I left one suggestion about parsing the data URL structurally instead of broadening the regex, along with a few related edge cases that would make the behavior explicit.
| // everything up to the ";base64," marker rather than a single ";"-free | ||
| // segment — otherwise such URLs fall through and the whole data URL is | ||
| // handed to the base64 decoder (corrupting the bytes or throwing). | ||
| const dataUrlMatch = input.match(/^data:[^,]*;base64,(.+)$/); |
There was a problem hiding this comment.
Could we avoid encoding the data-URL grammar in a regex here? Since the format has a well-defined comma delimiter, a small helper could find the first comma and inspect the semicolon-delimited metadata before it for the final base64 marker. That would be easier to follow and give us a natural place to handle the scheme and marker case-insensitively, percent-decode the payload, and return a clear error for malformed data: input.
As written, inputs such as DATA:image/png;BASE64,SGVsbG8%3D still miss the match and are passed to the raw-base64 decoder. Would you be open to extracting something like parseBase64DataUrl(input): string | undefined, leaving base64ToBytes responsible only for decoding the extracted payload?
| it("decodes a data URL whose media type carries a parameter", async () => { | ||
| // Valid per RFC 2397: the media type may be followed by ";param=value" | ||
| // (e.g. charset) before ";base64,". "Hello" base64-encoded. | ||
| const dataUrl = "data:image/svg+xml;charset=utf-8;base64,SGVsbG8="; |
There was a problem hiding this comment.
These two regression cases are a good start. If we switch to a small parser, could we also cover an uppercase scheme/BASE64 marker, percent-escaped padding such as %3D, and one explicit empty-payload case? Those are the boundaries where the current regex falls through into the raw-base64 path, and the tests would make the intended behavior clear.
2 participants
Problem
toImageBytes()inpackages/sie_ts_sdk/src/images.tsdetects base64 data URLs with the regex:This requires the media type segment to contain no
;. Per RFC 2397, though, the media type may carry parameters (e.g.;charset=utf-8) or be omitted entirely. Data URLs like:data:image/svg+xml;charset=utf-8;base64,...data:;base64,...don't match this pattern and fall through to the plain base64 branch, which hands the entire data URL string (including the
data:...;base64,prefix) to the base64 decoder. That decoder then either throwsInvalidCharacterErrorunderatob(browser) or silently produces corrupted bytes underBuffer.from(Node) — neither of which surfaces as a clear "unsupported input" error.Fix
Match up to the
;base64,marker instead of requiring a;-free segment:Base64 payloads never contain a comma, so the capture group still correctly isolates just the payload. Behavior for all previously-matching data URLs is unchanged.
Tests
Added two regression tests to
packages/sie_ts_sdk/tests/images.test.ts:image/svg+xml;charset=utf-8)data:;base64,...)Verified locally: fails-before (both new tests throw on the unpatched regex) / passes-after.
tests/images.test.ts17/17; full@superlinked/sie-sdksuite 484/484;biome checkclean;tsc --noEmit(typecheck) clean.Summary by CodeRabbit
Bug Fixes
Tests