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/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/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/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-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 + + + 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/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/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/DefaultArtifactDescriptorReaderRelocationValidationTest.java b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java new file mode 100644 index 000000000000..5a53640066c8 --- /dev/null +++ b/maven-resolver-provider/src/test/java/org/apache/maven/repository/internal/DefaultArtifactDescriptorReaderRelocationValidationTest.java @@ -0,0 +1,109 @@ +/* + * 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")); + } + + @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/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, 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 + + 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 + + + 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 + + +