This is an automated email from the ASF dual-hosted git repository. gnodet pushed a commit to branch fix-12301 in repository https://gitbox.apache.org/repos/asf/maven.git
commit 865bbcd28231f0a6c1b589724aacf85bdc14b664 Author: Guillaume Nodet <[email protected]> AuthorDate: Thu Jun 18 13:34:28 2026 +0000 [#12301] Fix StackOverflowError with internal parent and CI-friendly revision Co-Authored-By: Claude Opus 4.6 <[email protected]> --- .../maven/impl/model/DefaultModelBuilder.java | 375 +++++++++++---------- ...301StackOverflowInternalParentRevisionTest.java | 48 +++ .../.mvn/.gitkeep | 0 .../parent/pom.xml | 29 ++ .../gh-12301-stackoverflow-internal-parent/pom.xml | 38 +++ 5 files changed, 314 insertions(+), 176 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java index 481730dd5c..4898b82be7 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java @@ -276,6 +276,11 @@ List<RemoteRepository> getExternalRepositories() { // Contains both GAV coordinates (groupId:artifactId:version) and file paths final Set<String> parentChain; + // Tracks file paths currently being read by doReadFileModel, shared across + // all derived sessions to prevent recursive loops between parent resolution + // and enhanced property resolution (GH-12301) + final Set<Path> activeModelReads; + ModelBuilderSessionState(ModelBuilderRequest request) { this( request.getSession(), @@ -286,7 +291,8 @@ List<RemoteRepository> getExternalRepositories() { List.of(), repos(request), repos(request), - new LinkedHashSet<>()); + new LinkedHashSet<>(), + ConcurrentHashMap.newKeySet()); } static List<RemoteRepository> repos(ModelBuilderRequest request) { @@ -306,7 +312,8 @@ private ModelBuilderSessionState( List<RemoteRepository> pomRepositories, List<RemoteRepository> externalRepositories, List<RemoteRepository> repositories, - Set<String> parentChain) { + Set<String> parentChain, + Set<Path> activeModelReads) { this.session = session; this.request = request; this.result = result; @@ -316,6 +323,7 @@ private ModelBuilderSessionState( this.externalRepositories = externalRepositories; this.repositories = repositories; this.parentChain = parentChain; + this.activeModelReads = activeModelReads; this.result.setSource(this.request.getSource()); } @@ -366,7 +374,8 @@ ModelBuilderSessionState derive(ModelBuilderRequest request, DefaultModelBuilder pomRepositories, derivedExtRepos, derivedRepos, - new LinkedHashSet<>()); + new LinkedHashSet<>(), + activeModelReads); } @Override @@ -699,8 +708,12 @@ private Map<String, String> getEnhancedProperties(Model model, Path rootDirector if (rootModelPath != null) { // Check if the root model path is within the root directory to prevent infinite loops // This can happen when a .mvn directory exists in a subdirectory and parent inference - // tries to read models above the discovered root directory - if (isParentWithinRootDirectory(rootModelPath, rootDirectory)) { + // tries to read models above the discovered root directory. + // Also skip if the root model is already being read in an outer call frame + // to prevent StackOverflowError when a project has an internal parent in a + // subdirectory with CI-friendly ${revision} and a .mvn/ root marker (GH-12301). + if (isParentWithinRootDirectory(rootModelPath, rootDirectory) + && !activeModelReads.contains(rootModelPath.normalize())) { Model rootModel = derive(Sources.buildSource(rootModelPath)).readFileModel(); properties.putAll(getPropertiesWithProfiles(rootModel, properties)); @@ -1495,213 +1508,223 @@ Model doReadFileModel() throws ModelBuilderException { boolean rootDirectoryFromSession = false; setSource(modelSource.getLocation()); logger.debug("Reading file model from " + modelSource.getLocation()); + Path sourcePath = modelSource.getPath(); + boolean trackRead = sourcePath != null && activeModelReads.add(sourcePath.normalize()); try { - boolean strict = isBuildRequest(); try { - rootDirectory = request.getSession().getRootDirectory(); - rootDirectoryFromSession = true; - } catch (IllegalStateException ignore) { - rootDirectory = modelSource.getPath(); - while (rootDirectory != null && !Files.isDirectory(rootDirectory)) { - rootDirectory = rootDirectory.getParent(); - } - } - try (InputStream is = modelSource.openStream()) { - model = modelProcessor.read(XmlReaderRequest.builder() - .strict(strict) - .location(modelSource.getLocation()) - .path(modelSource.getPath()) - .rootDirectory(rootDirectory) - .inputStream(is) - .transformer(new InterningTransformer(session)) - .build()); - } catch (XmlReaderException e) { - if (!strict) { - throw e; + boolean strict = isBuildRequest(); + try { + rootDirectory = request.getSession().getRootDirectory(); + rootDirectoryFromSession = true; + } catch (IllegalStateException ignore) { + rootDirectory = modelSource.getPath(); + while (rootDirectory != null && !Files.isDirectory(rootDirectory)) { + rootDirectory = rootDirectory.getParent(); + } } try (InputStream is = modelSource.openStream()) { model = modelProcessor.read(XmlReaderRequest.builder() - .strict(false) + .strict(strict) .location(modelSource.getLocation()) .path(modelSource.getPath()) .rootDirectory(rootDirectory) .inputStream(is) .transformer(new InterningTransformer(session)) .build()); - } catch (XmlReaderException ne) { - // still unreadable even in non-strict mode, rethrow original error - throw e; - } + } catch (XmlReaderException e) { + if (!strict) { + throw e; + } + try (InputStream is = modelSource.openStream()) { + model = modelProcessor.read(XmlReaderRequest.builder() + .strict(false) + .location(modelSource.getLocation()) + .path(modelSource.getPath()) + .rootDirectory(rootDirectory) + .inputStream(is) + .transformer(new InterningTransformer(session)) + .build()); + } catch (XmlReaderException ne) { + // still unreadable even in non-strict mode, rethrow original error + throw e; + } + add( + Severity.ERROR, + Version.V20, + "Malformed POM " + modelSource.getLocation() + ": " + e.getMessage(), + e); + } + } catch (XmlReaderException e) { add( - Severity.ERROR, - Version.V20, - "Malformed POM " + modelSource.getLocation() + ": " + e.getMessage(), + Severity.FATAL, + Version.BASE, + "Non-parseable POM " + modelSource.getLocation() + ": " + e.getMessage(), e); - } - } catch (XmlReaderException e) { - add( - Severity.FATAL, - Version.BASE, - "Non-parseable POM " + modelSource.getLocation() + ": " + e.getMessage(), - e); - throw newModelBuilderException(); - } catch (IOException e) { - String msg = e.getMessage(); - if (msg == null || msg.isEmpty()) { - // NOTE: There's java.nio.charset.MalformedInputException and sun.io.MalformedInputException - if (e.getClass().getName().endsWith("MalformedInputException")) { - msg = "Some input bytes do not match the file encoding."; - } else { - msg = e.getClass().getSimpleName(); + throw newModelBuilderException(); + } catch (IOException e) { + String msg = e.getMessage(); + if (msg == null || msg.isEmpty()) { + // NOTE: There's java.nio.charset.MalformedInputException and sun.io.MalformedInputException + if (e.getClass().getName().endsWith("MalformedInputException")) { + msg = "Some input bytes do not match the file encoding."; + } else { + msg = e.getClass().getSimpleName(); + } } + add(Severity.FATAL, Version.BASE, "Non-readable POM " + modelSource.getLocation() + ": " + msg, e); + throw newModelBuilderException(); } - add(Severity.FATAL, Version.BASE, "Non-readable POM " + modelSource.getLocation() + ": " + msg, e); - throw newModelBuilderException(); - } - if (model.getModelVersion() == null) { - String namespace = model.getNamespaceUri(); - if (namespace != null && namespace.startsWith(NAMESPACE_PREFIX)) { - model = model.withModelVersion(namespace.substring(NAMESPACE_PREFIX.length())); + if (model.getModelVersion() == null) { + String namespace = model.getNamespaceUri(); + if (namespace != null && namespace.startsWith(NAMESPACE_PREFIX)) { + model = model.withModelVersion(namespace.substring(NAMESPACE_PREFIX.length())); + } } - } - if (isBuildRequest()) { - model = model.withPomFile(modelSource.getPath()); - - Parent parent = model.getParent(); - if (parent != null) { - String groupId = parent.getGroupId(); - String artifactId = parent.getArtifactId(); - String version = parent.getVersion(); - String path = parent.getRelativePath(); - boolean versionContainsExpression = version != null && version.contains("${"); - if ((groupId == null || artifactId == null || version == null || versionContainsExpression) - && (path == null || !path.isEmpty())) { - Path pomFile = model.getPomFile(); - Path relativePath = Paths.get(path != null ? path : ".."); - Path pomPath = pomFile.resolveSibling(relativePath).normalize(); - if (Files.isDirectory(pomPath)) { - pomPath = modelProcessor.locateExistingPom(pomPath); - } - if (pomPath != null && Files.isRegularFile(pomPath)) { - if (rootDirectoryFromSession && !isParentWithinRootDirectory(pomPath, rootDirectory)) { - add( - Severity.FATAL, - Version.BASE, - "Parent POM " + pomPath + " is located above the root directory " - + rootDirectory - + ". This setup is invalid when a .mvn directory exists in a subdirectory.", - parent.getLocation("relativePath")); - throw newModelBuilderException(); + if (isBuildRequest()) { + model = model.withPomFile(modelSource.getPath()); + + Parent parent = model.getParent(); + if (parent != null) { + String groupId = parent.getGroupId(); + String artifactId = parent.getArtifactId(); + String version = parent.getVersion(); + String path = parent.getRelativePath(); + boolean versionContainsExpression = version != null && version.contains("${"); + if ((groupId == null || artifactId == null || version == null || versionContainsExpression) + && (path == null || !path.isEmpty())) { + Path pomFile = model.getPomFile(); + Path relativePath = Paths.get(path != null ? path : ".."); + Path pomPath = pomFile.resolveSibling(relativePath).normalize(); + if (Files.isDirectory(pomPath)) { + pomPath = modelProcessor.locateExistingPom(pomPath); } + if (pomPath != null && Files.isRegularFile(pomPath)) { + if (rootDirectoryFromSession && !isParentWithinRootDirectory(pomPath, rootDirectory)) { + add( + Severity.FATAL, + Version.BASE, + "Parent POM " + pomPath + " is located above the root directory " + + rootDirectory + + ". This setup is invalid when a .mvn directory exists in a subdirectory.", + parent.getLocation("relativePath")); + throw newModelBuilderException(); + } - Model parentModel = - derive(Sources.buildSource(pomPath)).readFileModel(); - String parentGroupId = getGroupId(parentModel); - String parentArtifactId = parentModel.getArtifactId(); - String parentVersion = getVersion(parentModel); - if ((groupId == null || groupId.equals(parentGroupId)) - && (artifactId == null || artifactId.equals(parentArtifactId)) - && (version == null - || version.equals(parentVersion) - || versionContainsExpression)) { - model = model.withParent(parent.with() - .groupId(parentGroupId) - .artifactId(parentArtifactId) - .version(parentVersion) - .build()); + Model parentModel = + derive(Sources.buildSource(pomPath)).readFileModel(); + String parentGroupId = getGroupId(parentModel); + String parentArtifactId = parentModel.getArtifactId(); + String parentVersion = getVersion(parentModel); + if ((groupId == null || groupId.equals(parentGroupId)) + && (artifactId == null || artifactId.equals(parentArtifactId)) + && (version == null + || version.equals(parentVersion) + || versionContainsExpression)) { + model = model.withParent(parent.with() + .groupId(parentGroupId) + .artifactId(parentArtifactId) + .version(parentVersion) + .build()); + } else { + mismatchRelativePathAndGA(model, parentGroupId, parentArtifactId); + } } else { - mismatchRelativePathAndGA(model, parentGroupId, parentArtifactId); - } - } else { - if (!MODEL_VERSION_4_0_0.equals(model.getModelVersion()) && path != null) { - wrongParentRelativePath(model); + if (!MODEL_VERSION_4_0_0.equals(model.getModelVersion()) && path != null) { + wrongParentRelativePath(model); + } } } } - } - // subprojects discovery - if (!hasSubprojectsDefined(model) - // only discover subprojects if POM > 4.0.0 - && !MODEL_VERSION_4_0_0.equals(model.getModelVersion()) - // and if packaging is POM (we check type, but the session is not yet available, - // we would require the project realm if we want to support extensions - && Type.POM.equals(model.getPackaging())) { - List<String> subprojects = new ArrayList<>(); - try (Stream<Path> files = Files.list(model.getProjectDirectory())) { - for (Path f : files.toList()) { - if (Files.isDirectory(f)) { - Path subproject = modelProcessor.locateExistingPom(f); - if (subproject != null) { - subprojects.add(f.getFileName().toString()); + // subprojects discovery + if (!hasSubprojectsDefined(model) + // only discover subprojects if POM > 4.0.0 + && !MODEL_VERSION_4_0_0.equals(model.getModelVersion()) + // and if packaging is POM (we check type, but the session is not yet available, + // we would require the project realm if we want to support extensions + && Type.POM.equals(model.getPackaging())) { + List<String> subprojects = new ArrayList<>(); + try (Stream<Path> files = Files.list(model.getProjectDirectory())) { + for (Path f : files.toList()) { + if (Files.isDirectory(f)) { + Path subproject = modelProcessor.locateExistingPom(f); + if (subproject != null) { + subprojects.add(f.getFileName().toString()); + } } } + if (!subprojects.isEmpty()) { + model = model.withSubprojects(subprojects); + } + } catch (IOException e) { + add(Severity.FATAL, Version.V41, "Error discovering subprojects", e); } - if (!subprojects.isEmpty()) { - model = model.withSubprojects(subprojects); - } - } catch (IOException e) { - add(Severity.FATAL, Version.V41, "Error discovering subprojects", e); } - } - // Enhanced property resolution with profile activation for CI-friendly versions and repository URLs - // This includes directory properties, profile properties, and user properties - Map<String, String> properties = getEnhancedProperties(model, rootDirectory); - - // CI friendly version processing with profile-aware properties - model = model.with() - .version(replaceCiFriendlyVersion(properties, model.getVersion())) - .parent( - model.getParent() != null - ? model.getParent() - .withVersion(replaceCiFriendlyVersion( - properties, - model.getParent().getVersion())) - : null) - .build(); + // Enhanced property resolution with profile activation for CI-friendly versions and repository URLs + // This includes directory properties, profile properties, and user properties + Map<String, String> properties = getEnhancedProperties(model, rootDirectory); + + // CI friendly version processing with profile-aware properties + model = model.with() + .version(replaceCiFriendlyVersion(properties, model.getVersion())) + .parent( + model.getParent() != null + ? model.getParent() + .withVersion(replaceCiFriendlyVersion( + properties, + model.getParent().getVersion())) + : null) + .build(); - // Repository URL interpolation with the same profile-aware properties - UnaryOperator<String> callback = properties::get; - model = model.with() - .repositories(interpolateRepository(model.getRepositories(), callback)) - .pluginRepositories(interpolateRepository(model.getPluginRepositories(), callback)) - .profiles(map(model.getProfiles(), this::interpolateRepository, callback)) - .distributionManagement(interpolateRepository(model.getDistributionManagement(), callback)) - .build(); - // Override model properties with user properties (use request properties - // to ensure consistency with model interpolation) - Map<String, String> userProps = request.getUserProperties(); - Map<String, String> newProps = merge(model.getProperties(), userProps); - if (newProps != null) { - model = model.withProperties(newProps); + // Repository URL interpolation with the same profile-aware properties + UnaryOperator<String> callback = properties::get; + model = model.with() + .repositories(interpolateRepository(model.getRepositories(), callback)) + .pluginRepositories(interpolateRepository(model.getPluginRepositories(), callback)) + .profiles(map(model.getProfiles(), this::interpolateRepository, callback)) + .distributionManagement(interpolateRepository(model.getDistributionManagement(), callback)) + .build(); + // Override model properties with user properties (use request properties + // to ensure consistency with model interpolation) + Map<String, String> userProps = request.getUserProperties(); + Map<String, String> newProps = merge(model.getProperties(), userProps); + if (newProps != null) { + model = model.withProperties(newProps); + } + model = model.withProfiles(merge(model.getProfiles(), userProps)); } - model = model.withProfiles(merge(model.getProfiles(), userProps)); - } - for (var transformer : transformers) { - model = transformer.transformFileModel(model); - } + for (var transformer : transformers) { + model = transformer.transformFileModel(model); + } - setSource(model); - modelValidator.validateFileModel( - session, - model, - isBuildRequest() ? ModelValidator.VALIDATION_LEVEL_STRICT : ModelValidator.VALIDATION_LEVEL_MINIMAL, - this); - InternalSession internalSession = InternalSession.from(session); - if (Features.mavenMaven3Personality(internalSession.getSession().getConfigProperties()) - && Objects.equals(ModelBuilder.MODEL_VERSION_4_1_0, model.getModelVersion())) { - add(Severity.FATAL, Version.BASE, "Maven3 mode: no higher model version than 4.0.0 allowed"); - } - if (hasFatalErrors()) { - throw newModelBuilderException(); - } + setSource(model); + modelValidator.validateFileModel( + session, + model, + isBuildRequest() + ? ModelValidator.VALIDATION_LEVEL_STRICT + : ModelValidator.VALIDATION_LEVEL_MINIMAL, + this); + InternalSession internalSession = InternalSession.from(session); + if (Features.mavenMaven3Personality(internalSession.getSession().getConfigProperties()) + && Objects.equals(ModelBuilder.MODEL_VERSION_4_1_0, model.getModelVersion())) { + add(Severity.FATAL, Version.BASE, "Maven3 mode: no higher model version than 4.0.0 allowed"); + } + if (hasFatalErrors()) { + throw newModelBuilderException(); + } - return model; + return model; + } finally { + if (trackRead) { + activeModelReads.remove(sourcePath.normalize()); + } + } } private DistributionManagement interpolateRepository( diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12301StackOverflowInternalParentRevisionTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12301StackOverflowInternalParentRevisionTest.java new file mode 100644 index 0000000000..f9543011c3 --- /dev/null +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh12301StackOverflowInternalParentRevisionTest.java @@ -0,0 +1,48 @@ +/* + * 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.io.File; + +import org.junit.jupiter.api.Test; + +/** + * This is a test set for <a href="https://github.com/apache/maven/issues/12301";>Issue #12301</a>. + * Verifies that a project with an internal parent in a subdirectory using CI-friendly + * {@code ${revision}} and a {@code .mvn/} root marker does not cause a StackOverflowError. + */ +public class MavenITgh12301StackOverflowInternalParentRevisionTest extends AbstractMavenIntegrationTestCase { + + public MavenITgh12301StackOverflowInternalParentRevisionTest() { + super("[4.0.0-rc-6,)"); + } + + @Test + public void testNoStackOverflowWithInternalParentAndRevision() throws Exception { + File testDir = extractResources("/gh-12301-stackoverflow-internal-parent"); + + Verifier verifier = newVerifier(testDir.getAbsolutePath()); + verifier.setAutoclean(false); + verifier.deleteArtifacts("org.apache.maven.its.gh12301"); + verifier.addCliArgument("-Drevision=1.0-SNAPSHOT"); + verifier.addCliArgument("validate"); + verifier.execute(); + verifier.verifyErrorFreeLog(); + } +} diff --git a/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/.mvn/.gitkeep b/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/.mvn/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/parent/pom.xml b/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/parent/pom.xml new file mode 100644 index 0000000000..940ecc2231 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/parent/pom.xml @@ -0,0 +1,29 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + <modelVersion>4.1.0</modelVersion> + + <groupId>org.apache.maven.its.gh12301</groupId> + <artifactId>parent</artifactId> + <version>1.0-SNAPSHOT</version> + <packaging>pom</packaging> + + <name>Maven Integration Test :: GH-12301 :: Parent</name> +</project> diff --git a/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/pom.xml b/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/pom.xml new file mode 100644 index 0000000000..e3a1d65be1 --- /dev/null +++ b/its/core-it-suite/src/test/resources/gh-12301-stackoverflow-internal-parent/pom.xml @@ -0,0 +1,38 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- +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. +--> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd";> + <modelVersion>4.1.0</modelVersion> + + <parent> + <groupId>org.apache.maven.its.gh12301</groupId> + <artifactId>parent</artifactId> + <version>${revision}</version> + <relativePath>parent</relativePath> + </parent> + + <artifactId>root</artifactId> + <packaging>pom</packaging> + + <name>Maven Integration Test :: GH-12301 :: Root</name> + + <properties> + <revision>1.0-SNAPSHOT</revision> + </properties> +</project>
