Copilot commented on code in PR #3855:
URL: https://github.com/apache/avro/pull/3855#discussion_r3564613410
##########
lang/perl/lib/Avro/DataFileReader.pm:
##########
@@ -28,14 +28,19 @@ use Object::Tiny qw{
use constant MARKER_SIZE => 16;
-# TODO: refuse to read a block more than block_max_size, instead
-# do partial reads
+# A data-file block is decompressed according to the file's codec. A block with
+# a very high compression ratio (or a malformed block) can expand to far more
+# memory than its compressed size. To guard against unbounded allocation, the
+# decompressed size of a single block is capped. This mirrors the Java SDK's
+# decompression limit (AVRO-4247). The default can be overridden with the
+# AVRO_MAX_DECOMPRESS_LENGTH environment variable.
+use constant DEFAULT_MAX_DECOMPRESS_LENGTH => 200 * 1024 * 1024; # 200 MiB
use Avro::DataFile;
use Avro::BinaryDecoder;
use Avro::Schema;
use Carp;
-use Compress::Zstd;
+use Compress::Zstd::Decompressor;
use IO::Uncompress::Bunzip2 qw(bunzip2);
Review Comment:
`IO::Uncompress::Bunzip2` is now used via the class method
(`IO::Uncompress::Bunzip2->new`), but `bunzip2` is still imported and never
referenced. This unused import adds noise and may trigger linting/Perl::Critic
warnings.
##########
lang/perl/lib/Avro/DataFileReader.pm:
##########
@@ -215,24 +220,98 @@ sub read_block_header {
my $marker = substr $block, -(MARKER_SIZE), MARKER_SIZE, '';
$datafile->{block_marker} = $marker;
+ ## The decompressed size of a block is capped to guard against a block with
+ ## a very high compression ratio expanding to far more memory than its
+ ## compressed size. The limit is the AVRO_MAX_DECOMPRESS_LENGTH environment
+ ## variable, or DEFAULT_MAX_DECOMPRESS_LENGTH. (Note: block_max_size is a
+ ## writer-side flush threshold measured in compressed bytes and is not
reused
+ ## here to avoid conflating the two units.)
+ my $limit = _max_decompress_length();
+
Review Comment:
The PR description says the decompression limit uses the existing
`block_max_size` attribute when set, but the implementation now always uses
`AVRO_MAX_DECOMPRESS_LENGTH` / the default and explicitly does *not* reuse
`block_max_size` (see the note here). Please update the PR description to match
the current behavior so users aren’t misled about how to configure the limit.
--
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]
