Skip to content

AVRO-4304: [java][python] Add shared must-reject binary interop vectors - #3932

Open
iemejia wants to merge 3 commits into
apache:mainfrom
iemejia:AVRO-4304-shared-reject-interop-vectors
Open

AVRO-4304: [java][python] Add shared must-reject binary interop vectors#3932
iemejia wants to merge 3 commits into
apache:mainfrom
iemejia:AVRO-4304-shared-reject-interop-vectors

Conversation

@iemejia

@iemejia iemejia commented Aug 7, 2026

Copy link
Copy Markdown
Member

What is the purpose of the change

Decoder hardening across the SDKs now rejects a range of malformed binary
encodings (overlong varints, Long.MIN_VALUE collection block counts,
out-of-range union branch and enum symbol indices, negative bytes/string
lengths), but there is no shared cross-language fixture guaranteeing that every
SDK rejects the same malformed inputs identically, so the SDKs can drift (one
accepts what another rejects).

This adds a shared set of must-reject vectors under
share/test/data/binary-rejections.json, each a schema plus a raw binary
payload (hex) that a conformant decoder must reject with a bounded,
well-defined error rather than accepting it, crashing, or exhausting memory.
It seeds the set with cases already fixed per SDK, and wires thin per-SDK
harnesses (Java and Python) that load the vectors and assert rejection.

The shared fixture is a single cross-language artifact, so it is kept together
with its first consumers here; additional SDK harnesses can be added later as
small changes on top of the merged fixture.

Verifying this change

This change added tests and can be verified as follows:

  • Added share/test/data/binary-rejections.json with 9 seed vectors.
  • Added the Java harness TestBinaryDecodingRejections, which asserts each
    vector is rejected on both the classic and fast reader paths.
  • Added the Python harness test_binary_decoding_rejections.py, which asserts
    each vector is rejected with a bounded AvroException.

Documentation

  • Does this pull request introduce a new feature? (no — shared conformance test
    fixtures)
  • If yes, how is the feature documented? (the fixture file carries a
    self-describing description field explaining the format and intent)

iemejia added 2 commits August 7, 2026 17:03
Decoder hardening across the SDKs now rejects a range of malformed binary
encodings, but there is no shared cross-language fixture guaranteeing that
every SDK rejects the same malformed inputs identically, so the SDKs can
drift (one accepts what another rejects).

Add a shared set of must-reject vectors under share/test, each a schema
plus a raw binary payload that a conformant decoder must reject with a
bounded, well-defined error rather than accepting it, crashing, or
exhausting memory. Seed it with cases already fixed per SDK: overlong
varints, Long.MIN_VALUE array block counts, negative bytes/string
lengths, and out-of-range union branch and enum symbol indices.

Add a Java harness that loads the vectors and asserts each is rejected on
both the classic and fast reader paths. Payloads are stored as hex and
the schema as a JSON string so the fixtures are language neutral and can
be wired into every SDK's test suite.
Wire the Python SDK into the shared cross-SDK must-reject binary decoding
fixtures added under share/test/data/binary-rejections.json. The harness
loads each vector (a schema plus a raw binary payload) and asserts the
Avro binary decoder rejects it with a bounded AvroException rather than
accepting it or crashing, guaranteeing the Python SDK rejects the same
malformed inputs as the other language SDKs and does not drift.
@github-actions github-actions Bot added Java Pull Requests for Java binding Python labels Aug 7, 2026
@iemejia
iemejia requested a lite review from Copilot August 7, 2026 16:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a shared, cross-language “must-reject” fixture of malformed Avro binary payloads and introduces thin Java/Python test harnesses to assert that decoders reject those payloads with bounded errors (preventing cross-SDK drift in decoder hardening behavior).

Changes:

  • Added share/test/data/binary-rejections.json containing seed malformed binary vectors (schema + hex payload + category).
  • Added a Java parameterized test harness that loads the shared vectors and asserts both classic and fast-reader paths reject them.
  • Added a Python unittest harness that locates the shared vectors in a source checkout and asserts the decoder rejects each payload with AvroException.

Reviewed changes

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

File Description
share/test/data/binary-rejections.json Introduces the shared cross-SDK reject-vector manifest (schema + hex payloads).
lang/py/avro/test/test_binary_decoding_rejections.py Python harness that loads the shared vectors and asserts bounded decoder rejection.
lang/java/avro/src/test/java/org/apache/avro/TestBinaryDecodingRejections.java Java harness that loads the shared vectors and asserts rejection on classic + fast reader paths.

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

Comment thread lang/java/avro/src/test/java/org/apache/avro/TestBinaryDecodingRejections.java Outdated
Address review feedback on the Java reject-vector harness:

- fromHex now rejects odd-length hex strings and invalid hex characters
  instead of silently truncating, so a malformed fixture fails loudly
  rather than decoding a different payload than intended.
- Drop the try/catch that re-threw the assertion via fail(getMessage()),
  which discarded the original AssertionError (and any Error) context.
  The assertThrows failure now propagates directly with full detail.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

lang/java/avro/src/test/java/org/apache/avro/TestBinaryDecodingRejections.java:114

  • Schema.Parser().parse(schemaJson) happens inside decode() which is executed inside assertThrows(Exception.class, ...). This means an invalid fixture schema (SchemaParseException) would still satisfy the assertion and be reported as a “pass”, even though decoding was never exercised. Also, assertThrows(Exception.class, …) accepts any Exception (e.g. NullPointerException), which weakens the “bounded, well-defined Avro/IO error” intent of the harness.
  private static void decode(String schemaJson, byte[] bytes, boolean fastReader) throws IOException {
    Schema schema = new Schema.Parser().parse(schemaJson);
    GenericData data = new GenericData();
    data.setFastReaderEnabled(fastReader);
    GenericDatumReader<Object> reader = new GenericDatumReader<>(schema, schema, data);

lang/py/avro/test/test_binary_decoding_rejections.py:63

  • The fixture is read with manifest.read_text() without an explicit encoding, which can break on systems whose default locale encoding is not UTF-8. Also, schema/hex parsing happens before subTest, so a single malformed vector would fail the whole test without identifying the offending vector in the test output.
        manifest = _find_manifest()
        vectors = json.loads(manifest.read_text())["vectors"]
        self.assertTrue(vectors, "no reject vectors found")
        for vector in vectors:
            name = vector["name"]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java Pull Requests for Java binding Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants