Issue 821 #823
Merged
Issue 821 #823
Conversation
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
"base64 decoding with stop_before_partial returning OUTPUT_BUFFER_IS_TOO_SMALL for illegal padded chunk" The issue was reported by Shu-yu Guo. The fix in this commit is quite simple and involves adding a missing test. It also adds more tests and fixes some typos in the documentation.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issue #821 by adding missing tests for padded-base64 handling, strengthening error checks in decoding implementations, and correcting typos in documentation.
- Introduce a
std::stringoverload ofadd_simple_spacesand add TC39-based tests for illegal padded chunks. - Add guards in scalar and Ice Lake implementations to reject invalid padding lengths.
- Fix spelling (“taylor” → “tailor”) and expand the README with detailed WHATWG forgiving-base64 documentation.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/base64_tests.cpp | Added string version of add_simple_spaces, new padded-chunk tests, and removed outdated test |
| src/scalar/base64.h | Added check to reject cases where idx + padding_characters > 4 |
| src/icelake/icelake_base64.inl.cpp | Duplicated the invalid-padding-length guard for Ice Lake backend |
| include/simdutf/implementation.h | Fixed “taylor” → “tailor” typos in documentation comments |
| README.md | Expanded WHATWG forgiving-base64 section; corrected grammar and typos |
Comments suppressed due to low confidence (1)
README.md:1848
- [nitpick] Grammar issue: change 'We also converting from' to 'We also convert from' or 'We also support converting from'.
We also converting from [WHATWG forgiving-base64](https://infra.spec.whatwg.org/#forgiving-base64-decode) to binary, and back. In particular, you can convert base64 inputs which contain ASCII spaces (' ', '\t', '\n', '\r', '\f') to binary. We also support the base64 URL encoding alternative. These functions are part of the Node.js JavaScript runtime: in particular `atob` in Node.js relies on simdutf.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| ASSERT_EQUAL(back, expected); | ||
| } | ||
|
|
||
| // https://github.com/tc39/test262 |
There was a problem hiding this comment.
Member
Author
There was a problem hiding this comment.
Thanks. I will fix in a pre-release commit.
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #821
The issue was reported by Shu-yu Guo (@syg) with feedback from Kevin Gibbons (@bakkot).
The fix in this commit is quite simple and involves adding a missing test that we probably should have had.
It also adds more tests (in addition to what @syg provided) and fixes some typos in the documentation: somehow, I was using 'taylor' instead of 'tailor'.
I am deliberately keeping @syg's commit so that they get credit.
This is related to WebKit PR WebKit/WebKit#47926