fix(media-core): accept unpadded inline base64 images#96437
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 11:37 AM ET / 15:37 UTC. Summary Reproducibility: yes. from source with high confidence: current main rejects unpadded cleaned base64 before the inline-image sanitizer decodes it, and the branch adds coverage for the accepted unpadded case. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land or adopt the media-core canonicalizer fix after final current-head checks, with review anchored to the current-main three-file diff rather than the stale-base file list. Do we have a high-confidence way to reproduce the issue? Yes, from source with high confidence: current main rejects unpadded cleaned base64 before the inline-image sanitizer decodes it, and the branch adds coverage for the accepted unpadded case. Is this the best way to solve the issue? Yes. The shared canonicalizeBase64 helper is the narrowest maintainable boundary because inline-image URLs and other base64 media consumers already validate there before decoding. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fa2379dbc883. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Merged via squash.
Thanks @lin-hongkuan! |
Merged via squash. Prepared head SHA: dc4693b Co-authored-by: lin-hongkuan <234943746+lin-hongkuan@users.noreply.github.com> Co-authored-by: vincentkoc <25068+vincentkoc@users.noreply.github.com> Reviewed-by: @vincentkoc
Merged via squash. Prepared head SHA: dc4693b Co-authored-by: lin-hongkuan <234943746+lin-hongkuan@users.noreply.github.com> Co-authored-by: vincentkoc <25068+vincentkoc@users.noreply.github.com> Reviewed-by: @vincentkoc
What Problem This Solves
canonicalizeBase64()rejected every payload whose length was not already a multiple of four. That rejects common unpadded base64 such asSGVsbG8, even though the same bytes are valid once canonical padding is restored.This also made inline image data URLs fail when the image bytes were valid but the base64 payload omitted trailing
=padding.Why This Change Was Made
The canonicalizer now pads unpadded base64 only when the length remainder is valid for base64 decoding. It still rejects empty input, impossible length remainder
1, more than two padding characters, and any data after padding.The tests cover both the low-level helper and the inline image sanitizer path so the user-visible data URL behavior is protected.
User Impact
Valid unpadded inline image data URLs can now be normalized and used instead of being dropped as malformed. Malformed or hostile payloads are still rejected before decoding.
Evidence
Direct behavior probe:
Node base64 decode reference probe:
Targeted tests:
Formatting:
Whitespace:
AI-assisted
Prepared with Codex. I reviewed the change, understand the touched code path, and kept the PR focused on the bug described above.