Skip to content
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

It's not an error to pass a buffer larger than the decompressed data. #37

Closed

Conversation

TannerRogalsky
Copy link

As per the reference implementation of LZ4_decompress_safe:

the size of destination buffer (which must be already allocated), presumed an upper bound of decompressed size

I think this mistake originates from a misleading comment on lzzzz which seems to indicate that the buffer must be exactly the size of the decompressed data. However neither those bindings nor the underlying library make that assertion.

This PR is a breaking change since I removed the now unused UncompressedSizeDiffers variant from the DecompressError enum.

The reference implementation specifies that the size of the destination
buffer is presumed to be an upper bound. Passing too large a buffer is
not an error. Overwriting the buffer is handled as an error internally.
@codecov-commenter
Copy link

Codecov Report

Merging #37 (93fdf9c) into main (d3e3dab) will increase coverage by 0.90%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #37      +/-   ##
==========================================
+ Coverage   91.07%   91.98%   +0.90%     
==========================================
  Files          11       11              
  Lines        2197     2171      -26     
==========================================
- Hits         2001     1997       -4     
+ Misses        196      174      -22     
Impacted Files Coverage Δ
src/block/mod.rs 25.00% <ø> (+4.41%) ⬆️
src/block/decompress.rs 94.44% <100.00%> (+2.01%) ⬆️
src/block/decompress_safe.rs 97.11% <100.00%> (+3.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3e3dab...93fdf9c. Read the comment docs.

actual: decomp_len,
});
}
vec.truncate(decomp_len);
Copy link
Owner

Choose a reason for hiding this comment

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

this check also included if the passed buffer was smaller

@PSeitz
Copy link
Owner

PSeitz commented Feb 6, 2023

Thanks, I just realized that in that step only sizes that are larger are checked

It's implemented in this PR: #78

@PSeitz PSeitz closed this Feb 6, 2023
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.

None yet

3 participants