diff --git a/lang/perl/lib/Avro/BinaryDecoder.pm b/lang/perl/lib/Avro/BinaryDecoder.pm index d2bb65e14f7..df3316248f7 100644 --- a/lang/perl/lib/Avro/BinaryDecoder.pm +++ b/lang/perl/lib/Avro/BinaryDecoder.pm @@ -124,7 +124,12 @@ sub skip_bytes { my $class = shift; my $reader = pop; my $size = decode_long($class, undef, undef, $reader); - $reader->seek($size, 0); + throw Avro::Schema::Error::Parse("bytes length is negative: $size") + if $size < 0; + ## SEEK_CUR (1): skip forward $size bytes relative to the current position. + ## (Using SEEK_SET here would jump to an absolute offset and corrupt the + ## decoding of every subsequent field.) + $reader->seek($size, 1); return; } @@ -219,14 +224,17 @@ sub skip_block { my ($reader, $block_content) = @_; my $block_count = decode_long($class, undef, undef, $reader); while ($block_count) { + ## A negative block count means abs($block_count) items follow, preceded + ## by a long block size (in bytes). Take the absolute value and consume + ## the block size (matching decode_array/decode_map). if ($block_count < 0) { - $reader->seek($block_count, 0); - next; + $block_count = -$block_count; + my $block_size = decode_long($class, undef, undef, $reader); + throw Avro::Schema::Error::Parse("block size is negative: $block_size") + if $block_size < 0; } - else { - for (1..$block_count) { - $block_content->(); - } + for (1..$block_count) { + $block_content->(); } $block_count = decode_long($class, undef, undef, $reader); } @@ -235,7 +243,7 @@ sub skip_block { sub skip_array { my $class = shift; my ($schema, $reader) = @_; - skip_block($reader, sub { $class->skip($schema->items, $reader) }); + $class->skip_block($reader, sub { $class->skip($schema->items, $reader) }); } ## 1.3.2 Arrays are encoded as a series of blocks. Each block consists of a @@ -273,7 +281,7 @@ sub decode_array { sub skip_map { my $class = shift; my ($schema, $reader) = @_; - skip_block($reader, sub { + $class->skip_block($reader, sub { skip_string($class, $reader); $class->skip($schema->values, $reader); }); @@ -350,7 +358,8 @@ sub decode_union { sub skip_fixed { my $class = shift; my ($schema, $reader) = @_; - $reader->seek($schema->size, 0); + ## SEEK_CUR (1): skip forward relative to the current position. + $reader->seek($schema->size, 1); } ## 1.3.2 Fixed instances are encoded using the number of bytes declared in the diff --git a/lang/perl/t/03_bin_decode.t b/lang/perl/t/03_bin_decode.t index e5da35d9983..7cc968eab8b 100644 --- a/lang/perl/t/03_bin_decode.t +++ b/lang/perl/t/03_bin_decode.t @@ -255,4 +255,100 @@ EOP is $dec->{one}[0], 1.0, "kind of dumb test"; } +## AVRO-4343: schema-resolution skip path (skip fields present in the writer but +## absent from the reader). Regression tests for the broken skip implementation. +{ + my $enc_with = sub { + my ($schema, $data) = @_; + my $out = ''; + Avro::BinaryEncoder->encode( + schema => $schema, data => $data, + emit_cb => sub { $out .= ${ $_[0] } }, + ); + return $out; + }; + + ## skip a NON-trailing bytes field, then decode the field after it + { + my $w = Avro::Schema->parse(q( + { "type":"record","name":"S","fields":[ + {"name":"b","type":"bytes"}, + {"name":"a","type":"long"} ]})); + my $r = Avro::Schema->parse(q( + { "type":"record","name":"S","fields":[ + {"name":"a","type":"long"} ]})); + my $enc = $enc_with->($w, { b => "xx", a => 777 }); + open my $reader, '<', \$enc or die $!; + my $dec = Avro::BinaryDecoder->decode( + writer_schema => $w, reader_schema => $r, reader => $reader); + is $dec->{a}, 777, "skip non-trailing bytes field, next field intact"; + } + + ## skip an array field (previously crashed) + { + my $w = Avro::Schema->parse(q( + { "type":"record","name":"A","fields":[ + {"name":"a","type":"long"}, + {"name":"arr","type":{"type":"array","items":"long"}} ]})); + my $r = Avro::Schema->parse(q( + { "type":"record","name":"A","fields":[ + {"name":"a","type":"long"} ]})); + my $enc = $enc_with->($w, { a => 1, arr => [ 1, 2, 3 ] }); + open my $reader, '<', \$enc or die $!; + my $dec = Avro::BinaryDecoder->decode( + writer_schema => $w, reader_schema => $r, reader => $reader); + is $dec->{a}, 1, "skip array field during resolution"; + } + + ## skip a map field (previously crashed) + { + my $w = Avro::Schema->parse(q( + { "type":"record","name":"M","fields":[ + {"name":"a","type":"long"}, + {"name":"m","type":{"type":"map","values":"long"}} ]})); + my $r = Avro::Schema->parse(q( + { "type":"record","name":"M","fields":[ + {"name":"a","type":"long"} ]})); + my $enc = $enc_with->($w, { a => 2, m => { x => 1, y => 2 } }); + open my $reader, '<', \$enc or die $!; + my $dec = Avro::BinaryDecoder->decode( + writer_schema => $w, reader_schema => $r, reader => $reader); + is $dec->{a}, 2, "skip map field during resolution"; + } + + ## skip an array encoded with a NEGATIVE block count (+ block size) + { + my $w = Avro::Schema->parse(q( + { "type":"record","name":"A","fields":[ + {"name":"a","type":"long"}, + {"name":"arr","type":{"type":"array","items":"long"}} ]})); + my $r = Avro::Schema->parse(q( + { "type":"record","name":"A","fields":[ + {"name":"a","type":"long"} ]})); + # a=1; arr: count=-3, size=3, items 1,2,3, end=0 + my $enc = "\x02" . "\x05\x06" . "\x02\x04\x06" . "\x00"; + open my $reader, '<', \$enc or die $!; + my $dec = Avro::BinaryDecoder->decode( + writer_schema => $w, reader_schema => $r, reader => $reader); + is $dec->{a}, 1, "skip array with negative block count"; + } + + ## a negative bytes length in a skipped field must be rejected, not mis-seek + { + my $w = Avro::Schema->parse(q( + { "type":"record","name":"S","fields":[ + {"name":"b","type":"bytes"}, + {"name":"a","type":"long"} ]})); + my $r = Avro::Schema->parse(q( + { "type":"record","name":"S","fields":[ + {"name":"a","type":"long"} ]})); + my $enc = "\x01" . "\x0a"; # b: bytes length -1 (malformed); a=5 + open my $reader, '<', \$enc or die $!; + throws_ok { + Avro::BinaryDecoder->decode( + writer_schema => $w, reader_schema => $r, reader => $reader); + } qr/negative/, "negative bytes length in skipped field is rejected"; + } +} + done_testing;