Skip to content

fixing Issue 824 #825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

fixing Issue 824 #825

wants to merge 4 commits into from

Conversation

lemire
Copy link
Member

@lemire lemire commented Jul 17, 2025

This should fix #824

Thanks for the report by @syg

This PR changes the following things...

  1. Previously, we intentionally removed (ignored) trailing ignorable characters. They are now included. This when decoding AAAA , we would declare having read 4 characters.
  2. When the input was empty (except for ignorable characters), we would report reading zero characters.
  3. With partial chunk handling, we would stop right before the partial chunk, we now end right after the last complete chunk (thus dropping the ignorable characters).

I have added some tests.

@lemire lemire requested a review from Copilot July 17, 2025 23:10
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes issue #824 related to handling of trailing ignorable characters in base64 decoding, specifically addressing how the input read count is reported according to TC39 base64 proposal specifications.

Key changes made:

  • Modified the handling of trailing ignorable characters to include them in the reported read count instead of ignoring them
  • Updated the behavior for empty input (containing only ignorable characters) to return the full input length instead of zero
  • Enhanced partial chunk handling with stop_before_partial option to properly position after the last complete chunk

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/base64_tests.cpp Adds new test cases for issue #824 and updates existing WebKit test cases to verify correct read/write counts
src/scalar/base64.h Updates find_end function to include trailing spaces in full input length and modifies empty input handling
src/lsx/lsx_base64.cpp Changes empty input case to return full_input_length instead of 0
src/lasx/lasx_base64.cpp Changes empty input case to return full_input_length instead of 0
src/icelake/icelake_base64.inl.cpp Changes empty input case and adds logic to skip ignored characters for stop_before_partial
src/generic/base64.h Changes empty input case to return full_input_length instead of 0
src/arm64/arm_base64.cpp Changes empty input case to return full_input_length instead of 0

@lemire lemire marked this pull request as draft July 18, 2025 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read count is incorrect when using last_chunk_handling::stop_before_partial
1 participant