Validate coordinates and repository precedence during artifact resolution

compat/maven-compat/src/main/java/org/apache/maven/project/artifact/MavenMetadataSource.java

@@ -595,16 +595,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;
@@ -668,6 +671,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 mbe) {
             for (ModelProblem problem : mbe.getProblems()) {

compat/maven-compat/src/test/java/org/apache/maven/project/artifact/MavenMetadataSourceRelocationTest.java

@@ -0,0 +1,167 @@
+/*
+ * 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.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.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) {
+        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);
+
+        return new MavenMetadataSource(
+                mock(RepositoryMetadataManager.class),
+                artifactFactory,
+                projectBuilder,
+                mock(MavenMetadataCache.class),
+                legacySupport,
+                mock(MavenRepositorySystem.class));
+    }
+
+    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 testRelocationInvalidArtifactIdIsRejected() 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());
+    }
+}

compat/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultModelResolver.java

@@ -76,6 +76,8 @@ class DefaultModelResolver implements ModelResolver {
 
     private final Set<String> repositoryIds;
 
+    private final Set<String> externalRepositoryIds;
+
     DefaultModelResolver(
             RepositorySystemSession session,
             RequestTrace trace,
@@ -94,6 +96,11 @@ class DefaultModelResolver implements ModelResolver {
         this.externalRepositories = Collections.unmodifiableList(new ArrayList<>(repositories));
 
         this.repositoryIds = new HashSet<>();
+        Set<String> externalIds = new HashSet<>();
+        for (RemoteRepository externalRepository : this.externalRepositories) {
+            externalIds.add(externalRepository.getId());
+        }
+        this.externalRepositoryIds = Collections.unmodifiableSet(externalIds);
     }
 
     private DefaultModelResolver(DefaultModelResolver original) {
@@ -106,6 +113,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
@@ -124,6 +132,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());
         }
 

compat/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionRangeResolver.java

@@ -22,6 +22,7 @@
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
 import java.util.ArrayList;
@@ -235,8 +236,12 @@ private Versioning readVersions(
 
                     if (metadata.getPath() != null && Files.exists(metadata.getPath())) {
                         try (InputStream in = Files.newInputStream(metadata.getPath())) {
-                            versioning = new Versioning(
+                            Versioning parsed = new Versioning(
                                     new MetadataStaxReader().read(in, false).getVersioning());
+
+                            validateVersioning(parsed);
+
+                            versioning = parsed;
                         }
                     }
                 }
@@ -249,6 +254,35 @@ 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, "version");
+        }
+        validateVersionToken(versioning.getLatest(), "latest version");
+        validateVersionToken(versioning.getRelease(), "release version");
+    }
+
+    private static void validateVersionToken(String value, String description) throws IOException {
+        if (value == null || value.isEmpty()) {
+            return;
+        }
+        boolean invalid = "..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":");
+        for (int i = 0; i < value.length() && !invalid; i++) {
+            invalid = Character.isISOControl(value.charAt(i));
+        }
+        if (invalid) {
+            throw new IOException("Rejecting metadata with invalid " + description + " '" + value
+                    + "': must not contain '..', '/', '\\', ':' or control characters");
+        }
+    }
+
     private Versioning filterVersionsByRepositoryType(Versioning versioning, RemoteRepository remoteRepository) {
         if (remoteRepository == null) {
             return versioning;

compat/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/DefaultVersionResolver.java

@@ -245,9 +245,13 @@ private Versioning readVersions(
 
                     if (metadata.getPath() != null && Files.exists(metadata.getPath())) {
                         try (InputStream in = Files.newInputStream(metadata.getPath())) {
-                            versioning = new Versioning(
+                            Versioning parsed = new Versioning(
                                     new MetadataStaxReader().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
@@ -278,6 +282,39 @@ 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(), "latest version");
+        validateVersionToken(versioning.getRelease(), "release version");
+        for (SnapshotVersion snapshotVersion : versioning.getSnapshotVersions()) {
+            validateVersionToken(snapshotVersion.getVersion(), "snapshot version");
+        }
+        Snapshot snapshot = versioning.getSnapshot();
+        if (snapshot != null) {
+            validateVersionToken(snapshot.getTimestamp(), "snapshot timestamp");
+        }
+    }
+
+    private static void validateVersionToken(String value, String description) throws IOException {
+        if (value == null || value.isEmpty()) {
+            return;
+        }
+        boolean invalid = "..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":");
+        for (int i = 0; i < value.length() && !invalid; i++) {
+            invalid = Character.isISOControl(value.charAt(i));
+        }
+        if (invalid) {
+            throw new IOException("Rejecting metadata with invalid " + description + " '" + value
+                    + "': must not contain '..', '/', '\\', ':' or control characters");
+        }
+    }
+
     private void invalidMetadata(
             RepositorySystemSession session,
             RequestTrace trace,