Skip to content

refactor: Fix signed/unsigned comparison in lz4Decompress - #7687

Open
BraedonKlock wants to merge 1 commit into
XRPLF:developfrom
BraedonKlock:bug-7521-lz4decompress-size-types
Open

refactor: Fix signed/unsigned comparison in lz4Decompress#7687
BraedonKlock wants to merge 1 commit into
XRPLF:developfrom
BraedonKlock:bug-7521-lz4decompress-size-types

Conversation

@BraedonKlock

Copy link
Copy Markdown

High Level Overview of Change

Fixes a signed/unsigned comparison in the stream-based lz4Decompress overload.

The stream returns chunkSize as an int, while the expected input size is represented as std::size_t. This change validates chunkSize before converting it to std::size_t, uses std::size_t for copied byte tracking, and simplifies the final insufficient-input check.

Fixes #7521

Context of Change

The previous implementation compared chunkSize directly against inSize. Since chunkSize is signed and inSize is unsigned, a negative chunkSize could be converted to a large unsigned value during comparison.

This change checks for a negative chunkSize before conversion, then uses a validated std::size_t value for size comparisons and copy calculations. It also changes copiedInSize to std::size_t and simplifies the final insufficient-input check to copiedInSize != inSize, so the final validation is based on whether the requested number of compressed input bytes was actually accounted for.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

⚠️ This PR contains unsigned commits. To get your PR merged, please sign them. ⚠️

If only the most recent commit is unsigned, you can run:

  1. Amend the commit: git commit --amend --no-edit -n -S
  2. Overwrite the commit: git push --force-with-lease

If multiple commits are unsigned, you can run:

  1. Go into interactive rebase mode: git rebase --interactive HEAD~<NUM_OF_COMMITS>, where NUM_OF_COMMITS is the number of most recent commits that will be available to edit.
  2. Change "pick" to "edit" for the commits you need to sign, and then save and exit.
  3. For each commit, run: git commit --amend --no-edit -n -S
  4. Continue the rebase: git rebase --continue
  5. Overwrite the commit(s): git push --force-with-lease

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

  1. Generate a GPG key
  2. Add the GPG key to your GitHub account
  3. Configure git to use your GPG key for commit signing
Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

  1. Generate an SSH key and add it to ssh-agent
  2. Add the SSH key to your GitHub account
  3. Configure git to use your SSH key for commit signing
Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.
See use 1Password to sign your commits.

@BraedonKlock
BraedonKlock force-pushed the bug-7521-lz4decompress-size-types branch from e65de33 to e7a216e Compare July 1, 2026 16:00
@ximinez

ximinez commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Hi @BraedonKlock. Thank you for your contributions! We really appreciate them. At this point, we ask that you hold off on submitting any more until we've had a chance to review the current batch. There is a not insignificant engineer overhead for each review, and it may take some time to get caught up. Too many PRs can make that harder. When we've gotten down to 3 open or fewer, feel free to create more!

In the meantime, you can open "Draft" PRs so that changes will be ready to go once we're ready for more. At that time, you'll be able to decide which ones are the best, and convert them to "Ready to review". Keep in mind that you will be responsible for keeping your PRs up to date with develop. The more you open, the more chances you'll have for conflicts, so don't get too carried away. 😄

@BraedonKlock

Copy link
Copy Markdown
Author

Thank you @ximinez for the clarification! I really appreciate the feedback. I completely understand. I'll hold off on opening additional ready for review PRs until my open count is down to three or fewer. In the meantime, I'll continue working locally and use draft PRs if appropriate. Thanks again!

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.

Signed/unsigned comparison in stream-based lz4Decompress

2 participants