Skip to content

Fix builder forward references resolved within typed arrays - #6226

Open
jakezwang wants to merge 4 commits into
FasterXML:3.2from
jakezwang:fix/builder-array-forward-reference
Open

jakezwang wants to merge 4 commits into
FasterXML:3.2from
jakezwang:fix/builder-array-forward-reference

Conversation

@jakezwang

Copy link
Copy Markdown

Fixes #6225.

When a later element resolves a forward Object Id reference in the same typed array, the builder-to-value rebind occurs before the array is allocated. Update the accumulator in that case too, so the final array receives the built value rather than the transient builder.

Adds regressions for a single forward reference and repeated references to multiple ids, including a null element. Both new tests fail with ArrayStoreException before the fix.

Validation: full Maven verify on JDK17 and21 (6,189 tests each, one skipped), plus the JDK17 Android API34 compatibility check. JDK25 and other operating systems were not run locally.

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.86% 📈 +0.010%
Branches branches 75.51% 📈 +0.030%

Coverage data generated from JaCoCo test results

}

@Test
public void forwardReferenceResolvedWithinTypedArrayWithBuilder() throws Exception

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When adding to pre existing test class, we courage tests to contain some information about original GitHub issue.

@JooHyukKim

Copy link
Copy Markdown
Member

@jakezwang thank u for ur contribution!

@cowtowncoder cowtowncoder added the cla-needed PR looks good (although may also require code review), but CLA needed from submitter label Sep 24, 2026
@cowtowncoder

Copy link
Copy Markdown
Member

Need CLA unless already sent, see: https://github.com/FasterXML/jackson/blob/main/CONTRIBUTING.md#paperwork

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.85% 📈 +0.000%
Branches branches 75.52% 📈 +0.000%

Coverage data generated from JaCoCo test results

@gitar-bot

gitar-bot Bot commented Sep 26, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

🟡 Medium risk · Updates pre-allocation typed-array rebinding so resolved builder references populate final slots.

Fixes builder forward references resolved within typed arrays by updating the accumulator when a later element resolves a forward Object Id reference, ensuring the final array receives the built value rather than the transient builder. Comprehensive regression tests added and validated across JDK 17 and 21. No issues found.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.85% 📈 +0.000%
Branches branches 75.51% 📉 -0.010%

Coverage data generated from JaCoCo test results

@github-actions

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 81.86% 📈 +0.010%
Branches branches 75.55% 📈 +0.030%

Coverage data generated from JaCoCo test results

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

cla-needed PR looks good (although may also require code review), but CLA needed from submitter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants