Skip to content

AVRO-4343: [perl] Fix broken decoder skip path used in schema resolution #3947

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.

Sign up for GitHub

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

Open
iemejia wants to merge 1 commit into
base: main
Choose a base branch
from
+115 −10
Open

AVRO-4343: [perl] Fix broken decoder skip path used in schema resolution #3947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions lang/perl/lib/Avro/BinaryDecoder.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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
Expand Down
96 changes: 96 additions & 0 deletions lang/perl/t/03_bin_decode.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Loading