Copilot commented on code in PR #3863:
URL: https://github.com/apache/avro/pull/3863#discussion_r3566791929
##########
lang/php/lib/Datum/AvroIODatumReader.php:
##########
@@ -284,12 +307,19 @@ public function readArray(
AvroIOBinaryDecoder $decoder
): array {
$items = [];
+ $minBytes = self::minBytesPerElement($writersSchema->items());
$blockCount = $decoder->readLong();
- while (0 !== $blockCount) {
+ while (0 != $blockCount) {
if ($blockCount < 0) {
+ // PHP_INT_MIN cannot be negated: -PHP_INT_MIN promotes to a
+ // float, so reject it rather than propagating a non-int count.
+ if (PHP_INT_MIN === $blockCount) {
Review Comment:
PHP_INT_MIN handling uses a strict comparison. On 32-bit builds with GMP,
readLong() returns numeric strings, so a block count of '-2147483648' won't
match PHP_INT_MIN and will still be negated, promoting to float. Use a
non-strict numeric comparison so both ints and numeric strings are rejected
consistently.
##########
lang/php/lib/Datum/AvroIOBinaryDecoder.php:
##########
@@ -235,28 +273,39 @@ public function skipRecord(AvroRecordSchema
$writersSchema, AvroIOBinaryDecoder
public function skipArray(AvroArraySchema $writersSchema,
AvroIOBinaryDecoder $decoder): void
{
+ $minBytes =
AvroIODatumReader::collectionElementMinBytes($writersSchema->items());
+ $skipped = 0;
$blockCount = $decoder->readLong();
- while (0 !== $blockCount) {
+ while (0 != $blockCount) {
if ($blockCount < 0) {
$decoder->skip($this->readLong());
- }
- for ($i = 0; $i < $blockCount; $i++) {
- AvroIODatumReader::skipData($writersSchema->items(), $decoder);
+ } else {
+ AvroIODatumReader::checkSkipCollectionCount($skipped,
$blockCount, $minBytes);
+ $skipped += $blockCount;
+ for ($i = 0; $i < $blockCount; $i++) {
+ AvroIODatumReader::skipData($writersSchema->items(),
$decoder);
+ }
}
$blockCount = $decoder->readLong();
}
}
public function skipMap(AvroMapSchema $writersSchema, AvroIOBinaryDecoder
$decoder): void
{
+ // Map entries always carry a >= 1 byte key, so the minimum is
positive.
+ $minBytes = 1 +
AvroIODatumReader::collectionElementMinBytes($writersSchema->values());
+ $skipped = 0;
$blockCount = $decoder->readLong();
- while (0 !== $blockCount) {
+ while (0 != $blockCount) {
if ($blockCount < 0) {
$decoder->skip($this->readLong());
- }
- for ($i = 0; $i < $blockCount; $i++) {
- $decoder->skipString();
- AvroIODatumReader::skipData($writersSchema->values(),
$decoder);
+ } else {
+ AvroIODatumReader::checkSkipCollectionCount($skipped,
$blockCount, $minBytes);
Review Comment:
skipMap() applies checkSkipCollectionCount() only for non-negative blocks. A
stream can use negative block counts (with a declared block byte-size) to
bypass the skip limit entirely; also the block byte-size is read from $this
rather than the provided $decoder. Enforce the skip count for negative blocks
too, and read the block size from $decoder.
##########
lang/php/lib/Datum/AvroIODatumReader.php:
##########
@@ -312,14 +342,27 @@ public function readMap(
AvroIOBinaryDecoder $decoder
): array {
$items = [];
+ $minBytes = 1 + self::minBytesPerElement($writersSchema->values());
+ $read = 0; // Cumulative pairs read; count($items) would undercount
duplicate keys.
$pair_count = $decoder->readLong();
while (0 != $pair_count) {
if ($pair_count < 0) {
+ // PHP_INT_MIN cannot be negated: -PHP_INT_MIN promotes to a
+ // float, so reject it rather than propagating a non-int count.
+ if (PHP_INT_MIN === $pair_count) {
Review Comment:
PHP_INT_MIN handling uses a strict comparison. On 32-bit builds with GMP,
readLong() returns numeric strings, so a block count of '-2147483648' won't
match PHP_INT_MIN and will still be negated, promoting to float. Use a
non-strict numeric comparison so both ints and numeric strings are rejected
consistently.
##########
lang/php/lib/Datum/AvroIOBinaryDecoder.php:
##########
@@ -235,28 +273,39 @@ public function skipRecord(AvroRecordSchema
$writersSchema, AvroIOBinaryDecoder
public function skipArray(AvroArraySchema $writersSchema,
AvroIOBinaryDecoder $decoder): void
{
+ $minBytes =
AvroIODatumReader::collectionElementMinBytes($writersSchema->items());
+ $skipped = 0;
$blockCount = $decoder->readLong();
- while (0 !== $blockCount) {
+ while (0 != $blockCount) {
if ($blockCount < 0) {
$decoder->skip($this->readLong());
- }
- for ($i = 0; $i < $blockCount; $i++) {
- AvroIODatumReader::skipData($writersSchema->items(), $decoder);
+ } else {
+ AvroIODatumReader::checkSkipCollectionCount($skipped,
$blockCount, $minBytes);
Review Comment:
skipArray() applies checkSkipCollectionCount() only for non-negative blocks.
A stream can use negative block counts (with a declared block byte-size) to
bypass the skip limit entirely; also the block byte-size is read from $this
rather than the provided $decoder. Enforce the skip count for negative blocks
too, and read the block size from $decoder.
--
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]
