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: Advanced 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
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The SDK now accepts more valid base64 data URL forms and rejects malformed or empty payloads before image submission. Covered parsing and error behavior leave no current merge-blocking risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Biome (2.5.8)packages/sie_ts_sdk/src/images.tsBiome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins. packages/sie_ts_sdk/tests/images.test.tsBiome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins. 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.
…ases Replace the data-URL regex in toImageBytes with a parseBase64DataUrl helper that walks the RFC 2397 grammar structurally: it splits on the first comma, matches the data: scheme and the base64 marker case-insensitively, percent-decodes the payload, and throws a clear error for a malformed or non-base64 data: URL instead of silently handing it to the base64 decoder. base64ToBytes is now responsible only for decoding the extracted payload. Add tests for an uppercase scheme and BASE64 marker, percent-escaped padding (%3D), an empty payload, and the two clear-error cases.
|
Thanks for the review — reworked it along your suggestion.
Full SDK suite is green (513) with typecheck + biome clean. Happy to adjust the error wording if you'd prefer something different. |
… bytes An empty base64 payload (e.g. "data:;base64,") previously decoded to an empty Uint8Array and succeeded, so a zero-byte image was sent over the wire and only failed deep in the server, far from the mistake. parseBase64DataUrl now throws a clear error for an empty payload, and the empty-payload test expects it.
3 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