AVRO-4279: [c] Bound collection size when decoding arrays and maps by iemejia · Pull Request #3848 · apache/avro

iemejia · 2026-07-11T12:14:34Z

What is the purpose of the change

When decoding an array or map, the block count is read from the input and drives
allocation of the resulting collection. A very large or malformed block count
(for example from truncated or hostile input) could request an unbounded
allocation, since items such as null occupy no bytes on the wire.

read_array_value() / read_map_value() now validate the block count — per
block and cumulatively — against a configurable maximum before allocating,
mirroring the Java SDK's collection item limit
(org.apache.avro.limits.collectionItems.maxLength, AVRO-3819). The limit
defaults to 2^31 - 8 items and can be overridden with the
AVRO_MAX_COLLECTION_ITEMS environment variable. Exceeding it fails decoding
with EINVAL and a descriptive error.

This is part of the umbrella issue AVRO-4277.

Verifying this change

This change added tests and can be verified as follows:

Added lang/c/tests/test_avro_4279.c (registered in tests/CMakeLists.txt)
covering: a single block count above the limit (array and map), a cumulative
limit across blocks, a negative block count bounded by its absolute value, and
a within-limit collection still decoding correctly.
Run: ctest -R test_avro_4279 (also passes under the valgrind memcheck variant)

Documentation

Does this pull request introduce a new feature? (no — hardening/robustness)
If yes, how is the feature documented? (not applicable; the new
AVRO_MAX_COLLECTION_ITEMS environment variable is documented in code comments)

The block count of an array or map is read from the input and drives allocation of the resulting collection. A very large or malformed block count (for example from truncated input) could request an unbounded allocation. Validate the block count per block and cumulatively against a configurable maximum (AVRO_MAX_COLLECTION_ITEMS), mirroring the Java SDK's collection item limit, and fail decoding with EINVAL when exceeded. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

This PR hardens the C decoder against unbounded allocations when decoding arrays and maps by enforcing a configurable maximum number of collection items derived from the encoded block counts (including cumulative counts across blocks).

Changes:

Added a configurable max collection item limit (defaulting near 2^31) and enforced it during array/map decoding before allocating/appending items.
Introduced a new C regression test (test_avro_4279.c) to validate rejection/acceptance behavior around the configured limit.
Registered the new test in the C test CMake configuration.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
lang/c/src/value-read.c	Adds max-collection-item limit logic and applies it in array/map decoding loops to prevent unbounded allocation.
lang/c/tests/test_avro_4279.c	Adds tests covering over-limit, cumulative-limit, negative-count, and within-limit decoding behavior.
lang/c/tests/CMakeLists.txt	Registers the new AVRO-4279 test with the test suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

iemejia · 2026-07-11T13:35:09Z

+ * with the AVRO_MAX_COLLECTION_ITEMS environment variable.
+ */
+
+#define AVRO_DEFAULT_MAX_COLLECTION_ITEMS  ((int64_t) 2147483639)  /* 2^31 - 8 */


The value 2147483639 is intentional — it matches the Java SDK's MAX_ARRAY_VM_LIMIT = Integer.MAX_VALUE - 8. Fixed the misleading comment in aa1d9eb (it now reads Integer.MAX_VALUE - 8 instead of 2^31 - 8).

iemejia · 2026-07-11T13:35:11Z

 				     read_long(reader, &block_size),
 				     "Cannot read array block size: ");
 		}

+		check(rval, avro_check_collection_items(index, block_count));


Fixed in aa1d9eb. Added an explicit INT64_MIN guard before negating in read_array_value (returns EINVAL), so hostile input can no longer trigger signed-overflow UB. Added a test that feeds an INT64_MIN block count (zigzag of 2^64-1).

iemejia · 2026-07-11T13:35:12Z

 				     read_long(reader, &block_size),
 				     "Cannot read map block size: ");
 		}

+		check(rval, avro_check_collection_items(index, block_count));


Fixed in aa1d9eb — the same INT64_MIN guard was added to read_map_value, covered by the same test (map case included).

- Reject a block count of INT64_MIN before negating it, avoiding signed integer overflow (undefined behavior) on hostile input, in both the array and map decoders. - Correct the default-limit comment: 2147483639 is Integer.MAX_VALUE - 8 (matching the Java SDK), not 2^31 - 8. - Add a test feeding an INT64_MIN block count. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

iemejia · 2026-07-11T14:16:20Z

 #include <avro/platform.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>



Fixed in d978884 — <errno.h> (and <limits.h>) are now included explicitly, and avro_max_collection_items checks errno.

iemejia · 2026-07-11T14:16:21Z

+	avro_value_t value;
+	avro_generic_value_new(iface, &value);
+	avro_reader_t reader = avro_reader_memory(bytes, (int64_t) len);
+	int rc = avro_value_read(reader, &value);
+	avro_reader_free(reader);
+	avro_value_decref(&value);
+	return rc;
+}


Fixed in d978884 — try_read now checks avro_generic_value_new and avro_reader_memory for failure instead of proceeding with a possibly-uninitialized value or NULL reader.

iemejia · 2026-07-11T14:16:23Z

+	/* A negative count (unsigned varint 0x15 -> -11) uses its absolute value
+	 * (11) as the item count and is followed by a block size (0x00). It must
+	 * still be bounded. */
+	{
+		const char bytes[] = { 0x15, 0x00 };
+		expect_rejected(array_iface, bytes, sizeof(bytes), "array negative count");
+	}


Fixed in d978884 — the negative-block-count rejection is now asserted for maps as well as arrays.

iemejia · 2026-07-11T14:16:24Z

+	const char *env = getenv("AVRO_MAX_COLLECTION_ITEMS");
+	if (env != NULL && *env != '\0') {
+		char *end = NULL;
+		long long value = strtoll(env, &end, 10);
+		if (end != NULL && *end == '\0' && value > 0) {
+			return (int64_t) value;
+		}
+	}
+	return AVRO_DEFAULT_MAX_COLLECTION_ITEMS;
+}


Fixed in d978884 — avro_max_collection_items resets errno before strtoll and rejects out-of-range values (ERANGE -> LLONG_MAX/MIN) as well as anything above INT64_MAX, falling back to the default.

- avro_max_collection_items now resets errno and rejects out-of-range values (strtoll returns LLONG_MAX/MIN with errno == ERANGE) and values above INT64_MAX; include <errno.h> and <limits.h> explicitly. - try_read now checks avro_generic_value_new and avro_reader_memory for failure instead of dereferencing a possibly-uninitialized value/NULL reader. - Assert the negative-block-count rejection for maps as well as arrays. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

iemejia · 2026-07-11T14:32:47Z

+		if (errno == 0 && end != NULL && *end == '\0' &&
+		    value > 0 && value <= (long long) INT64_MAX) {
+			return (int64_t) value;
+		}


Fixed in eb42895 — a configured limit is now clamped to SIZE_MAX so the (size_t) cast of the block count in the decode loops cannot truncate on 32-bit builds (no-op on 64-bit, where size_t >= int64_t).

iemejia · 2026-07-11T14:32:49Z

+	/* Two blocks of 6 items (zigzag(6) = 0x0c) exceed the limit cumulatively
+	 * even though neither block does on its own. */
+	{
+		const char bytes[] = { 0x0c, 0x0c };
+		expect_rejected(array_iface, bytes, sizeof(bytes), "array cumulative");
+	}


Fixed in eb42895 — added a cumulative-limit test for maps (two blocks of 6 pairs) mirroring the array case.

iemejia · 2026-07-11T14:32:50Z

+	/* Three items (zigzag(3) = 0x06) then the end-of-array marker (0x00) is
+	 * within the limit and must decode successfully. */
+	{
+		const char bytes[] = { 0x06, 0x00 };
+		expect_accepted(array_iface, bytes, sizeof(bytes), "array within limit");
+	}


Fixed in eb42895 — added a within-limit map decode assertion (three pairs with distinct keys) to confirm valid maps under the limit are not rejected.

- Clamp a configured AVRO_MAX_COLLECTION_ITEMS to SIZE_MAX so the (size_t) cast of the block count in the decode loops cannot truncate on 32-bit platforms (no-op on 64-bit). - Add cumulative-limit and within-limit test cases for maps, mirroring the array cases, so regressions in the map path are caught. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

github-actions Bot added the C label Jul 11, 2026

iemejia requested a review from Copilot July 11, 2026 12:32

Copilot started reviewing on behalf of iemejia July 11, 2026 12:33 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

iemejia requested a review from Copilot July 11, 2026 13:46

Copilot started reviewing on behalf of iemejia July 11, 2026 13:46 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

iemejia requested a review from Copilot July 11, 2026 14:19

Copilot started reviewing on behalf of iemejia July 11, 2026 14:20 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

iemejia requested a review from Copilot July 11, 2026 14:36

Copilot started reviewing on behalf of iemejia July 11, 2026 14:36 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

Uh oh!

Conversation

iemejia commented Jul 11, 2026

What is the purpose of the change

Verifying this change

Documentation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants