Skip to content

Resolver 2.0.23-SNAPSHOT and validation cleanup #13078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cstamas wants to merge 1 commit into
base: maven-3.10.x
Choose a base branch
from
Draft

Resolver 2.0.23-SNAPSHOT and validation cleanup #13078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 0 additions & 49 deletions .../java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,65 +285,16 @@ 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) {
throw new RepositoryMetadataReadException(
"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.
Expand Down
4 changes: 2 additions & 2 deletions ...a/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions ...he/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
69 changes: 3 additions & 66 deletions ...src/main/java/org/apache/maven/artifact/repository/metadata/io/DefaultMetadataReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -49,18 +45,14 @@ public class DefaultMetadataReader implements MetadataReader {
public Metadata read(File input, Map<String, ?> 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<String, ?> 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);
}
Expand All @@ -70,9 +62,7 @@ public Metadata read(InputStream input, Map<String, ?> 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);
}
Expand All @@ -82,57 +72,4 @@ private boolean isStrict(Map<String, ?> 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 + "'");
}
}
}
28 changes: 23 additions & 5 deletions ...main/java/org/apache/maven/repository/internal/metadata/ValidatingMetadataXpp3Reader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -40,33 +42,49 @@ 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");
}

@gnodet gnodet Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ascheman's observation is correct — plugin.getPrefix() validation was present in the removed DefaultMetadataReader.validateMetadata() but was not carried over here. Since prefixes are used as coordinate/path components downstream, this should be:

Suggested change
}
PathUtils.validatePathComponent(plugin.getArtifactId(), "plugin/artifactId");
PathUtils.validatePathComponent(plugin.getPrefix(), "plugin/prefix");

Versioning versioning = metadata.getVersioning();
if (versioning != null) {
PathUtils.validatePathComponent(versioning.getLatest(), "versioning/latest");
PathUtils.validatePathComponent(versioning.getRelease(), "versioning/release");
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");
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ under the License.
<!-- plexus-cipher 2.1.0 uses an incompatible KDF; ciphertext encrypted with 2.0 will no longer decrypt — see #12811. Do not bump on this branch. -->
<cipherVersion>2.0</cipherVersion>
<jxpathVersion>1.4.0</jxpathVersion>
<resolverVersion>2.0.22</resolverVersion>
<resolverVersion>2.0.23-SNAPSHOT</resolverVersion>
<slf4jVersion>2.0.19</slf4jVersion>
<xmlunitVersion>2.13.0</xmlunitVersion>
<powermockVersion>2.0.9</powermockVersion>
Expand Down
Loading