Copilot commented on code in PR #582:
URL: https://github.com/apache/avro-rs/pull/582#discussion_r3566725522
##########
avro/src/codec.rs:
##########
@@ -144,21 +144,34 @@ impl Codec {
/// Decompress a stream of bytes in-place.
pub fn decompress(self, stream: &mut Vec<u8>) -> AvroResult<()> {
+ // Cap the decompressed output at the configured allocation budget so a
+ // small compressed block cannot inflate to an enormous buffer (a
+ // "decompression bomb") and exhaust memory.
+ let max_bytes =
+
crate::util::max_allocation_bytes(crate::util::DEFAULT_MAX_ALLOCATION_BYTES);
*stream = match self {
Codec::Null => return Ok(()),
- Codec::Deflate(_settings) =>
miniz_oxide::inflate::decompress_to_vec(stream).map_err(|e| {
+ Codec::Deflate(_settings) =>
miniz_oxide::inflate::decompress_to_vec_with_limit(stream,
max_bytes).map_err(|e| {
+ use
miniz_oxide::inflate::TINFLStatus::{FailedCannotMakeProgress, BadParam,
Adler32Mismatch, Failed, Done, NeedsMoreInput, HasMoreOutput};
+ // The output would grow past the allocation budget: reject it
as
+ // a decompression bomb rather than allocating without bound.
+ if let HasMoreOutput = e.status {
+ return Error::new(Details::MemoryAllocation {
+ desired: max_bytes.saturating_add(1),
+ maximum: max_bytes,
+ });
+ }
let err = {
- use
miniz_oxide::inflate::TINFLStatus::{FailedCannotMakeProgress, BadParam,
Adler32Mismatch, Failed, Done, NeedsMoreInput, HasMoreOutput};
- use std::io::{Error,ErrorKind};
+ use std::io::{Error as IoError, ErrorKind};
match e.status {
- FailedCannotMakeProgress =>
Error::from(ErrorKind::UnexpectedEof),
- BadParam => Error::other("Unexpected error:
miniz_oxide reported invalid output buffer size. Please report this to avro-rs
developers."), // not possible for _to_vec()
- Adler32Mismatch => Error::from(ErrorKind::InvalidData),
- Failed => Error::from(ErrorKind::InvalidData),
- Done => Error::other("Unexpected error: miniz_oxide
reported an error with a success status. Please report this to avro-rs
developers."),
- NeedsMoreInput =>
Error::from(ErrorKind::UnexpectedEof),
- HasMoreOutput => Error::other("Unexpected error:
miniz_oxide has more data than the output buffer can hold. Please report this
to avro-rs developers."), // not possible for _to_vec()
- other => Error::other(format!("Unexpected error:
{other:?}"))
+ FailedCannotMakeProgress =>
IoError::from(ErrorKind::UnexpectedEof),
+ BadParam => IoError::other("Unexpected error:
miniz_oxide reported invalid output buffer size. Please report this to avro-rs
developers."), // not possible for _to_vec()
+ Adler32Mismatch =>
IoError::from(ErrorKind::InvalidData),
Review Comment:
The `BadParam` arm still uses an error message/comment that assumes
`decompress_to_vec()` ("invalid output buffer size" / "not possible for
_to_vec()"), but this code now uses `decompress_to_vec_with_limit()`. That
message is misleading for users and the comment is stale; update it to reflect
an invalid parameter case (or drop the claim that it’s impossible).
##########
avro/src/error.rs:
##########
@@ -513,6 +513,12 @@ pub enum Details {
#[error("Failed to decompress with zstd: {0}")]
ZstdDecompress(#[source] std::io::Error),
+ #[error("Failed to decompress with bzip2: {0}")]
+ Bzip2Decompress(#[source] std::io::Error),
+
+ #[error("Failed to decompress with xz: {0}")]
+ XzDecompress(#[source] std::io::Error),
Review Comment:
`Details` is a public enum, so adding unconditional variants is a breaking
change for downstream crates that exhaustively `match` on it. Since these
errors are only produced when the corresponding codec feature is enabled,
consider gating them behind the same features (like the existing Snappy
variants) to avoid breaking users who don’t enable `bzip`/`xz`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
