From 94ac5ee5a853f1cd5b26a8dca25190032bf54fd3 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 4 Nov 2025 09:39:01 +0000 Subject: [PATCH 1/2] Fix GH-11384: Resolve property before model reflection to avoid recursion When a POM contains ${project.url} in the field and also defines a property named 'project.url', Maven was incorrectly attempting to resolve the expression via model reflection first, which would return the same ${project.url} value, causing a recursive variable reference error. This fix changes the interpolation resolution order to check model properties before prefixed model reflection. This allows properties like 'project.url' to be resolved from the section before attempting to use reflection on the model object. The new resolution order is: 1. basedir 2. build.timestamp 3. user properties 4. model properties (moved up from position 5) 5. prefixed model reflection (moved down from position 3) 6. system properties 7. environment variables 8. unprefixed model reflection This change is consistent with the approach used for MNG-8469, which established that explicit property definitions should take precedence over model field reflection. Fixes #11384 --- .../impl/model/DefaultModelInterpolator.java | 24 +++++---- .../model/DefaultModelInterpolatorTest.java | 20 +++++++ ...gh11384RecursiveVariableReferenceTest.java | 54 +++++++++++++++++++ .../src/test/resources/gh-11384/pom.xml | 18 +++++++ 4 files changed, 105 insertions(+), 11 deletions(-) create mode 100644 its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java create mode 100644 its/core-it-suite/src/test/resources/gh-11384/pom.xml diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java index 6194c1289f61..2e8c7b1fe84a 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelInterpolator.java @@ -179,22 +179,24 @@ String doCallback( return new MavenBuildTimestamp(request.getSession().getStartTime(), model.getProperties()) .formattedTimestamp(); } - // prefixed model reflection - for (String prefix : getProjectPrefixes(request)) { - if (expression.startsWith(prefix)) { - String subExpr = expression.substring(prefix.length()); - String v = projectProperty(model, projectDir, subExpr, true); - if (v != null) { - return v; - } - } - } // user properties String value = request.getUserProperties().get(expression); - // model properties + // model properties (check before prefixed model reflection to avoid recursion) if (value == null) { value = model.getProperties().get(expression); } + // prefixed model reflection + if (value == null) { + for (String prefix : getProjectPrefixes(request)) { + if (expression.startsWith(prefix)) { + String subExpr = expression.substring(prefix.length()); + value = projectProperty(model, projectDir, subExpr, true); + if (value != null) { + return value; + } + } + } + } // system properties if (value == null) { value = request.getSystemProperties().get(expression); diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelInterpolatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelInterpolatorTest.java index 7afe7a82e2de..065bc79ecd5e 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelInterpolatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelInterpolatorTest.java @@ -575,6 +575,26 @@ void shouldIgnorePropertiesWithPomPrefix() throws Exception { assertEquals(uninterpolatedName, out.getName()); } + @Test + void testProjectUrlPropertyDoesNotCauseRecursion() throws Exception { + // GH-11384: ${project.url} should resolve to the property "project.url" before + // trying to resolve via model reflection, which would cause recursion + Map modelProperties = new HashMap<>(); + modelProperties.put("project.url", "https://github.com/slackapi/java-slack-sdk"); + + Model model = Model.newBuilder() + .url("${project.url}") + .properties(modelProperties) + .build(); + + SimpleProblemCollector collector = new SimpleProblemCollector(); + Model out = interpolator.interpolateModel( + model, null, createModelBuildingRequest(context).build(), collector); + + assertProblemFree(collector); + assertEquals("https://github.com/slackapi/java-slack-sdk", out.getUrl()); + } + @Provides @Priority(10) @SuppressWarnings("unused") diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java new file mode 100644 index 000000000000..d33e79f1391a --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.it; + +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; + +/** + * This is a test set for GH-11384. + * + * Verifies that ${project.url} can refer to a property named "project.url" without causing + * a recursive variable reference error. This pattern is used by slack-sdk-parent. + */ +class MavenITgh11384RecursiveVariableReferenceTest extends AbstractMavenIntegrationTestCase { + + MavenITgh11384RecursiveVariableReferenceTest() { + super("[4.0.0-rc-4,)"); + } + + /** + * Verify that ${project.url} in the url field can reference a property named project.url + * without causing a recursive variable reference error. + */ + @Test + void testIt() throws Exception { + Path basedir = extractResources("/gh-11384").getAbsoluteFile().toPath(); + + Verifier verifier = newVerifier(basedir.toString()); + verifier.addCliArgument("help:effective-pom"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + + // Verify that the URL was correctly interpolated from the property + verifier.verifyTextInLog("https://github.com/slackapi/java-slack-sdk"); + } +} + diff --git a/its/core-it-suite/src/test/resources/gh-11384/pom.xml b/its/core-it-suite/src/test/resources/gh-11384/pom.xml new file mode 100644 index 000000000000..0c23b6b95c84 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-11384/pom.xml @@ -0,0 +1,18 @@ + + + org.apache.maven.its.mng11384 + test + 1.0 + pom + + Maven Integration Test :: GH-11384 + Test that project.url can refer to a property named project.url without causing recursion + + + ${project.url} + + + https://github.com/slackapi/java-slack-sdk + + + From e34b0637582869bef87ab8be9faabce6c0a1e584 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 4 Nov 2025 10:12:24 +0000 Subject: [PATCH 2/2] Fix integration test constructor Remove incorrect constructor with version parameter. AbstractMavenIntegrationTestCase uses a no-argument constructor and version constraints are handled differently in the new test framework. --- .../it/MavenITgh11384RecursiveVariableReferenceTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java index d33e79f1391a..5c60d9082091 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11384RecursiveVariableReferenceTest.java @@ -27,13 +27,11 @@ * * Verifies that ${project.url} can refer to a property named "project.url" without causing * a recursive variable reference error. This pattern is used by slack-sdk-parent. + * + * @since 4.0.0-rc-4 */ class MavenITgh11384RecursiveVariableReferenceTest extends AbstractMavenIntegrationTestCase { - MavenITgh11384RecursiveVariableReferenceTest() { - super("[4.0.0-rc-4,)"); - } - /** * Verify that ${project.url} in the url field can reference a property named project.url * without causing a recursive variable reference error.