Skip to content

Fix #6398: honor null recovery for array-shaped builders - #6406

Merged
cowtowncoder merged 6 commits into
FasterXML:3.xfrom
wozhenbang2004:fix-6398-array-builder-null-recovery
Nov 7, 2026
Merged

cowtowncoder merged 6 commits into
FasterXML:3.xfrom
wozhenbang2004:fix-6398-array-builder-null-recovery

Conversation

@wozhenbang2004

@wozhenbang2004 wozhenbang2004 commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6398.

An array-shaped builder can reach finishBuild() with a null builder after a DeserializationProblemHandler recovers. This happens both when object input is handled as an unexpected token and when array input has no usable builder creator. Calling the build method on that null receiver throws.

Check for null in finishBuild() so all callers preserve the handler result. Add regressions for both recovery paths and include the 3.3.0 entries in VERSION and CREDITS.

The missing-instantiator regression reproduces the reported failure on the prior PR head. Maven clean verify on JDK 21 passed with 7,774 tests passed and 1 skipped.

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 83.10% 📈 +0.010%
Branches branches 77.28% 📉 -0.010%

Coverage data generated from JaCoCo test results

@cowtowncoder cowtowncoder added the cla-received PR already covered by CLA (optional label) label Nov 5, 2026
@cowtowncoder

Copy link
Copy Markdown
Member

As usual, needs release notes (I'd add but I don't have push access to this pr).

@cowtowncoder

cowtowncoder commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

(comment generated by Claude Code)

The same NPE is still reachable through the array path, because the null check only covers the non-array branch. Line 137 passes the result of _deserializeNonVanilla() straight to finishBuild():

if (!_useVanillaProcessing(ctxt)) {
    return finishBuild(ctxt, _deserializeNonVanilla(p, ctxt));
}

With no usable Creator on the builder, _deserializeNonVanilla() goes through deserializeFromObjectUsingNonDefault() to handleMissingInstantiator(), and a handler may return null from that.

To reproduce, on this PR's head (dd89e0e):

@JsonDeserialize(builder = Value.Builder.class)
@JsonFormat(shape = JsonFormat.Shape.ARRAY)
public static class Value {
    public int x;

    @JsonPropertyOrder({ "x" })
    @JsonFormat(shape = JsonFormat.Shape.ARRAY)
    public static class Builder {
        int x;

        public Builder(int a, int b) { } // no default, delegating or properties-based Creator
        public Builder withX(int v) { x = v; return this; }
        public Value build() {
            Value r = new Value();
            r.x = x;
            return r;
        }
    }
}

ObjectMapper mapper = JsonMapper.builder()
        .addHandler(new DeserializationProblemHandler() {
            @Override
            public Object handleMissingInstantiator(DeserializationContext ctxt, Class<?> instClass,
                    ValueInstantiator valueInsta, JsonParser p, String msg) {
                p.skipChildren();
                return null;
            }
        })
        .build();

mapper.readValue("[1,2]", Value.class);

Actual result:

ValueInstantiationException: Cannot construct instance of `Value$Builder`, problem: `java.lang.NullPointerException`

Expected result: null.

Could you move the null check into finishBuild() so that every call site is covered, and add a test for this case? For example:

protected final Object finishBuild(DeserializationContext ctxt, Object builder)
    throws JacksonException
{
    if (builder == null) {
        return null;
    }
    try {
        return _buildMethod.getMember().invoke(builder, (Object[]) null);
    } catch (Exception e) {
        return wrapInstantiationProblem(ctxt, e);
    }
}

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 83.12% 📈 +0.020%
Branches branches 77.31% 📉 -0.020%

Coverage data generated from JaCoCo test results

@gitar-bot

gitar-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

🟡 Medium risk · Returning a handler-provided null before invoking an array builder’s build method changes deserialization results; callers may receive null instead of a built value.

Preserves a null result from DeserializationProblemHandler when an array-shaped builder receives an unexpected token, returning it directly instead of passing it to the build method. No issues found.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.

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

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 83.12% 📈 +0.020%
Branches branches 77.33% 📈 +0.000%

Coverage data generated from JaCoCo test results

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 83.12% 📈 +0.020%
Branches branches 77.31% 📉 -0.020%

Coverage data generated from JaCoCo test results

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

🧪 Code Coverage Report

Metric Coverage Change
Instructions coverage 83.12% 📈 +0.020%
Branches branches 77.33% 📈 +0.000%

Coverage data generated from JaCoCo test results

@cowtowncoder
cowtowncoder merged commit 3e05cc2 into FasterXML:3.x Nov 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-received PR already covered by CLA (optional label)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@JsonFormat(shape = ARRAY) builder: null from DeserializationProblemHandler causes NPE

2 participants