Copilot commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3567638028
##########
lang/csharp/src/apache/test/IO/BinaryCodecTests.cs:
##########
@@ -430,5 +441,330 @@ public void TestFixed(int size)
TestSkip(b, (Decoder d) => d.SkipFixed(size),
(Encoder e, byte[] t) => e.WriteFixed(t), size);
}
+
+ // A bytes/string value is a length prefix followed by that many bytes.
+ // A malicious or truncated input can declare a huge length with little
+ // actual data; on a seekable stream the reader must reject it before
+ // allocating, rather than attempting a huge allocation.
+ [Test]
+ public void TestReadBytesRejectsLengthBeyondStream()
+ {
+ MemoryStream ms = new MemoryStream();
+ Encoder e = new BinaryEncoder(ms);
+ e.WriteLong(1_000_000); // declares 1,000,000 bytes...
+ ms.Position = 0; // ...but no data follows
+ Decoder d = new BinaryDecoder(ms);
+ Assert.Throws<AvroException>(() => d.ReadBytes());
+ }
+
+ [Test]
+ public void TestReadStringRejectsLengthBeyondStream()
+ {
+ MemoryStream ms = new MemoryStream();
+ Encoder e = new BinaryEncoder(ms);
+ e.WriteLong(1_000_000); // declares 1,000,000 bytes...
+ ms.Position = 0; // ...but no data follows
+ Decoder d = new BinaryDecoder(ms);
+ Assert.Throws<AvroException>(() => d.ReadString());
+ }
+
+ // A well-formed value whose declared length fits the stream still
reads.
+ [Test]
+ public void TestReadBytesWithinStreamStillReads()
+ {
+ byte[] payload = Encoding.UTF8.GetBytes("hello");
+ MemoryStream ms = new MemoryStream();
+ Encoder e = new BinaryEncoder(ms);
+ e.WriteBytes(payload);
+ ms.Position = 0;
+ Decoder d = new BinaryDecoder(ms);
+ Assert.AreEqual(payload, d.ReadBytes());
+ }
+
+ // On a non-seekable stream the remaining length is unknown, so the
+ // pre-check is skipped and a valid value still decodes.
+ [Test]
+ public void TestReadBytesNonSeekableStreamStillReads()
+ {
+ byte[] payload = Encoding.UTF8.GetBytes("hello");
+ MemoryStream backing = new MemoryStream();
+ Encoder e = new BinaryEncoder(backing);
+ e.WriteBytes(payload);
+ byte[] encoded = backing.ToArray();
+
+ using (var ns = new NonSeekableStream(new MemoryStream(encoded)))
+ {
+ Decoder d = new BinaryDecoder(ns);
+ Assert.AreEqual(payload, d.ReadBytes());
+ }
+ }
+
+ // An array/map block declares an element count; a malicious or
truncated
+ // input can declare far more elements than the remaining bytes could
+ // hold. The count is validated against the bytes remaining before
+ // allocating, using the minimum on-wire size of the element schema (so
+ // 0-byte elements like null are not falsely rejected).
+ [Test]
+ public void TestReadArrayRejectsCountBeyondStream()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"long\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(1000000); // 1,000,000 longs, no
data
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
+
+ [Test]
+ public void TestReadMapRejectsCountBeyondStream()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"map\",\"values\":\"long\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(1000000);
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
+
+ [Test]
+ public void TestReadArrayOfNullNotFalselyRejected()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}");
+ var ms = new MemoryStream();
+ var enc = new BinaryEncoder(ms);
+ enc.WriteLong(100000); // one block of 100,000 nulls (zero bytes
each)
+ enc.WriteLong(0); // end-of-array marker
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ var result = (Array)reader.Read(null, new BinaryDecoder(ms));
+ Assert.AreEqual(100000, result.Length);
+ }
+
+ // A negative bytes length must be rejected explicitly rather than
+ // flowing into a negative array allocation.
+ [Test]
+ public void TestReadBytesRejectsNegativeLength()
+ {
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(-5);
+ ms.Position = 0;
+ var d = new BinaryDecoder(ms);
+ Assert.Throws<AvroException>(() => d.ReadBytes());
+ }
+
+ // A block count larger than int.MaxValue must be rejected before the
+ // int cast, even for a null-element array where the byte check is
+ // skipped.
+ [Test]
+ public void TestReadArrayRejectsCountAboveIntMax()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong((long)int.MaxValue + 1);
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
+
+ // A bytes length above the maximum .NET array length must be rejected
+ // with a consistent AvroException rather than letting new byte[p]
throw.
+ // A non-seekable stream is used so the length reaches the array-length
+ // check without needing gigabytes of backing data.
+ [Test]
+ public void TestReadBytesRejectsLengthAboveMaxArrayLength()
+ {
+ var backing = new MemoryStream();
+ new BinaryEncoder(backing).WriteLong(3_000_000_000L); // > any
MaxDotNetArrayLength
+ byte[] encoded = backing.ToArray();
+ using (var ns = new NonSeekableStream(new MemoryStream(encoded)))
+ {
+ var d = new BinaryDecoder(ns);
+ Assert.Throws<AvroException>(() => d.ReadBytes());
+ }
+ }
+
+ // A zero-byte element type (null) consumes no input, so the
+ // bytes-remaining check cannot bound its block count; a tiny payload
+ // declaring a huge count must instead be rejected by the item cap
+ // before building the array.
+ [Test]
+ public void TestReadArrayOfNullRejectsHugeCount()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(200_000_000); // ~4 byte payload
Review Comment:
Using a declared count of 200,000,000 makes this test potentially
catastrophic (very long run / OOM) if the collection-size guard regresses in
the future, since an array<null> consumes no input and the loop could attempt
to allocate/grow toward that size. Prefer a count just above the default
zero-byte item cap (10,000,000) to keep the failure mode bounded while still
exercising the rejection path.
##########
lang/csharp/src/apache/test/IO/BinaryCodecTests.cs:
##########
@@ -430,5 +441,330 @@ public void TestFixed(int size)
TestSkip(b, (Decoder d) => d.SkipFixed(size),
(Encoder e, byte[] t) => e.WriteFixed(t), size);
}
+
+ // A bytes/string value is a length prefix followed by that many bytes.
+ // A malicious or truncated input can declare a huge length with little
+ // actual data; on a seekable stream the reader must reject it before
+ // allocating, rather than attempting a huge allocation.
+ [Test]
+ public void TestReadBytesRejectsLengthBeyondStream()
+ {
+ MemoryStream ms = new MemoryStream();
+ Encoder e = new BinaryEncoder(ms);
+ e.WriteLong(1_000_000); // declares 1,000,000 bytes...
+ ms.Position = 0; // ...but no data follows
+ Decoder d = new BinaryDecoder(ms);
+ Assert.Throws<AvroException>(() => d.ReadBytes());
+ }
+
+ [Test]
+ public void TestReadStringRejectsLengthBeyondStream()
+ {
+ MemoryStream ms = new MemoryStream();
+ Encoder e = new BinaryEncoder(ms);
+ e.WriteLong(1_000_000); // declares 1,000,000 bytes...
+ ms.Position = 0; // ...but no data follows
+ Decoder d = new BinaryDecoder(ms);
+ Assert.Throws<AvroException>(() => d.ReadString());
+ }
+
+ // A well-formed value whose declared length fits the stream still
reads.
+ [Test]
+ public void TestReadBytesWithinStreamStillReads()
+ {
+ byte[] payload = Encoding.UTF8.GetBytes("hello");
+ MemoryStream ms = new MemoryStream();
+ Encoder e = new BinaryEncoder(ms);
+ e.WriteBytes(payload);
+ ms.Position = 0;
+ Decoder d = new BinaryDecoder(ms);
+ Assert.AreEqual(payload, d.ReadBytes());
+ }
+
+ // On a non-seekable stream the remaining length is unknown, so the
+ // pre-check is skipped and a valid value still decodes.
+ [Test]
+ public void TestReadBytesNonSeekableStreamStillReads()
+ {
+ byte[] payload = Encoding.UTF8.GetBytes("hello");
+ MemoryStream backing = new MemoryStream();
+ Encoder e = new BinaryEncoder(backing);
+ e.WriteBytes(payload);
+ byte[] encoded = backing.ToArray();
+
+ using (var ns = new NonSeekableStream(new MemoryStream(encoded)))
+ {
+ Decoder d = new BinaryDecoder(ns);
+ Assert.AreEqual(payload, d.ReadBytes());
+ }
+ }
+
+ // An array/map block declares an element count; a malicious or
truncated
+ // input can declare far more elements than the remaining bytes could
+ // hold. The count is validated against the bytes remaining before
+ // allocating, using the minimum on-wire size of the element schema (so
+ // 0-byte elements like null are not falsely rejected).
+ [Test]
+ public void TestReadArrayRejectsCountBeyondStream()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"long\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(1000000); // 1,000,000 longs, no
data
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
+
+ [Test]
+ public void TestReadMapRejectsCountBeyondStream()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"map\",\"values\":\"long\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(1000000);
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
+
+ [Test]
+ public void TestReadArrayOfNullNotFalselyRejected()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}");
+ var ms = new MemoryStream();
+ var enc = new BinaryEncoder(ms);
+ enc.WriteLong(100000); // one block of 100,000 nulls (zero bytes
each)
+ enc.WriteLong(0); // end-of-array marker
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ var result = (Array)reader.Read(null, new BinaryDecoder(ms));
+ Assert.AreEqual(100000, result.Length);
+ }
+
+ // A negative bytes length must be rejected explicitly rather than
+ // flowing into a negative array allocation.
+ [Test]
+ public void TestReadBytesRejectsNegativeLength()
+ {
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(-5);
+ ms.Position = 0;
+ var d = new BinaryDecoder(ms);
+ Assert.Throws<AvroException>(() => d.ReadBytes());
+ }
+
+ // A block count larger than int.MaxValue must be rejected before the
+ // int cast, even for a null-element array where the byte check is
+ // skipped.
+ [Test]
+ public void TestReadArrayRejectsCountAboveIntMax()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong((long)int.MaxValue + 1);
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
+
+ // A bytes length above the maximum .NET array length must be rejected
+ // with a consistent AvroException rather than letting new byte[p]
throw.
+ // A non-seekable stream is used so the length reaches the array-length
+ // check without needing gigabytes of backing data.
+ [Test]
+ public void TestReadBytesRejectsLengthAboveMaxArrayLength()
+ {
+ var backing = new MemoryStream();
+ new BinaryEncoder(backing).WriteLong(3_000_000_000L); // > any
MaxDotNetArrayLength
+ byte[] encoded = backing.ToArray();
+ using (var ns = new NonSeekableStream(new MemoryStream(encoded)))
+ {
+ var d = new BinaryDecoder(ns);
+ Assert.Throws<AvroException>(() => d.ReadBytes());
+ }
+ }
+
+ // A zero-byte element type (null) consumes no input, so the
+ // bytes-remaining check cannot bound its block count; a tiny payload
+ // declaring a huge count must instead be rejected by the item cap
+ // before building the array.
+ [Test]
+ public void TestReadArrayOfNullRejectsHugeCount()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(200_000_000); // ~4 byte payload
+ ms.Position = 0;
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
+
+ // A non-zero-byte array on a non-seekable stream cannot have its block
+ // count bounded by the bytes remaining (the length is unknown). The
+ // backing array must therefore be grown on demand rather than
+ // preallocated to the declared count, so a huge count with truncated
data
+ // fails with a bounded AvroException instead of attempting a multi-
+ // gigabyte allocation.
+ [Test]
+ public void TestReadArrayHugeCountOnStreamClampsPreallocation()
+ {
+ var schema =
Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"long\"}");
+ var backing = new MemoryStream();
+ new BinaryEncoder(backing).WriteLong(200_000_000); // block count;
no element data
+ byte[] encoded = backing.ToArray();
+ using (var ns = new NonSeekableStream(new MemoryStream(encoded)))
+ {
+ var reader = new GenericReader<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ns)));
+ }
+ }
+
+ // The skip path (a writer field absent from the reader schema) must be
+ // bounded too, so skipping a huge zero-byte block cannot loop
endlessly.
+ [Test]
+ public void TestSkipArrayOfNullRejectsHugeCount()
+ {
+ var writer = Avro.Schema.Parse(
+ "{\"type\":\"record\",\"name\":\"Foo\",\"fields\":[" +
+
"{\"name\":\"arr\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}," +
+ "{\"name\":\"val\",\"type\":\"int\"}]}");
+ var reader = Avro.Schema.Parse(
+ "{\"type\":\"record\",\"name\":\"Foo\",\"fields\":[" +
+ "{\"name\":\"val\",\"type\":\"int\"}]}");
+ var ms = new MemoryStream();
+ new BinaryEncoder(ms).WriteLong(200_000_000); // arr block count;
skipped
Review Comment:
As with the array<null> decode test, declaring a 200,000,000-element block
here can hang or OOM the test process if the skip-path size guard regresses,
because skipping null elements consumes no input and the loop could iterate
toward that count. Prefer a count just above the default cap (10,000,000) to
keep the test's failure mode bounded while still verifying rejection.
--
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]
