Copilot commented on code in PR #3623:
URL: https://github.com/apache/avro/pull/3623#discussion_r2687965545
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
Review Comment:
The block count (`blocks_total`) read from the file should also be validated
to ensure it's not negative, similar to the block size validation added below.
A negative block count could lead to incorrect behavior in the read logic since
`blocks_read` (which starts at 0 and only increments) would never equal a
negative `blocks_total`, potentially preventing proper block reading.
```suggestion
"Cannot read file block count: ");
if (r->blocks_total < 0) {
avro_set_error("Invalid block count: %" PRId64,
r->blocks_total);
return EINVAL;
}
```
##########
lang/c/src/datafile.c:
##########
@@ -451,6 +451,10 @@ static int file_read_block_count(avro_file_reader_t r)
"Cannot read file block count: ");
check_prefix(rval, enc->read_long(r->reader, &len),
"Cannot read file block size: ");
+ if (len < 0) {
+ avro_set_error("Invalid block size: %" PRId64, len);
+ return EINVAL;
+ }
Review Comment:
Consider adding a regression test that verifies the negative block size
validation. While the fix was verified with AddressSanitizer fuzzing, a unit
test with a malformed Avro file containing a negative block size would help
prevent regressions. This could follow the pattern of other test files like
test_avro_1237.c which test handling of malformed Avro files.
--
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]
