iemejia commented on code in PR #3863:
URL: https://github.com/apache/avro/pull/3863#discussion_r3566894008
##########
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:
Fixed in 063699d58a: the PHP_INT_MIN guard uses a non-strict comparison
(==), so on 32-bit GMP builds where readLong() returns numeric strings the
minimum block count is still rejected.
##########
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:
Fixed in 063699d58a: same non-strict comparison applied to the map path.
##########
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:
Fixed in 063699d58a: skipArray now applies checkSkipCollectionCount() on the
negative (byte-sized) path too (normalizing the count, rejecting PHP_INT_MIN
and a negative block size), and reads the block byte-size from the passed
$decoder rather than $this. Added a regression test.
##########
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:
Fixed in 063699d58a: skipMap got the same treatment (bound negative blocks,
reject negative size, read 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]
