diff --git a/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java b/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java index a22500bdf5e4..477f1e65a75d 100644 --- a/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java +++ b/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java @@ -285,8 +285,6 @@ protected Metadata readMetadata(File mappingFile) throws RepositoryMetadataReadE ValidatingMetadataXpp3Reader mappingReader = new ValidatingMetadataXpp3Reader(); result = mappingReader.read(reader, false); - - validateVersioning(result); } catch (FileNotFoundException e) { throw new RepositoryMetadataReadException("Cannot read metadata from '" + mappingFile + "'", e); } catch (IOException | XmlPullParserException e) { @@ -294,56 +292,9 @@ protected Metadata readMetadata(File mappingFile) throws RepositoryMetadataReadE "Cannot read metadata from '" + mappingFile + "': " + e.getMessage(), e); } - validateVersioning(result); - return result; } - /** - * Version tokens adopted from repository metadata must be valid coordinate components; metadata carrying - * anything else is treated as invalid. - */ - private static void validateVersioning(Metadata metadata) throws RepositoryMetadataReadException { - if (metadata == null) { - return; - } - Versioning versioning = metadata.getVersioning(); - if (versioning == null) { - return; - } - validateVersionToken(versioning.getLatest()); - validateVersionToken(versioning.getRelease()); - for (String version : versioning.getVersions()) { - validateVersionToken(version); - } - 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 RepositoryMetadataReadException { - 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 RepositoryMetadataReadException("Metadata contains an invalid version token: '" + value + "'"); - } - } - /** * Ensures the last updated timestamp of the specified metadata does not refer to the future and fixes the local * metadata if necessary to allow proper merging/updating of metadata during deployment. diff --git a/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java b/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java index c2e6615dccad..12e53f17e1a3 100644 --- a/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java +++ b/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java @@ -93,7 +93,7 @@ void testMetadataWithInvalidVersionTokenIsRejected() { RepositoryMetadataReadException exception = assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile)); - assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage()); + assertTrue(exception.getMessage().contains("Invalid versioning/release"), exception.getMessage()); } @Test @@ -103,7 +103,7 @@ void testMetadataWithInvalidSnapshotTimestampIsRejected() { RepositoryMetadataReadException exception = assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile)); - assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage()); + assertTrue(exception.getMessage().contains("Invalid versioning/snapshot/timestamp"), exception.getMessage()); } private static File testFile(String resource) { diff --git a/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java b/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java index 2167606b810b..e2a72be2b701 100644 --- a/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java +++ b/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java @@ -42,7 +42,7 @@ void testMetadataWithInvalidVersionTokenIsRejected() { RepositoryMetadataReadException exception = assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile)); - assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage()); + assertTrue(exception.getMessage().contains("Invalid versioning/release"), exception.getMessage()); } @Test @@ -52,7 +52,7 @@ void testMetadataWithInvalidSnapshotTimestampIsRejected() { RepositoryMetadataReadException exception = assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile)); - assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage()); + assertTrue(exception.getMessage().contains("Invalid versioning/snapshot/timestamp"), exception.getMessage()); } private static File testFile(String resource) { 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 d4a94851b788..d674fec281d4 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 @@ -29,10 +29,6 @@ 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.repository.internal.metadata.ValidatingMetadataXpp3Reader; import org.codehaus.plexus.util.ReaderFactory; import org.codehaus.plexus.util.xml.pull.XmlPullParserException; @@ -49,18 +45,14 @@ public class DefaultMetadataReader implements MetadataReader { public Metadata read(File input, Map options) throws IOException { Objects.requireNonNull(input, "input cannot be null"); - Metadata metadata = read(ReaderFactory.newXmlReader(input), options); - - return metadata; + return read(ReaderFactory.newXmlReader(input), options); } public Metadata read(Reader input, Map options) throws IOException { Objects.requireNonNull(input, "input cannot be null"); try (Reader in = input) { - Metadata metadata = new ValidatingMetadataXpp3Reader().read(in, isStrict(options)); - validateMetadata(metadata); - return metadata; + return new ValidatingMetadataXpp3Reader().read(in, isStrict(options)); } catch (XmlPullParserException e) { throw new MetadataParseException(e.getMessage(), e.getLineNumber(), e.getColumnNumber(), e); } @@ -70,9 +62,7 @@ public Metadata read(InputStream input, Map options) throws IOExcepti Objects.requireNonNull(input, "input cannot be null"); try (InputStream in = input) { - Metadata metadata = new ValidatingMetadataXpp3Reader().read(in, isStrict(options)); - validateMetadata(metadata); - return metadata; + return new ValidatingMetadataXpp3Reader().read(in, isStrict(options)); } catch (XmlPullParserException e) { throw new MetadataParseException(e.getMessage(), e.getLineNumber(), e.getColumnNumber(), e); } @@ -82,57 +72,4 @@ 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-resolver-provider/src/main/java/org/apache/maven/repository/internal/metadata/ValidatingMetadataXpp3Reader.java b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/metadata/ValidatingMetadataXpp3Reader.java index 338befe1be1b..e7ef16732e7d 100644 --- a/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/metadata/ValidatingMetadataXpp3Reader.java +++ b/maven-resolver-provider/src/main/java/org/apache/maven/repository/internal/metadata/ValidatingMetadataXpp3Reader.java @@ -23,6 +23,8 @@ import java.io.Reader; import org.apache.maven.artifact.repository.metadata.Metadata; +import org.apache.maven.artifact.repository.metadata.Plugin; +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.util.xml.pull.XmlPullParserException; @@ -40,22 +42,33 @@ public final class ValidatingMetadataXpp3Reader { * Delegates to {@link MetadataXpp3Reader#read(Reader, boolean)} */ public Metadata read(Reader reader, boolean strict) throws IOException, XmlPullParserException { - return validate(mr.read(reader, strict)); + try { + return validate(mr.read(reader, strict)); + } catch (IllegalArgumentException e) { + throw new IOException("Invalid metadata detected: " + e.getMessage(), e); + } } /** * Delegates to {@link MetadataXpp3Reader#read(InputStream, boolean)} */ public Metadata read(InputStream in, boolean strict) throws IOException, XmlPullParserException { - return validate(mr.read(in, strict)); + try { + return validate(mr.read(in, strict)); + } catch (IllegalArgumentException e) { + throw new IOException("Invalid metadata detected: " + e.getMessage(), e); + } } /** * Validates {@link Metadata}. */ - public static Metadata validate(Metadata metadata) { + private static Metadata validate(Metadata metadata) { if (metadata != null) { PathUtils.validatePathComponent(metadata.getVersion(), "version"); + for (Plugin plugin : metadata.getPlugins()) { + PathUtils.validatePathComponent(plugin.getArtifactId(), "plugin/artifactId"); + } Versioning versioning = metadata.getVersioning(); if (versioning != null) { PathUtils.validatePathComponent(versioning.getLatest(), "versioning/latest"); @@ -63,10 +76,15 @@ public static Metadata validate(Metadata metadata) { for (int i = 0; i < versioning.getVersions().size(); i++) { PathUtils.validatePathComponent(versioning.getVersions().get(i), "versioning/versions[" + i + "]"); } + if (versioning.getSnapshot() != null) { + PathUtils.validatePathComponent( + versioning.getSnapshot().getTimestamp(), "versioning/snapshot/timestamp"); + } for (int i = 0; i < versioning.getSnapshotVersions().size(); i++) { + SnapshotVersion snapshotVersion = + versioning.getSnapshotVersions().get(i); PathUtils.validatePathComponent( - versioning.getSnapshotVersions().get(i).getVersion(), - "versioning/snapshotVersions[" + i + "]/version"); + snapshotVersion.getVersion(), "versioning/snapshotVersions[" + i + "]/version"); } } } diff --git a/pom.xml b/pom.xml index b2f2ba4d17cf..08bfbd49f02b 100644 --- a/pom.xml +++ b/pom.xml @@ -145,7 +145,7 @@ under the License. 2.0 1.4.0 - 2.0.22 + 2.0.23-SNAPSHOT 2.0.19 2.13.0 2.0.9