From f6d76d3a8041c896a2b26f35b6e9a6d25fc9c870 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 15:05:19 +0200 Subject: [PATCH 1/7] Keep request and session repositories ahead of model-declared ones Repositories declared by a resolved model no longer replace a same-id repository supplied by the request or session; the ids supplied at resolver construction keep their precedence on the replace pass. A repository the model itself declared is still refreshed in place, e.g. once its URL has been interpolated. --- .../internal/DefaultModelResolver.java | 15 +++++++ .../internal/DefaultModelResolverTest.java | 41 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelResolver.java b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelResolver.java index 398652003453..4dc86735a1fb 100644 --- a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelResolver.java +++ b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelResolver.java @@ -75,6 +75,8 @@ class DefaultModelResolver implements ModelResolver { private final Set repositoryIds; + private final Set externalRepositoryIds; + DefaultModelResolver( RepositorySystemSession session, RequestTrace trace, @@ -93,6 +95,11 @@ class DefaultModelResolver implements ModelResolver { this.externalRepositories = Collections.unmodifiableList(new ArrayList<>(repositories)); this.repositoryIds = new HashSet<>(); + Set externalIds = new HashSet<>(); + for (RemoteRepository externalRepository : this.externalRepositories) { + externalIds.add(externalRepository.getId()); + } + this.externalRepositoryIds = Collections.unmodifiableSet(externalIds); } private DefaultModelResolver(DefaultModelResolver original) { @@ -105,6 +112,7 @@ private DefaultModelResolver(DefaultModelResolver original) { this.repositories = new ArrayList<>(original.repositories); this.externalRepositories = original.externalRepositories; this.repositoryIds = new HashSet<>(original.repositoryIds); + this.externalRepositoryIds = original.externalRepositoryIds; } @Override @@ -123,6 +131,13 @@ public void addRepository(final Repository repository, boolean replace) throws I return; } + if (externalRepositoryIds.contains(repository.getId())) { + // Replacement is meant to refresh a repository this model declared earlier, e.g. + // once its URL has been interpolated. Repositories supplied by the request or the + // session are not model-declared, so they keep precedence and are left in place. + return; + } + removeMatchingRepository(repositories, repository.getId()); } diff --git a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultModelResolverTest.java b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultModelResolverTest.java index 747d24461f94..6b2b880ded4d 100644 --- a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultModelResolverTest.java +++ b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultModelResolverTest.java @@ -20,19 +20,25 @@ import javax.inject.Inject; +import java.io.File; import java.net.MalformedURLException; +import java.nio.file.Path; import java.util.Arrays; import org.apache.maven.model.Dependency; import org.apache.maven.model.Parent; +import org.apache.maven.model.Repository; import org.apache.maven.model.resolution.ModelResolver; import org.apache.maven.model.resolution.UnresolvableModelException; import org.codehaus.plexus.component.repository.exception.ComponentLookupException; import org.codehaus.plexus.testing.PlexusTest; +import org.eclipse.aether.DefaultRepositorySystemSession; import org.eclipse.aether.impl.ArtifactResolver; import org.eclipse.aether.impl.RemoteRepositoryManager; import org.eclipse.aether.impl.VersionRangeResolver; +import org.eclipse.aether.repository.LocalRepository; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -195,6 +201,41 @@ public void testResolveDependencySuccessfullyResolvesExistingDependencyUsingHigh @Inject private RemoteRepositoryManager remoteRepositoryManager; + @Test + public void testConstructionSuppliedRepositoryKeepsPrecedence(@TempDir Path localRepository) throws Exception { + // An empty local repository, so resolution has to consult the remote repository list + // rather than a copy cached by another test in this class. + final DefaultRepositorySystemSession isolatedSession = MavenRepositorySystemUtils.newSession(); + isolatedSession.setLocalRepositoryManager( + system.newLocalRepositoryManager(isolatedSession, new LocalRepository(localRepository.toFile()))); + + final ModelResolver resolver = new DefaultModelResolver( + isolatedSession, + null, + this.getClass().getName(), + artifactResolver, + versionRangeResolver, + remoteRepositoryManager, + Arrays.asList(newTestRepository())); + + // A model-declared repository that reuses the external repository's id; the external + // repository must keep its slot. + final Repository repository = new Repository(); + repository.setId("repo"); + repository.setUrl(new File("target/no-such-repository").toURI().toURL().toString()); + + resolver.addRepository(repository); + resolver.addRepository(repository, true); + + final Parent parent = new Parent(); + parent.setGroupId("ut.simple"); + parent.setArtifactId("artifact"); + parent.setVersion("1.0"); + + // The external repository kept its slot, so the artifact still resolves. + assertNotNull(resolver.resolveModel(parent)); + } + private ModelResolver newModelResolver() throws ComponentLookupException, MalformedURLException { return new DefaultModelResolver( this.session, From 7491fa9f4805e1f3544bc3ea3ceaf9f2fdfa8e7e Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 15:11:15 +0200 Subject: [PATCH 2/7] Validate relocation coordinate components before use --- .../project/artifact/MavenMetadataSource.java | 31 +++ .../MavenMetadataSourceRelocationTest.java | 179 ++++++++++++++++++ .../DefaultArtifactDescriptorReader.java | 34 ++++ ...criptorReaderRelocationValidationTest.java | 87 +++++++++ .../1.0/dep-invalid-relocation-1.0.pom | 38 ++++ 5 files changed, 369 insertions(+) create mode 100644 maven-core/src/test/java/org/apache/maven/project/artifact/MavenMetadataSourceRelocationTest.java create mode 100644 maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java create mode 100644 maven-resolver-provider/src/test/resources/repo/ut/simple/dep-invalid-relocation/1.0/dep-invalid-relocation-1.0.pom diff --git a/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java b/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java index 9df7059d070e..9ee93364b090 100644 --- a/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java +++ b/maven-core/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java @@ -612,16 +612,19 @@ private ProjectRelocation retrieveRelocatedProject(Artifact artifact, MetadataRe if (relocation != null) { if (relocation.getGroupId() != null) { + requireValidCoordinateComponent(relocation.getGroupId(), "groupId", artifact); artifact.setGroupId(relocation.getGroupId()); relocatedArtifact = artifact; project.setGroupId(relocation.getGroupId()); } if (relocation.getArtifactId() != null) { + requireValidCoordinateComponent(relocation.getArtifactId(), "artifactId", artifact); artifact.setArtifactId(relocation.getArtifactId()); relocatedArtifact = artifact; project.setArtifactId(relocation.getArtifactId()); } if (relocation.getVersion() != null) { + requireValidCoordinateComponent(relocation.getVersion(), "version", artifact); // note: see MNG-3454. This causes a problem, but fixing it may break more. artifact.setVersionRange(VersionRange.createFromVersion(relocation.getVersion())); relocatedArtifact = artifact; @@ -677,6 +680,34 @@ private ProjectRelocation retrieveRelocatedProject(Artifact artifact, MetadataRe return rel; } + /** + * Checks that a relocation coordinate component is usable as an artifact coordinate component before it + * is applied to the artifact and project. A component outside the coordinate character set is rejected so + * that only well-formed coordinates enter resolution. + */ + private static void requireValidCoordinateComponent(String value, String component, Artifact artifact) + throws ArtifactMetadataRetrievalException { + if (isInvalidCoordinateComponent(value)) { + throw new ArtifactMetadataRetrievalException( + "Invalid relocation " + component + " '" + value + "' for " + artifact.getId() + + ": not a valid artifact coordinate component", + null, + artifact); + } + } + + private static boolean isInvalidCoordinateComponent(String value) { + if ("..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":")) { + return true; + } + for (int i = 0; i < value.length(); i++) { + if (Character.isISOControl(value.charAt(i))) { + return true; + } + } + return false; + } + private ModelProblem hasMissingParentPom(ProjectBuildingException e) { if (e.getCause() instanceof ModelBuildingException) { ModelBuildingException mbe = (ModelBuildingException) e.getCause(); diff --git a/maven-core/src/test/java/org/apache/maven/project/artifact/MavenMetadataSourceRelocationTest.java b/maven-core/src/test/java/org/apache/maven/project/artifact/MavenMetadataSourceRelocationTest.java new file mode 100644 index 000000000000..33699037cdaa --- /dev/null +++ b/maven-core/src/test/java/org/apache/maven/project/artifact/MavenMetadataSourceRelocationTest.java @@ -0,0 +1,179 @@ +/* + * 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.project.artifact; + +import java.lang.reflect.Field; +import java.util.Collections; + +import org.apache.maven.artifact.Artifact; +import org.apache.maven.artifact.DefaultArtifact; +import org.apache.maven.artifact.factory.ArtifactFactory; +import org.apache.maven.artifact.handler.DefaultArtifactHandler; +import org.apache.maven.artifact.metadata.ArtifactMetadataRetrievalException; +import org.apache.maven.artifact.metadata.ResolutionGroup; +import org.apache.maven.artifact.repository.ArtifactRepository; +import org.apache.maven.artifact.repository.metadata.RepositoryMetadataManager; +import org.apache.maven.bridge.MavenRepositorySystem; +import org.apache.maven.model.DistributionManagement; +import org.apache.maven.model.Relocation; +import org.apache.maven.plugin.LegacySupport; +import org.apache.maven.project.MavenProject; +import org.apache.maven.project.ProjectBuilder; +import org.apache.maven.project.ProjectBuildingRequest; +import org.apache.maven.project.ProjectBuildingResult; +import org.apache.maven.repository.legacy.metadata.DefaultMetadataResolutionRequest; +import org.apache.maven.repository.legacy.metadata.MetadataResolutionRequest; +import org.codehaus.plexus.logging.Logger; +import org.eclipse.aether.RepositorySystemSession; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Verifies that a relocation read from a resolved project's distribution management is validated before its + * components are applied to the artifact and project being resolved, exercising the real + * {@link MavenMetadataSource#retrieve(MetadataResolutionRequest)} code path with mocked collaborators (no + * on-disk artifact resolution, so this test does not depend on, or share, any module-level local repository). + */ +class MavenMetadataSourceRelocationTest { + + private MavenMetadataSource newSource(ProjectBuilder projectBuilder) throws Exception { + MavenMetadataSource source = new MavenMetadataSource(); + + ArtifactFactory artifactFactory = mock(ArtifactFactory.class); + when(artifactFactory.createProjectArtifact(any(), any(), any(), any())) + .thenAnswer(invocation -> new DefaultArtifact( + (String) invocation.getArgument(0), + (String) invocation.getArgument(1), + (String) invocation.getArgument(2), + (String) invocation.getArgument(3), + "pom", + null, + new DefaultArtifactHandler("pom"))); + + LegacySupport legacySupport = mock(LegacySupport.class); + RepositorySystemSession repositorySession = mock(RepositorySystemSession.class); + when(legacySupport.getRepositorySession()).thenReturn(repositorySession); + when(legacySupport.getSession()).thenReturn(null); + + setField(source, "artifactFactory", artifactFactory); + setField(source, "repositorySystem", mock(MavenRepositorySystem.class)); + setField(source, "repositoryMetadataManager", mock(RepositoryMetadataManager.class)); + setField(source, "projectBuilder", projectBuilder); + setField(source, "logger", mock(Logger.class)); + setField(source, "cache", mock(MavenMetadataCache.class)); + setField(source, "legacySupport", legacySupport); + + return source; + } + + private static void setField(Object target, String name, Object value) throws Exception { + Field field = MavenMetadataSource.class.getDeclaredField(name); + field.setAccessible(true); + field.set(target, value); + } + + private static Artifact newArtifact(String groupId, String artifactId, String version) { + return new DefaultArtifact( + groupId, artifactId, version, Artifact.SCOPE_COMPILE, "pom", null, new DefaultArtifactHandler("pom")); + } + + private static MavenProject newProject(String groupId, String artifactId, String version, Relocation relocation) { + MavenProject project = new MavenProject(); + project.setGroupId(groupId); + project.setArtifactId(artifactId); + project.setVersion(version); + if (relocation != null) { + DistributionManagement distMgmt = new DistributionManagement(); + distMgmt.setRelocation(relocation); + project.setDistributionManagement(distMgmt); + } + return project; + } + + private static MetadataResolutionRequest newRequest(Artifact artifact) { + MetadataResolutionRequest request = new DefaultMetadataResolutionRequest(); + request.setArtifact(artifact); + request.setLocalRepository(mock(ArtifactRepository.class)); + request.setRemoteRepositories(Collections.emptyList()); + return request; + } + + @Test + void testRelocationWithInvalidArtifactIdIsRejected() throws Exception { + Relocation relocation = new Relocation(); + relocation.setArtifactId("a/b"); + + MavenProject relocatingProject = newProject("group", "original", "1.0", relocation); + MavenProject finalProject = newProject("group", "a/b", "1.0", null); + + ProjectBuildingResult first = mock(ProjectBuildingResult.class); + when(first.getProject()).thenReturn(relocatingProject); + ProjectBuildingResult second = mock(ProjectBuildingResult.class); + when(second.getProject()).thenReturn(finalProject); + + ProjectBuilder projectBuilder = mock(ProjectBuilder.class); + when(projectBuilder.build(any(Artifact.class), any(ProjectBuildingRequest.class))) + .thenReturn(first, second); + + MavenMetadataSource source = newSource(projectBuilder); + Artifact artifact = newArtifact("group", "original", "1.0"); + MetadataResolutionRequest request = newRequest(artifact); + + ArtifactMetadataRetrievalException exception = + assertThrows(ArtifactMetadataRetrievalException.class, () -> source.retrieve(request)); + assertEquals(true, exception.getMessage().contains("a/b")); + assertEquals(true, exception.getMessage().contains("artifactId")); + } + + @Test + void testWellFormedRelocationIsApplied() throws Exception { + Relocation relocation = new Relocation(); + relocation.setGroupId("group.moved"); + relocation.setArtifactId("artifact-moved"); + relocation.setVersion("2.0"); + + MavenProject relocatingProject = newProject("group", "original", "1.0", relocation); + MavenProject finalProject = newProject("group.moved", "artifact-moved", "2.0", null); + + ProjectBuildingResult first = mock(ProjectBuildingResult.class); + when(first.getProject()).thenReturn(relocatingProject); + ProjectBuildingResult second = mock(ProjectBuildingResult.class); + when(second.getProject()).thenReturn(finalProject); + + ProjectBuilder projectBuilder = mock(ProjectBuilder.class); + when(projectBuilder.build(any(Artifact.class), any(ProjectBuildingRequest.class))) + .thenReturn(first, second); + + MavenMetadataSource source = newSource(projectBuilder); + Artifact artifact = newArtifact("group", "original", "1.0"); + MetadataResolutionRequest request = newRequest(artifact); + + ResolutionGroup result = source.retrieve(request); + + assertEquals("group.moved", artifact.getGroupId()); + assertEquals("artifact-moved", artifact.getArtifactId()); + assertEquals("2.0", artifact.getVersion()); + assertEquals(artifact, result.getRelocatedArtifact()); + } +} diff --git a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java index bfea91cb6cfb..59867cd2f538 100644 --- a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java +++ b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReader.java @@ -340,6 +340,9 @@ private Model loadPom( if (relocation != null) { result.addRelocation(a); + requireValidCoordinateComponent(relocation.getGroupId(), "groupId", a, result); + requireValidCoordinateComponent(relocation.getArtifactId(), "artifactId", a, result); + requireValidCoordinateComponent(relocation.getVersion(), "version", a, result); a = new RelocatedArtifact( a, relocation.getGroupId(), @@ -373,6 +376,37 @@ private Relocation getRelocation(Model model) { return relocation; } + /** + * Checks that a relocation coordinate component is usable as an artifact coordinate component before it + * is applied to the artifact being resolved. A component outside the coordinate character set is rejected + * so that only well-formed coordinates enter resolution. + */ + private static void requireValidCoordinateComponent( + String value, String component, Artifact artifact, ArtifactDescriptorResult result) + throws ArtifactDescriptorException { + if (value == null || value.isEmpty()) { + return; // component is not relocated: the original artifact's value is kept + } + if (isInvalidCoordinateComponent(value)) { + IllegalArgumentException cause = new IllegalArgumentException("Invalid relocation " + component + " '" + + value + "' for " + artifact + ": not a valid artifact coordinate component"); + result.addException(cause); + throw new ArtifactDescriptorException(result, cause.getMessage(), cause); + } + } + + private static boolean isInvalidCoordinateComponent(String value) { + if ("..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":")) { + return true; + } + for (int i = 0; i < value.length(); i++) { + if (Character.isISOControl(value.charAt(i))) { + return true; + } + } + return false; + } + private void missingDescriptor( RepositorySystemSession session, RequestTrace trace, Artifact artifact, Exception exception) { RepositoryEvent.Builder event = new RepositoryEvent.Builder(session, EventType.ARTIFACT_DESCRIPTOR_MISSING); diff --git a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java new file mode 100644 index 000000000000..7db05d4d4bf6 --- /dev/null +++ b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java @@ -0,0 +1,87 @@ +/* + * 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.repository.internal; + +import javax.inject.Inject; + +import java.io.File; + +import org.codehaus.plexus.testing.PlexusTest; +import org.eclipse.aether.DefaultRepositorySystemSession; +import org.eclipse.aether.RepositorySystem; +import org.eclipse.aether.RepositorySystemSession; +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.impl.ArtifactDescriptorReader; +import org.eclipse.aether.repository.LocalRepository; +import org.eclipse.aether.repository.RemoteRepository; +import org.eclipse.aether.resolution.ArtifactDescriptorException; +import org.eclipse.aether.resolution.ArtifactDescriptorRequest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.codehaus.plexus.testing.PlexusExtension.getTestFile; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that a relocation read from a resolved artifact descriptor's model is validated before its + * components are applied to the artifact being resolved, exercising the real + * {@link ArtifactDescriptorReader#readArtifactDescriptor(RepositorySystemSession, ArtifactDescriptorRequest)} code + * path against a fixture repository on disk. Each test method gets its own {@code @TempDir} local repository so + * runs do not share resolution state with the rest of this module's tests. + */ +@PlexusTest +public class DefaultArtifactDescriptorReaderRelocationValidationTest { + + @Inject + private RepositorySystem system; + + @Inject + private ArtifactDescriptorReader reader; + + private RepositorySystemSession session; + + @BeforeEach + void setUp(@TempDir File localRepoDir) { + DefaultRepositorySystemSession newSession = MavenRepositorySystemUtils.newSession(); + LocalRepository localRepo = new LocalRepository(localRepoDir); + newSession.setLocalRepositoryManager(system.newLocalRepositoryManager(newSession, localRepo)); + session = newSession; + } + + private static RemoteRepository testRepository() throws Exception { + return new RemoteRepository.Builder( + "repo", + "default", + getTestFile("target/test-classes/repo").toURI().toURL().toString()) + .build(); + } + + @Test + void testRelocationWithInvalidArtifactIdIsRejected() throws Exception { + ArtifactDescriptorRequest request = new ArtifactDescriptorRequest(); + request.addRepository(testRepository()); + request.setArtifact(new DefaultArtifact("ut.simple", "dep-invalid-relocation", "pom", "1.0")); + + ArtifactDescriptorException exception = assertThrows( + ArtifactDescriptorException.class, () -> reader.readArtifactDescriptor(session, request)); + assertTrue(exception.getMessage().contains("artifactId")); + } +} diff --git a/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-invalid-relocation/1.0/dep-invalid-relocation-1.0.pom b/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-invalid-relocation/1.0/dep-invalid-relocation-1.0.pom new file mode 100644 index 000000000000..c2ba28611d6b --- /dev/null +++ b/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-invalid-relocation/1.0/dep-invalid-relocation-1.0.pom @@ -0,0 +1,38 @@ + + + + + + 4.0.0 + + ut.simple + dep-invalid-relocation + 1.0 + pom + + Relocation With Invalid Coordinate Component + + + + a/b + + + From 4a9f8f59388040dcb28e2062c1aae2d6ab76368a Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 16:56:15 +0200 Subject: [PATCH 3/7] Cover remaining rejected character classes in relocation coordinate tests --- ...criptorReaderRelocationValidationTest.java | 26 ++++++++++++- .../1.0/dep-backslash-relocation-1.0.pom | 38 +++++++++++++++++++ .../1.0/dep-control-char-relocation-1.0.pom | 38 +++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 maven-resolver-provider/src/test/resources/repo/ut/simple/dep-backslash-relocation/1.0/dep-backslash-relocation-1.0.pom create mode 100644 maven-resolver-provider/src/test/resources/repo/ut/simple/dep-control-char-relocation/1.0/dep-control-char-relocation-1.0.pom diff --git a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java index 7db05d4d4bf6..5a53640066c8 100644 --- a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java +++ b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java @@ -80,8 +80,30 @@ void testRelocationWithInvalidArtifactIdIsRejected() throws Exception { request.addRepository(testRepository()); request.setArtifact(new DefaultArtifact("ut.simple", "dep-invalid-relocation", "pom", "1.0")); - ArtifactDescriptorException exception = assertThrows( - ArtifactDescriptorException.class, () -> reader.readArtifactDescriptor(session, request)); + ArtifactDescriptorException exception = + assertThrows(ArtifactDescriptorException.class, () -> reader.readArtifactDescriptor(session, request)); + assertTrue(exception.getMessage().contains("artifactId")); + } + + @Test + void testRelocationWithBackslashIsRejected() throws Exception { + ArtifactDescriptorRequest request = new ArtifactDescriptorRequest(); + request.addRepository(testRepository()); + request.setArtifact(new DefaultArtifact("ut.simple", "dep-backslash-relocation", "pom", "1.0")); + + ArtifactDescriptorException exception = + assertThrows(ArtifactDescriptorException.class, () -> reader.readArtifactDescriptor(session, request)); + assertTrue(exception.getMessage().contains("artifactId")); + } + + @Test + void testRelocationWithControlCharacterIsRejected() throws Exception { + ArtifactDescriptorRequest request = new ArtifactDescriptorRequest(); + request.addRepository(testRepository()); + request.setArtifact(new DefaultArtifact("ut.simple", "dep-control-char-relocation", "pom", "1.0")); + + ArtifactDescriptorException exception = + assertThrows(ArtifactDescriptorException.class, () -> reader.readArtifactDescriptor(session, request)); assertTrue(exception.getMessage().contains("artifactId")); } } diff --git a/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-backslash-relocation/1.0/dep-backslash-relocation-1.0.pom b/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-backslash-relocation/1.0/dep-backslash-relocation-1.0.pom new file mode 100644 index 000000000000..62bafd6a7f26 --- /dev/null +++ b/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-backslash-relocation/1.0/dep-backslash-relocation-1.0.pom @@ -0,0 +1,38 @@ + + + + + + 4.0.0 + + ut.simple + dep-backslash-relocation + 1.0 + pom + + Relocation With Backslash Coordinate Component + + + + a\b + + + diff --git a/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-control-char-relocation/1.0/dep-control-char-relocation-1.0.pom b/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-control-char-relocation/1.0/dep-control-char-relocation-1.0.pom new file mode 100644 index 000000000000..8f18a49bf1d0 --- /dev/null +++ b/maven-resolver-provider/src/test/resources/repo/ut/simple/dep-control-char-relocation/1.0/dep-control-char-relocation-1.0.pom @@ -0,0 +1,38 @@ + + + + + + 4.0.0 + + ut.simple + dep-control-char-relocation + 1.0 + pom + + Relocation With Control Character Coordinate Component + + + + a b + + + From 86d5e408e238c33d21344423ee2694f4d65014da Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 15:14:49 +0200 Subject: [PATCH 4/7] Reject repository metadata with invalid version tokens --- .../internal/DefaultVersionRangeResolver.java | 41 +++++++++++++- .../internal/DefaultVersionResolver.java | 44 ++++++++++++++- .../DefaultVersionRangeResolverTest.java | 54 +++++++++++++++++++ .../internal/DefaultVersionResolverTest.java | 28 ++++++++++ .../1.0-SNAPSHOT/maven-metadata.xml | 40 ++++++++++++++ .../1.0-SNAPSHOT/maven-metadata.xml | 33 ++++++++++++ .../dep-invalid-versions/maven-metadata.xml | 34 ++++++++++++ 7 files changed, 272 insertions(+), 2 deletions(-) create mode 100644 maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionRangeResolverTest.java create mode 100644 maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-sv/1.0-SNAPSHOT/maven-metadata.xml create mode 100644 maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-ts/1.0-SNAPSHOT/maven-metadata.xml create mode 100644 maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-versions/maven-metadata.xml diff --git a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionRangeResolver.java b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionRangeResolver.java index 23b7bc84b502..8b50bac5c004 100644 --- a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionRangeResolver.java +++ b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionRangeResolver.java @@ -23,6 +23,7 @@ import javax.inject.Singleton; import java.io.FileInputStream; +import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.Collections; @@ -274,8 +275,12 @@ private Versioning readVersions( if (metadata.getFile() != null && metadata.getFile().exists()) { try (InputStream in = new FileInputStream(metadata.getFile())) { - versioning = + Versioning parsed = new MetadataXpp3Reader().read(in, false).getVersioning(); + + validateVersioning(parsed); + + versioning = parsed; } } } @@ -288,6 +293,40 @@ private Versioning readVersions( return (versioning != null) ? versioning : new Versioning(); } + /** + * Version tokens adopted from repository metadata must be valid coordinate components; metadata carrying + * anything else is treated as invalid. + */ + private static void validateVersioning(Versioning versioning) throws IOException { + if (versioning == null) { + return; + } + for (String version : versioning.getVersions()) { + validateVersionToken(version); + } + validateVersionToken(versioning.getLatest()); + validateVersionToken(versioning.getRelease()); + } + + private static void validateVersionToken(String value) throws IOException { + if (value == null || value.isEmpty()) { + return; + } + boolean valid = !"..".equals(value); + if (valid) { + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c == '/' || c == '\\' || c == ':' || Character.isISOControl(c)) { + valid = false; + break; + } + } + } + if (!valid) { + throw new IOException("Metadata contains an invalid version token: '" + value + "'"); + } + } + private Versioning filterVersionsByRepositoryType(Versioning versioning, RemoteRepository remoteRepository) { if (remoteRepository == null) { return versioning; diff --git a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionResolver.java b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionResolver.java index cccbd9f5ff92..ca576c5df4e1 100644 --- a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionResolver.java +++ b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionResolver.java @@ -278,9 +278,13 @@ private Versioning readVersions( if (metadata.getFile() != null && metadata.getFile().exists()) { try (InputStream in = new FileInputStream(metadata.getFile())) { - versioning = + Versioning parsed = new MetadataXpp3Reader().read(in, false).getVersioning(); + validateVersioning(parsed); + + versioning = parsed; + /* NOTE: Users occasionally misuse the id "local" for remote repos which screws up the metadata of the local repository. This is especially troublesome during snapshot resolution so we try @@ -311,6 +315,44 @@ private Versioning readVersions( return (versioning != null) ? versioning : new Versioning(); } + /** + * Version tokens adopted from repository metadata must be valid coordinate components; metadata carrying + * anything else is treated as invalid. + */ + private static void validateVersioning(Versioning versioning) throws IOException { + if (versioning == null) { + return; + } + validateVersionToken(versioning.getLatest()); + validateVersionToken(versioning.getRelease()); + for (SnapshotVersion snapshotVersion : versioning.getSnapshotVersions()) { + validateVersionToken(snapshotVersion.getVersion()); + } + Snapshot snapshot = versioning.getSnapshot(); + if (snapshot != null) { + validateVersionToken(snapshot.getTimestamp()); + } + } + + private static void validateVersionToken(String value) throws IOException { + if (value == null || value.isEmpty()) { + return; + } + boolean valid = !"..".equals(value); + if (valid) { + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c == '/' || c == '\\' || c == ':' || Character.isISOControl(c)) { + valid = false; + break; + } + } + } + if (!valid) { + throw new IOException("Metadata contains an invalid version token: '" + value + "'"); + } + } + private void invalidMetadata( RepositorySystemSession session, RequestTrace trace, diff --git a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionRangeResolverTest.java b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionRangeResolverTest.java new file mode 100644 index 000000000000..94adc27b3a8b --- /dev/null +++ b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionRangeResolverTest.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.repository.internal; + +import javax.inject.Inject; + +import org.codehaus.plexus.testing.PlexusTest; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.impl.VersionRangeResolver; +import org.eclipse.aether.resolution.VersionRangeRequest; +import org.eclipse.aether.resolution.VersionRangeResult; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +@PlexusTest +public class DefaultVersionRangeResolverTest extends AbstractRepositoryTest { + + @Inject + private VersionRangeResolver versionRangeResolver; + + @Test + public void testVersionsListFromMetadataWithInvalidTokenIsRejected() throws Exception { + VersionRangeRequest request = new VersionRangeRequest(); + request.addRepository(newTestRepository()); + Artifact artifact = new DefaultArtifact("org.apache.maven.its", "dep-invalid-versions", "jar", "[1.0,)"); + request.setArtifact(artifact); + + VersionRangeResult result = versionRangeResolver.resolveVersionRange(session, request); + + // The metadata carries a version token that is not a valid coordinate component, so the whole + // metadata file is treated as invalid and none of its versions (valid or not) are offered. + assertTrue(result.getVersions().isEmpty()); + assertFalse(result.getVersions().stream().anyMatch(v -> v.toString().contains("1.0:2.0"))); + } +} diff --git a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionResolverTest.java b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionResolverTest.java index 5ad5727913cd..1fb81e82db9c 100644 --- a/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionResolverTest.java +++ b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultVersionResolverTest.java @@ -78,4 +78,32 @@ public void testResolveSeparateInstalledClassifiedNonVersionedArtifacts() throws VersionResult resultB = versionResolver.resolveVersion(session, requestB); assertEquals(versionB, resultB.getVersion()); } + + @Test + public void testSnapshotVersionFromMetadataWithInvalidTokenIsRejected() throws Exception { + VersionRequest request = new VersionRequest(); + request.addRepository(newTestRepository()); + Artifact artifact = new DefaultArtifact("org.apache.maven.its", "dep-invalid-sv", "", "jar", "1.0-SNAPSHOT"); + request.setArtifact(artifact); + + VersionResult result = versionResolver.resolveVersion(session, request); + + // The metadata carries a snapshotVersion value that is not a valid coordinate component, so the + // metadata is treated as invalid and resolution falls back to the requested base version. + assertEquals("1.0-SNAPSHOT", result.getVersion()); + } + + @Test + public void testSnapshotTimestampFromMetadataWithInvalidTokenIsRejected() throws Exception { + VersionRequest request = new VersionRequest(); + request.addRepository(newTestRepository()); + Artifact artifact = new DefaultArtifact("org.apache.maven.its", "dep-invalid-ts", "", "jar", "1.0-SNAPSHOT"); + request.setArtifact(artifact); + + VersionResult result = versionResolver.resolveVersion(session, request); + + // The metadata carries a snapshot timestamp that is not a valid coordinate component, so the + // metadata is treated as invalid and resolution falls back to the requested base version. + assertEquals("1.0-SNAPSHOT", result.getVersion()); + } } diff --git a/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-sv/1.0-SNAPSHOT/maven-metadata.xml b/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-sv/1.0-SNAPSHOT/maven-metadata.xml new file mode 100644 index 000000000000..55a21aedc56c --- /dev/null +++ b/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-sv/1.0-SNAPSHOT/maven-metadata.xml @@ -0,0 +1,40 @@ + + + + + + org.apache.maven.its + dep-invalid-sv + 1.0-SNAPSHOT + + + 20120809.112920 + 1 + + 20120809112920 + + + jar + 1.0:2.0 + 20120809112920 + + + + diff --git a/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-ts/1.0-SNAPSHOT/maven-metadata.xml b/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-ts/1.0-SNAPSHOT/maven-metadata.xml new file mode 100644 index 000000000000..3bf9f11780c0 --- /dev/null +++ b/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-ts/1.0-SNAPSHOT/maven-metadata.xml @@ -0,0 +1,33 @@ + + + + + + org.apache.maven.its + dep-invalid-ts + 1.0-SNAPSHOT + + + 20120809.112920:1 + 1 + + 20120809112920 + + diff --git a/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-versions/maven-metadata.xml b/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-versions/maven-metadata.xml new file mode 100644 index 000000000000..ef5fa6cac51a --- /dev/null +++ b/maven-resolver-provider/src/test/resources/repo/org/apache/maven/its/dep-invalid-versions/maven-metadata.xml @@ -0,0 +1,34 @@ + + + + + + org.apache.maven.its + dep-invalid-versions + + 1.0 + 1.0 + + 1.0 + 1.0:2.0 + + 20120809112920 + + From ef74d160841becf2b294b636acd3f521cc3a00f4 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 22:24:45 +0200 Subject: [PATCH 5/7] Keep request and session repositories ahead of model-declared ones Repositories declared by a resolved model no longer replace a same-id repository supplied by the request or session; the ids supplied at resolver construction keep their precedence on the replace pass. A repository the model itself declared is still refreshed in place, e.g. once its URL has been interpolated. --- .../maven/project/ProjectModelResolver.java | 15 ++++++++++ .../project/ProjectModelResolverTest.java | 29 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java b/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java index 6857716547e2..2be9407121bc 100644 --- a/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java +++ b/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java @@ -74,6 +74,8 @@ public class ProjectModelResolver implements ModelResolver { private final Set repositoryIds; + private final Set externalRepositoryIds; + private final ReactorModelPool modelPool; private final ProjectBuildingRequest.RepositoryMerging repositoryMerging; @@ -96,6 +98,11 @@ public ProjectModelResolver( this.repositories.addAll(externalRepositories); this.repositoryMerging = repositoryMerging; this.repositoryIds = new HashSet<>(); + Set externalIds = new HashSet<>(); + for (RemoteRepository externalRepository : this.externalRepositories) { + externalIds.add(externalRepository.getId()); + } + this.externalRepositoryIds = Collections.unmodifiableSet(externalIds); this.modelPool = modelPool; } @@ -109,6 +116,7 @@ private ProjectModelResolver(ProjectModelResolver original) { this.repositories = new ArrayList<>(original.repositories); this.repositoryMerging = original.repositoryMerging; this.repositoryIds = new HashSet<>(original.repositoryIds); + this.externalRepositoryIds = original.externalRepositoryIds; this.modelPool = original.modelPool; } @@ -123,6 +131,13 @@ public void addRepository(final Repository repository, boolean replace) throws I return; } + if (externalRepositoryIds.contains(repository.getId())) { + // Replacement is meant to refresh a repository this model declared earlier, e.g. + // once its URL has been interpolated. Repositories supplied by the request or the + // session are not model-declared, so they keep precedence and are left in place. + return; + } + // Remove any previous repository with this Id removeMatchingRepository(repositories, repository.getId()); removeMatchingRepository(pomRepositories, repository.getId()); diff --git a/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java b/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java index 532a0404739f..4ade743ed2e4 100644 --- a/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java +++ b/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java @@ -27,6 +27,7 @@ import org.apache.maven.artifact.InvalidRepositoryException; import org.apache.maven.model.Dependency; import org.apache.maven.model.Parent; +import org.apache.maven.model.Repository; import org.apache.maven.model.resolution.ModelResolver; import org.apache.maven.model.resolution.UnresolvableModelException; import org.apache.maven.repository.internal.MavenRepositorySystemUtils; @@ -210,6 +211,34 @@ public void testResolveDependencySuccessfullyResolvesExistingDependencyUsingHigh assertEquals("1", dependency.getVersion()); } + @Test + public void testConstructionSuppliedRepositoryKeepsPrecedenceOverModelDeclaredRepositoryWithSameId() + throws Exception { + final ModelResolver resolver = this.newModelResolver(); + + // A model-declared repository that reuses the id of a repository supplied at construction + // time (here, the "central" entry from getRemoteRepositories()) but points elsewhere. + final Repository repository = new Repository(); + repository.setId(org.apache.maven.repository.RepositorySystem.DEFAULT_REMOTE_REPO_ID); + repository.setUrl(new File(getBasedir(), "target/no-such-repository") + .toURI() + .toURL() + .toString()); + + resolver.addRepository(repository); + resolver.addRepository(repository, true); + + final Parent parent = new Parent(); + parent.setGroupId("org.apache"); + parent.setArtifactId("apache"); + parent.setVersion("1"); + + // The construction-supplied repository kept its slot, so resolution against it still + // succeeds. + assertNotNull(resolver.resolveModel(parent)); + assertEquals("1", parent.getVersion()); + } + private ModelResolver newModelResolver() throws Exception { final DefaultRepositorySystemSession repoSession = MavenRepositorySystemUtils.newSession(); LocalRepositoryManager localRepositoryManager = From 70f7be9f5e9e0c15848b39f62d065317897edd11 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 22:35:44 +0200 Subject: [PATCH 6/7] Validate coordinate tokens adopted from plugin group and version metadata DefaultMetadataReader now checks parsed metadata the same way DefaultVersionResolver already does: version tokens (release, latest, versions, snapshot versions, snapshot timestamp) and plugin entries' artifactId and prefix must be valid coordinate components, rejecting "..", '/', '\\', ':' and ISO control characters. Callers that read metadata through this class already treat a read failure as invalid metadata, so a rejected document degrades gracefully instead of being adopted as a resolved version or plugin artifactId. --- .../metadata/io/DefaultMetadataReader.java | 65 +++++++++++++++++- .../io/DefaultMetadataReaderTest.java | 67 +++++++++++++++++++ .../metadata/io/invalid-plugin-artifactid.xml | 31 +++++++++ .../metadata/io/invalid-version-token.xml | 28 ++++++++ .../metadata/io/well-formed-metadata.xml | 39 +++++++++++ 5 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 maven-core/src/test/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReaderTest.java create mode 100644 maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-plugin-artifactid.xml create mode 100644 maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-version-token.xml create mode 100644 maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/well-formed-metadata.xml diff --git a/maven-core/src/main/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReader.java b/maven-core/src/main/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReader.java index 7ef68e334aa6..889d157dfbe8 100644 --- a/maven-core/src/main/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReader.java +++ b/maven-core/src/main/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReader.java @@ -26,6 +26,10 @@ import java.util.Objects; import org.apache.maven.artifact.repository.metadata.Metadata; +import org.apache.maven.artifact.repository.metadata.Plugin; +import org.apache.maven.artifact.repository.metadata.Snapshot; +import org.apache.maven.artifact.repository.metadata.SnapshotVersion; +import org.apache.maven.artifact.repository.metadata.Versioning; import org.apache.maven.artifact.repository.metadata.io.xpp3.MetadataXpp3Reader; import org.codehaus.plexus.component.annotations.Component; import org.codehaus.plexus.util.ReaderFactory; @@ -51,7 +55,9 @@ public Metadata read(Reader input, Map options) throws IOException { Objects.requireNonNull(input, "input cannot be null"); try (Reader in = input) { - return new MetadataXpp3Reader().read(in, isStrict(options)); + Metadata metadata = new MetadataXpp3Reader().read(in, isStrict(options)); + validateMetadata(metadata); + return metadata; } catch (XmlPullParserException e) { throw new MetadataParseException(e.getMessage(), e.getLineNumber(), e.getColumnNumber(), e); } @@ -61,7 +67,9 @@ public Metadata read(InputStream input, Map options) throws IOExcepti Objects.requireNonNull(input, "input cannot be null"); try (InputStream in = input) { - return new MetadataXpp3Reader().read(in, isStrict(options)); + Metadata metadata = new MetadataXpp3Reader().read(in, isStrict(options)); + validateMetadata(metadata); + return metadata; } catch (XmlPullParserException e) { throw new MetadataParseException(e.getMessage(), e.getLineNumber(), e.getColumnNumber(), e); } @@ -71,4 +79,57 @@ private boolean isStrict(Map options) { Object value = (options != null) ? options.get(IS_STRICT) : null; return value == null || Boolean.parseBoolean(value.toString()); } + + /** + * Coordinate-shaped tokens read from this metadata (versions, plugin artifactIds and prefixes) get carried + * forward by callers as if they were already-validated path and coordinate components. Reject anything that + * would not itself be a valid coordinate component here, before it leaves this reader. + */ + private static void validateMetadata(Metadata metadata) throws IOException { + if (metadata == null) { + return; + } + + Versioning versioning = metadata.getVersioning(); + if (versioning != null) { + validateToken("version", versioning.getRelease()); + validateToken("version", versioning.getLatest()); + for (String version : versioning.getVersions()) { + validateToken("version", version); + } + for (SnapshotVersion snapshotVersion : versioning.getSnapshotVersions()) { + validateToken("version", snapshotVersion.getVersion()); + } + Snapshot snapshot = versioning.getSnapshot(); + if (snapshot != null) { + validateToken("snapshot timestamp", snapshot.getTimestamp()); + } + } + + if (metadata.getPlugins() != null) { + for (Plugin plugin : metadata.getPlugins()) { + validateToken("plugin artifactId", plugin.getArtifactId()); + validateToken("plugin prefix", plugin.getPrefix()); + } + } + } + + private static void validateToken(String field, String value) throws IOException { + if (value == null || value.isEmpty()) { + return; + } + boolean valid = !"..".equals(value); + if (valid) { + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c == '/' || c == '\\' || c == ':' || Character.isISOControl(c)) { + valid = false; + break; + } + } + } + if (!valid) { + throw new IOException("Metadata contains an invalid " + field + ": '" + value + "'"); + } + } } diff --git a/maven-core/src/test/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReaderTest.java b/maven-core/src/test/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReaderTest.java new file mode 100644 index 000000000000..f80da9ba2ac2 --- /dev/null +++ b/maven-core/src/test/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReaderTest.java @@ -0,0 +1,67 @@ +/* + * 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.artifact.repository.metadata.io; + +import java.io.File; +import java.io.IOException; +import java.net.URISyntaxException; +import java.util.Collections; + +import org.apache.maven.artifact.repository.metadata.Metadata; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class DefaultMetadataReaderTest { + + private final DefaultMetadataReader reader = new DefaultMetadataReader(); + + private File resource(String name) throws URISyntaxException { + return new File(getClass().getResource(name).toURI()); + } + + @Test + public void testWellFormedMetadataParsesUnchanged() throws Exception { + Metadata metadata = reader.read(resource("well-formed-metadata.xml"), Collections.emptyMap()); + + assertEquals("org.apache.maven.its", metadata.getGroupId()); + assertEquals("sample", metadata.getArtifactId()); + assertEquals("1.1", metadata.getVersioning().getRelease()); + assertEquals("1.1", metadata.getVersioning().getLatest()); + assertEquals("maven-sample-plugin", metadata.getPlugins().get(0).getArtifactId()); + } + + @Test + public void testVersionContainingColonIsRejected() throws Exception { + File input = resource("invalid-version-token.xml"); + + IOException e = assertThrows(IOException.class, () -> reader.read(input, Collections.emptyMap())); + assertTrue(e.getMessage().contains("1.0:evil")); + } + + @Test + public void testPluginArtifactIdContainingSlashIsRejected() throws Exception { + File input = resource("invalid-plugin-artifactid.xml"); + + IOException e = assertThrows(IOException.class, () -> reader.read(input, Collections.emptyMap())); + assertTrue(e.getMessage().contains("maven/sample-plugin")); + } +} diff --git a/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-plugin-artifactid.xml b/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-plugin-artifactid.xml new file mode 100644 index 000000000000..aec5e1aa04f3 --- /dev/null +++ b/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-plugin-artifactid.xml @@ -0,0 +1,31 @@ + + + + org.apache.maven.its + sample + + + Sample Plugin + sample + + maven/sample-plugin + + + diff --git a/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-version-token.xml b/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-version-token.xml new file mode 100644 index 000000000000..14216e28319d --- /dev/null +++ b/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/invalid-version-token.xml @@ -0,0 +1,28 @@ + + + + org.apache.maven.its + sample + + + 1.0:evil + 20150428055824 + + diff --git a/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/well-formed-metadata.xml b/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/well-formed-metadata.xml new file mode 100644 index 000000000000..ce3b4fa9365a --- /dev/null +++ b/maven-core/src/test/resources/org/apache/maven/artifact/repository/metadata/io/well-formed-metadata.xml @@ -0,0 +1,39 @@ + + + + org.apache.maven.its + sample + + 1.1 + 1.1 + + 1.0 + 1.1 + + 20150428055824 + + + + Sample Plugin + sample + maven-sample-plugin + + + From a1bed86e49ca837394e64d219290650ef5f42412 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 23:17:57 +0200 Subject: [PATCH 7/7] Revert "Keep request and session repositories ahead of model-declared ones" This reverts commit ef74d16084. ProjectModelResolver tracks in repositoryIds only the ids a model has declared, so the replace path is reached only for an id this model registered earlier. Guarding it on the id also being present in the externally supplied list therefore never protected an externally supplied repository; it only blocked a model from refreshing its own declaration when the two ids happened to match. MavenITmng5639ImportScopePomResolutionTest does exactly that: it declares central in the POM and relies on the refreshed URL once interpolation has run. --- .../maven/project/ProjectModelResolver.java | 15 ---------- .../project/ProjectModelResolverTest.java | 29 ------------------- 2 files changed, 44 deletions(-) diff --git a/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java b/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java index 2be9407121bc..6857716547e2 100644 --- a/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java +++ b/maven-core/src/main/java/org/apache/maven/project/ProjectModelResolver.java @@ -74,8 +74,6 @@ public class ProjectModelResolver implements ModelResolver { private final Set repositoryIds; - private final Set externalRepositoryIds; - private final ReactorModelPool modelPool; private final ProjectBuildingRequest.RepositoryMerging repositoryMerging; @@ -98,11 +96,6 @@ public ProjectModelResolver( this.repositories.addAll(externalRepositories); this.repositoryMerging = repositoryMerging; this.repositoryIds = new HashSet<>(); - Set externalIds = new HashSet<>(); - for (RemoteRepository externalRepository : this.externalRepositories) { - externalIds.add(externalRepository.getId()); - } - this.externalRepositoryIds = Collections.unmodifiableSet(externalIds); this.modelPool = modelPool; } @@ -116,7 +109,6 @@ private ProjectModelResolver(ProjectModelResolver original) { this.repositories = new ArrayList<>(original.repositories); this.repositoryMerging = original.repositoryMerging; this.repositoryIds = new HashSet<>(original.repositoryIds); - this.externalRepositoryIds = original.externalRepositoryIds; this.modelPool = original.modelPool; } @@ -131,13 +123,6 @@ public void addRepository(final Repository repository, boolean replace) throws I return; } - if (externalRepositoryIds.contains(repository.getId())) { - // Replacement is meant to refresh a repository this model declared earlier, e.g. - // once its URL has been interpolated. Repositories supplied by the request or the - // session are not model-declared, so they keep precedence and are left in place. - return; - } - // Remove any previous repository with this Id removeMatchingRepository(repositories, repository.getId()); removeMatchingRepository(pomRepositories, repository.getId()); diff --git a/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java b/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java index 4ade743ed2e4..532a0404739f 100644 --- a/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java +++ b/maven-core/src/test/java/org/apache/maven/project/ProjectModelResolverTest.java @@ -27,7 +27,6 @@ import org.apache.maven.artifact.InvalidRepositoryException; import org.apache.maven.model.Dependency; import org.apache.maven.model.Parent; -import org.apache.maven.model.Repository; import org.apache.maven.model.resolution.ModelResolver; import org.apache.maven.model.resolution.UnresolvableModelException; import org.apache.maven.repository.internal.MavenRepositorySystemUtils; @@ -211,34 +210,6 @@ public void testResolveDependencySuccessfullyResolvesExistingDependencyUsingHigh assertEquals("1", dependency.getVersion()); } - @Test - public void testConstructionSuppliedRepositoryKeepsPrecedenceOverModelDeclaredRepositoryWithSameId() - throws Exception { - final ModelResolver resolver = this.newModelResolver(); - - // A model-declared repository that reuses the id of a repository supplied at construction - // time (here, the "central" entry from getRemoteRepositories()) but points elsewhere. - final Repository repository = new Repository(); - repository.setId(org.apache.maven.repository.RepositorySystem.DEFAULT_REMOTE_REPO_ID); - repository.setUrl(new File(getBasedir(), "target/no-such-repository") - .toURI() - .toURL() - .toString()); - - resolver.addRepository(repository); - resolver.addRepository(repository, true); - - final Parent parent = new Parent(); - parent.setGroupId("org.apache"); - parent.setArtifactId("apache"); - parent.setVersion("1"); - - // The construction-supplied repository kept its slot, so resolution against it still - // succeeds. - assertNotNull(resolver.resolveModel(parent)); - assertEquals("1", parent.getVersion()); - } - private ModelResolver newModelResolver() throws Exception { final DefaultRepositorySystemSession repoSession = MavenRepositorySystemUtils.newSession(); LocalRepositoryManager localRepositoryManager =