diff --git a/api/maven-api-core/src/main/java/org/apache/maven/api/services/Sources.java b/api/maven-api-core/src/main/java/org/apache/maven/api/services/Sources.java index 4acab6006a34..4f08f2d62dcc 100644 --- a/api/maven-api-core/src/main/java/org/apache/maven/api/services/Sources.java +++ b/api/maven-api-core/src/main/java/org/apache/maven/api/services/Sources.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.util.Objects; @@ -228,7 +229,12 @@ public Source resolve(@Nonnull String relative) { @Nullable public ModelSource resolve(@Nonnull ModelLocator locator, @Nonnull String relative) { String norm = relative.replace('\\', File.separatorChar).replace('/', File.separatorChar); - Path path = getPath().getParent().resolve(norm); + Path path; + try { + path = getPath().getParent().resolve(norm); + } catch (InvalidPathException e) { + return null; + } Path relatedPom = locator.locateExistingPom(path); if (relatedPom != null) { return new BuildPathSource(relatedPom); diff --git a/api/maven-api-core/src/test/java/org/apache/maven/api/services/SourcesTest.java b/api/maven-api-core/src/test/java/org/apache/maven/api/services/SourcesTest.java index f9abbe66d28b..8b6a56668504 100644 --- a/api/maven-api-core/src/test/java/org/apache/maven/api/services/SourcesTest.java +++ b/api/maven-api-core/src/test/java/org/apache/maven/api/services/SourcesTest.java @@ -136,6 +136,26 @@ void testStreamReading() throws IOException { } } + /** + * Tests that BuildPathSource.resolve() gracefully handles relative paths + * that are not valid filesystem paths (e.g. containing ':' which is illegal + * on Windows) by returning null instead of throwing InvalidPathException. + * This reproduces MNG-8129. + */ + @Test + void testBuildPathSourceResolveWithInvalidPath() throws IOException { + Path pomFile = tempDir.resolve("pom.xml"); + Files.writeString(pomFile, ""); + + Sources.BuildPathSource source = (Sources.BuildPathSource) Sources.buildSource(pomFile); + ModelSource.ModelLocator locator = mock(ModelSource.ModelLocator.class); + when(locator.locateExistingPom(any(Path.class))).thenReturn(null); + + // Must not throw InvalidPathException on any platform (MNG-8129) + ModelSource result = source.resolve(locator, "org.apache:apache"); + assertNull(result); + } + @Test void testNullHandling() { assertThrows(NullPointerException.class, () -> Sources.fromPath(null)); diff --git a/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/FileModelSource.java b/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/FileModelSource.java index f1a4e150495f..ef0944e08110 100644 --- a/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/FileModelSource.java +++ b/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/FileModelSource.java @@ -21,6 +21,7 @@ import java.io.File; import java.net.URI; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Path; import org.apache.maven.building.FileSource; @@ -61,7 +62,12 @@ public File getPomFile() { public ModelSource2 getRelatedSource(String relPath) { relPath = relPath.replace('\\', File.separatorChar).replace('/', File.separatorChar); - Path relatedPom = getPath().getParent().resolve(relPath); + Path relatedPom; + try { + relatedPom = getPath().getParent().resolve(relPath); + } catch (InvalidPathException e) { + return null; + } if (Files.isDirectory(relatedPom)) { // TODO figure out how to reuse ModelLocator.locatePom(File) here diff --git a/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 704991b5a0ba..47210ce7a3bc 100644 --- a/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/compat/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -84,6 +84,8 @@ public class DefaultModelValidator implements ModelValidator { private static final String ILLEGAL_FS_CHARS = "\\/:\"<>|?*"; + private static final String ILLEGAL_RELATIVE_PATH_CHARS = ":\"<>|?*"; + private static final String ILLEGAL_VERSION_CHARS = ILLEGAL_FS_CHARS; private static final String ILLEGAL_REPO_ID_CHARS = ILLEGAL_FS_CHARS; @@ -156,6 +158,23 @@ public void validateRawModel(Model m, ModelBuildingRequest request, ModelProblem } } else if (request.getValidationLevel() >= ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_2_0) { Severity errOn30 = getSeverity(request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0); + Severity errOn31 = getSeverity(request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_1); + + // [MNG-8129] Validate that relativePath does not contain characters that are illegal in filesystem paths + if (parent != null + && parent.getRelativePath() != null + && !parent.getRelativePath().isEmpty()) { + validateBannedCharacters( + "parent.", + "relativePath", + problems, + errOn31, + Version.V20, + parent.getRelativePath(), + null, + parent, + ILLEGAL_RELATIVE_PATH_CHARS); + } // [MNG-6074] Maven should produce an error if no model version has been set in a POM file used to build an // effective model. diff --git a/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileModelSourceTest.java b/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileModelSourceTest.java index a03184de6b83..e4995642f3fe 100644 --- a/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileModelSourceTest.java +++ b/compat/maven-model-builder/src/test/java/org/apache/maven/model/building/FileModelSourceTest.java @@ -63,6 +63,22 @@ void testWindowsPaths() throws Exception { assertTrue(upperCaseFileSource.equals(lowerCaseFileSource)); } + /** + * Tests that getRelatedSource() gracefully handles relative paths that are + * not valid filesystem paths (e.g. containing ':' which is illegal on Windows) + * by returning null instead of throwing InvalidPathException. + * This reproduces MNG-8129. + */ + @Test + void testGetRelatedSourceWithInvalidRelativePath() throws Exception { + File tempFile = createTempFile("pomTest"); + FileModelSource source = new FileModelSource(tempFile); + + // Must not throw InvalidPathException on any platform (MNG-8129) + ModelSource2 result = source.getRelatedSource("org.apache:apache"); + org.junit.jupiter.api.Assertions.assertNull(result); + } + private File createTempFile(String name) throws IOException { File tempFile = File.createTempFile(name, ".xml"); tempFile.deleteOnExit(); diff --git a/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java b/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java index 2952d29c68da..7e3688614bcd 100644 --- a/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java +++ b/compat/maven-model-builder/src/test/java/org/apache/maven/model/validation/DefaultModelValidatorTest.java @@ -469,6 +469,16 @@ void testDistributionManagementStatus() throws Exception { assertTrue(result.getErrors().get(0).contains("distributionManagement.status")); } + @Test + void testBadParentRelativePath() throws Exception { + SimpleProblemCollector result = validateRaw("bad-parent-relativePath.xml"); + + assertViolations(result, 0, 0, 1); + + assertContains(result.getWarnings().get(0), "parent.relativePath"); + assertContains(result.getWarnings().get(0), "must not contain any of these characters"); + } + @Test void testIncompleteParent() throws Exception { SimpleProblemCollector result = validateRaw("incomplete-parent.xml"); diff --git a/compat/maven-model-builder/src/test/resources/poms/validation/bad-parent-relativePath.xml b/compat/maven-model-builder/src/test/resources/poms/validation/bad-parent-relativePath.xml new file mode 100644 index 000000000000..4bd9a4d469c8 --- /dev/null +++ b/compat/maven-model-builder/src/test/resources/poms/validation/bad-parent-relativePath.xml @@ -0,0 +1,33 @@ + + + + 4.0.0 + + + org.apache + apache + 1 + org.apache:apache + + + aid + gid + 0.1 + diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index e213c852eb58..b3c5ec23a888 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -93,6 +93,8 @@ public class DefaultModelValidator implements ModelValidator { private static final String ILLEGAL_FS_CHARS = "\\/:\"<>|?*"; + private static final String ILLEGAL_RELATIVE_PATH_CHARS = ":\"<>|?*"; + private static final String ILLEGAL_VERSION_CHARS = ILLEGAL_FS_CHARS; private static final String ILLEGAL_REPO_ID_CHARS = ILLEGAL_FS_CHARS; @@ -439,6 +441,23 @@ public void validateFileModel(Session s, Model m, int validationLevel, ModelProb } Severity errOn30 = getSeverity(validationLevel, ModelValidator.VALIDATION_LEVEL_MAVEN_3_0); + Severity errOn31 = getSeverity(validationLevel, ModelValidator.VALIDATION_LEVEL_MAVEN_3_1); + + // [MNG-8129] Validate that relativePath does not contain characters that are illegal in filesystem paths + if (parent != null + && parent.getRelativePath() != null + && !parent.getRelativePath().isEmpty()) { + validateBannedCharacters( + "parent.", + "relativePath", + problems, + errOn31, + Version.V20, + parent.getRelativePath(), + null, + parent, + ILLEGAL_RELATIVE_PATH_CHARS); + } boolean isModelVersion41OrMore = !Objects.equals(ModelBuilder.MODEL_VERSION_4_0_0, m.getModelVersion()); if (isModelVersion41OrMore) { diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java index 1f0329795d4c..f833a4586df0 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java @@ -539,6 +539,16 @@ void testDistributionManagementStatus() throws Exception { assertTrue(result.getErrors().get(0).contains("distributionManagement.status")); } + @Test + void testBadParentRelativePath() throws Exception { + SimpleProblemCollector result = validateFile("bad-parent-relativePath.xml"); + + assertViolations(result, 0, 1, 0); + + assertContains(result.getErrors().get(0), "parent.relativePath"); + assertContains(result.getErrors().get(0), "must not contain any of these characters"); + } + @Test void testIncompleteParent() throws Exception { SimpleProblemCollector result = validateRaw("incomplete-parent.xml"); diff --git a/impl/maven-impl/src/test/resources/poms/validation/bad-parent-relativePath.xml b/impl/maven-impl/src/test/resources/poms/validation/bad-parent-relativePath.xml new file mode 100644 index 000000000000..4bd9a4d469c8 --- /dev/null +++ b/impl/maven-impl/src/test/resources/poms/validation/bad-parent-relativePath.xml @@ -0,0 +1,33 @@ + + + + 4.0.0 + + + org.apache + apache + 1 + org.apache:apache + + + aid + gid + 0.1 +