Skip to content

[core] Validate record counts read from serialized bytes - #9985

Merged
JingsongLi merged 1 commit into
apache:masterfrom
jackylee-ch:validate-deserialized-list-count
Sep 20, 2026
Merged

JingsongLi merged 1 commit into
apache:masterfrom
jackylee-ch:validate-deserialized-list-count

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

Purpose

deserializeList pre-sized its ArrayList from a count read straight off the wire, so a corrupt count failed inside the JDK, not at the read: FF FF FF FF gives Illegal Capacity: -1, and 7F FF FF FF an OutOfMemoryError before record 0 is read.

SerializationUtils.readCount rejects a negative count with an IOException naming the serializer, next to the existing checkVersion, and presizedCapacity bounds the initial capacity, as PrimaryKeyIndexSourceMeta does. A count the remaining bytes cannot cover still fails on the read.

The same two lines are copied throughout paimon-core, so all 20 such counts now share the helper. Five more sit in four files that #9402 and #9949 are rewriting and are left for a follow-up. Raw arrays sized this way, in deserializedBytes and BinaryRowSerializer.deserialize, are a separate and larger hazard, not addressed here.

Tests

The three list checks are in ObjectSerializerTestBase, so its seven subclasses all run them. Reverting the two lines fails 14 of the 46, seven with each of:

java.lang.IllegalArgumentException: Illegal Capacity: -1
java.lang.OutOfMemoryError: Requested array size exceeds VM limit

paimon-core: 5630 run, 0 failures.

Written with Claude Code; verification is mine.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requirement fit: SUPPORTED (triage: GO)

Implementation: CLEAN

The patch consistently rejects negative serialized collection counts and caps only the initial allocation, so valid large payloads remain readable without trusting an unbounded count for eager allocation. I checked the serializer call sites, manifest compatibility fallback, and the new boundary tests. I did not find an actionable format-compatibility or correctness regression, and CI is green.

@JingsongLi
JingsongLi merged commit 82702c9 into apache:master Sep 20, 2026
18 checks passed
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