iemejia commented on code in PR #3858:
URL: https://github.com/apache/avro/pull/3858#discussion_r3566941708
##########
lang/c/src/value-read.c:
##########
@@ -38,6 +40,158 @@ static int
read_value(avro_reader_t reader, avro_value_t *dest);
+/*
+ * Minimum number of bytes a single value of the given schema can occupy on the
+ * wire. Used to reject an array/map block count that could not be backed by
the
+ * bytes remaining. A type that can encode to zero bytes (null) returns 0,
which
+ * disables the collection check for it (so an array of nulls is not falsely
+ * rejected). A recursion guard breaks self-referencing schemas.
+ */
+static int64_t
+min_bytes_per_element(avro_schema_t schema, int depth)
+{
+ if (schema == NULL) {
+ return 0;
+ }
+ switch (avro_typeof(schema)) {
+ case AVRO_NULL:
+ return 0;
+ case AVRO_FLOAT:
+ return 4;
+ case AVRO_DOUBLE:
+ return 8;
+ case AVRO_FIXED:
+ return avro_schema_fixed_size(schema);
+ case AVRO_RECORD: {
+ size_t n = avro_schema_record_size(schema);
+ int64_t total = 0;
+ size_t i;
+ if (depth > 64) {
+ /* A cyclic or pathologically deep record. Return 1 (not
+ * 0) so the collection check stays enabled; a valid
+ * recursive value always encodes to >= 1 byte. The
depth
+ * guard is applied only here, so zero-byte leaf types
+ * such as null still return 0 regardless of depth. */
+ return 1;
+ }
Review Comment:
Fixed in 9b626d9dd9: the depth guard now returns 0 (a conservative lower
bound) instead of 1, so a deeply-nested but zero-byte record isn't
misclassified as non-zero-byte; the tighter zero-byte cap is applied when the
true minimum can't be computed.
##########
lang/c/src/value-read.c:
##########
@@ -38,6 +40,158 @@ static int
read_value(avro_reader_t reader, avro_value_t *dest);
+/*
+ * Minimum number of bytes a single value of the given schema can occupy on the
+ * wire. Used to reject an array/map block count that could not be backed by
the
+ * bytes remaining. A type that can encode to zero bytes (null) returns 0,
which
+ * disables the collection check for it (so an array of nulls is not falsely
+ * rejected). A recursion guard breaks self-referencing schemas.
+ */
+static int64_t
+min_bytes_per_element(avro_schema_t schema, int depth)
+{
+ if (schema == NULL) {
+ return 0;
+ }
+ switch (avro_typeof(schema)) {
+ case AVRO_NULL:
+ return 0;
+ case AVRO_FLOAT:
+ return 4;
+ case AVRO_DOUBLE:
+ return 8;
+ case AVRO_FIXED:
+ return avro_schema_fixed_size(schema);
+ case AVRO_RECORD: {
+ size_t n = avro_schema_record_size(schema);
+ int64_t total = 0;
+ size_t i;
+ if (depth > 64) {
+ /* A cyclic or pathologically deep record. Return 1 (not
+ * 0) so the collection check stays enabled; a valid
+ * recursive value always encodes to >= 1 byte. The
depth
+ * guard is applied only here, so zero-byte leaf types
+ * such as null still return 0 regardless of depth. */
+ return 1;
+ }
+ for (i = 0; i < n; i++) {
+ avro_schema_t field =
+ avro_schema_record_field_get_by_index(schema, i);
+ int64_t field_min = min_bytes_per_element(field, depth
+ 1);
+ /* Saturate rather than overflow: a wrapped (negative)
+ * total would disable the collection check. */
+ if (field_min > INT64_MAX - total) {
+ return INT64_MAX;
+ }
+ total += field_min;
+ }
+ return total;
+ }
+ case AVRO_LINK:
+ return min_bytes_per_element(avro_schema_link_target(schema),
+ depth + 1);
+ default:
+ /* boolean, int, long, bytes, string, enum, union, array, map:
+ * all encode to at least one byte. */
+ return 1;
+ }
+}
+
+/* Non-static wrapper so the skip path (datum_skip.c) can compute the minimum
+ * on-wire size of a collection's element schema. */
+int64_t
+avro_min_bytes_per_element(avro_schema_t schema)
+{
+ return min_bytes_per_element(schema, 0);
+}
+
+/*
+ * Maximum number of zero-byte-encoded collection elements (e.g. an array of
+ * nulls) to allocate from a single decode. Such elements consume no input, so
+ * the bytes-remaining check cannot bound their count; without a cap a tiny
+ * payload can declare a huge block count and exhaust memory. Overridable via
+ * the AVRO_MAX_COLLECTION_ITEMS environment variable.
+ */
+#define AVRO_DEFAULT_MAX_COLLECTION_ITEMS ((int64_t) 10000000)
+
+/*
+ * Structural cap on the number of elements in any array or map (an overflow /
+ * defense-in-depth guard), matching the historical Integer.MAX_VALUE - 8
limit.
+ * Non-zero-byte elements are also bounded by the bytes remaining.
+ */
+#define AVRO_DEFAULT_MAX_COLLECTION_STRUCTURAL ((int64_t) 2147483639)
+
+/*
+ * Populate the zero-byte and structural collection limits. The
+ * AVRO_MAX_COLLECTION_ITEMS environment variable, when a non-negative integer,
+ * caps both; otherwise the tighter zero-byte default and the structural
default
+ * are used. Shared with the skip path (see avro_collection_limits).
+ */
+void
+avro_collection_limits(int64_t *zero_byte, int64_t *structural)
+{
+ const char *env = getenv("AVRO_MAX_COLLECTION_ITEMS");
+ if (env != NULL && *env != '\0') {
+ char *end;
+ long long value = strtoll(env, &end, 10);
+ if (*end == '\0' && value >= 0) {
+ *zero_byte = *structural = (int64_t) value;
+ return;
+ }
+ }
+ *zero_byte = AVRO_DEFAULT_MAX_COLLECTION_ITEMS;
+ *structural = AVRO_DEFAULT_MAX_COLLECTION_STRUCTURAL;
+}
Review Comment:
Fixed in 9b626d9dd9: avro_collection_limits() now clamps the parsed
AVRO_MAX_COLLECTION_ITEMS to INT64_MAX and SIZE_MAX, so a configured limit
above SIZE_MAX can't permit a block count that truncates when cast to size_t in
the read loops (notably on 32-bit).
--
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]
