Skip to content

Fix NPE on a recursive property reference in a list field - #13099

Open
renechoi wants to merge 1 commit into
apache:masterfrom
renechoi:fix/model-interpolator-null-on-cycle
Open

Fix NPE on a recursive property reference in a list field#13099
renechoi wants to merge 1 commit into
apache:masterfrom
renechoi:fix/model-interpolator-null-on-cycle

Conversation

@renechoi

Copy link
Copy Markdown

On a recursive reference DefaultModelInterpolator records an ERROR and returns null. The transformer writes it into List<String> fields such as build/filters (its Properties branch already guards), and the immutable constructor calls List.copyOf, so the build dies with a bare NPE that hides the collected error:

java.lang.NullPointerException
 at java.util.List.copyOf(List.java:1193)
 at o.a.m.model.v4.MavenTransformer.transformBuild(:514)
 at o.a.m.impl.model.DefaultModelInterpolator.interpolateModel(:111)

Returning the original text fixes it at the source instead of in the generated template, and matches DecryptingSettingsTransformer and MNG-8174 (#12932), which covered Maven 3 only.

The new test fails on master with that NPE. Full mvn test: 3136 tests, 0 failures. Can open a maven-4.0.x backport.

DefaultModelInterpolator records an ERROR and returns null when the
interpolator throws on a recursive variable reference. The generated
transformer stores that null in List<String> model fields such as
build/filters, whose Properties sibling branch already guards against
it, and the immutable model constructor then copies the list with
List.copyOf. The build fails with a bare NullPointerException instead
of the "recursive variable reference" error that was just collected.

Return the original text instead, which is what the Properties branch
and StringVisitorModelInterpolator (MNG-8174) already do. The returned
instance is identical, so the transformer's newVal != oldVal check
stays false and unaffected models are still returned unchanged.

@gnodet-bot gnodet-bot 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.

Clean, minimal fix. Returning the original text on an InterpolatorException instead of null is the correct approach — the error is already collected in the problem collector, and the build will fail with a clear "recursive variable reference" message. Returning null caused a secondary NullPointerException in List.copyOf() that masked the real error.

Consistent with DecryptingSettingsTransformer and the Maven 3 fix (MNG-8174 / #12932).

Test covers the exact failure path: recursive properties in a List<String> model field (build/filters).

📋 PR Metadata

Aspect Current Suggested
Category (unlabeled) bug
Labels (none) + bug
Milestone (none) 4.1.0

🔀 Backport Status

⚠️ This bug fix targets master but no backport PRs were found for:

  • maven-4.0.x — no backport PR found (author mentions willingness to open one)

This review was generated by an AI agent, Hermès on behalf of @gnodet.

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