iemejia commented on code in PR #3864:
URL: https://github.com/apache/avro/pull/3864#discussion_r3567581847


##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -206,7 +371,14 @@ sub decode_enum {
     my ($writer_schema, $reader_schema, $reader) = @_;
     my $index = decode_int($class, @_);
 
-    my $w_data = $writer_schema->symbols->[$index];
+    my $symbols = $writer_schema->symbols;
+    ## A negative or out-of-range index is malformed; reject it before indexing
+    ## (Perl's negative indexing would otherwise select the wrong symbol).
+    if ($index < 0 || $index >= scalar @$symbols) {
+        throw Avro::Schema::Error::Parse(
+            "Enum symbol index $index out of range for " . scalar(@$symbols) . 
" symbols");
+    }

Review Comment:
   Unlike skip_union (where the branch index selects *which* schema to skip, so 
a wrong index desyncs the stream), the enum index doesn't affect skipping: an 
enum is encoded as just the index varint with nothing following it, so 
skip_enum reads the varint and moves on regardless of the value. An 
out-of-range index is therefore skipped correctly without desync or allocation, 
so validating it here would only be for parity with decode_enum, not 
correctness/safety. I've left skip_enum as skip_int to keep the skip path 
minimal; happy to add the schema-aware check if you'd prefer strict parity.



##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -273,7 +502,8 @@ sub decode_array {
 sub skip_map {
     my $class = shift;
     my ($schema, $reader) = @_;
-    skip_block($reader, sub {
+    # Map entries always carry a >= 1 byte key, so pass a positive minimum.
+    skip_block($reader, 1 + _min_bytes_per_element($schema->values), sub {

Review Comment:
   Similarly, an empty map key is skipped correctly (a 0-length string = a 
1-byte length prefix and no data), so it doesn't desync or over-read during 
projection — decode_map rejects it for data validity, but skip only needs to 
advance past it. So there's no correctness/security impact here; it's a 
decode-vs-skip strictness parity point. Left as-is to keep skip minimal.



-- 
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]

Reply via email to