Visitar URL original
Validate string SerDe bounds before allocation by jaideeppyne · Pull Request #783 · apache/datasketches-java · GitHub
Skip to content

Validate string SerDe bounds before allocation - #783

Draft
jaideeppyne wants to merge 1 commit into
apache:mainfrom
jaideeppyne:fix/string-serde-input-validation
Draft

jaideeppyne wants to merge 1 commit into
apache:mainfrom
jaideeppyne:fix/string-serde-input-validation

Conversation

@jaideeppyne

Copy link
Copy Markdown
Contributor

Problem

The UTF-8 and UTF-16 string SerDes allocate arrays from serialized counts and length prefixes before checking that the input segment contains those bytes. Invalid input can therefore throw NegativeArraySizeException or OutOfMemoryError instead of the expected bounds exception.

Change

Check the minimum prefix space before allocating the output array, and validate each string's payload before allocating its byte buffer. Valid serialized bytes and zero-item behavior are unchanged.

The new tests exercise both SerDes with impossible item counts, negative and oversized length prefixes, truncated prefixes, invalid offsets, and offset Unicode/empty-string round trips. They also verify that decoded strings remain independent of later input mutations.

Verification

Tests-first and full verification on JDK 25:

  • Unchanged production: the new tests reproduce the wrong NegativeArraySizeException and OutOfMemoryError paths.
  • Fixed source: all 10 focused cases pass.
  • mvn -B clean verify: 2,305 tests passed, no failures, errors, or skips.

Source: 628ac813eb143c287ed9da5594e74f5222fc29be. No dependencies or serialization formats changed.

Prepared with OpenAI Codex assistance, including implementation and tests.

@leerho
leerho marked this pull request as draft October 29, 2026 20:41
@leerho

leerho commented Oct 29, 2026

Copy link
Copy Markdown
Member

Thanks for this, @jaideeppyne — the fix and tests look good. We’re holding it briefly so we can merge it together with related work going into the 9.1.0 release, and we’ve marked it as a draft so it isn’t merged early. No changes are needed from you. We’ll follow up when it’s ready to merge.

@leerho

leerho commented Oct 29, 2026

Copy link
Copy Markdown
Member

Note to Reviewers: Please go ahead and review this PR even though it is in Draft. See my note to the author above.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants