Resolver 2.0.23-SNAPSHOT and validation cleanup by cstamas · Pull Request #13078 · apache/maven

cstamas · 2026-09-08T10:01:33Z

We had overlapping changes in Resolver and Maven, align them.

ascheman · 2026-09-08T10:21:22Z

Sure about the SNAPSHOT-dependency, @cstamas ? Or will you resolve this before you make the draft final?

cstamas · 2026-09-08T10:24:48Z

We usually run "main" branches (in quotes, as we have several "main" branches, the master, the maven-4.0.x, the maven-3.10.x, etc) just before the release with SNAPSHOT versions, as we do produce and deploy them, is fine to get another layer of confirmation. OTOH, you cannot release anyway with a SNAPSHOT, hence, this is completely okay, and we did it before as well.

cstamas · 2026-09-08T10:33:15Z

So, all of ITs explode, reason is f013 in this commit:
apache/maven-resolver@339161b

@gnodet ping

gnodet · 2026-09-08T11:32:32Z

@cstamas The root cause is the tracking key function change from nid to nid_hurl in commit 339161b (f013). All existing _remote.repositories entries use ID-only keys (e.g. artifact>central=), but the new resolver looks for URL-qualified keys (artifact>central-<sha1>=). No match → every cached artifact appears "present but unavailable" → re-download from the IT's fake repos → 💥

This affects all 9 IT jobs (4229 test failures, 507 unique test classes on ubuntu/JDK26 alone). The existenceCheckRelabel change (f005) is secondary — even with the old existence-check behavior, the tracking key mismatch would cause failures because the fake repos can't serve the artifacts either way.

Fix PR: apache/maven-resolver#2133 — adds a backward-compatible fallback in applyTracking(): when the nid_hurl lookup misses, tries the system-wide key function (nid) as fallback. If the legacy key matches, the artifact is accepted with a debug log. The entry gets upgraded to nid_hurl on next download. Security intent of f013 is preserved for new entries.

…allback (#2133) ## Summary When the tracking key function is URL-qualified (`nid_hurl`, the default since 2.0.23 / commit 339161b), tracking entries written by an older resolver using ID-only keys (`nid` format, e.g. `artifact>central=`) are invisible to the new lookup which expects `artifact>central-<sha1>=`. This causes **all** artifacts cached in the local repository before the upgrade to appear as "present but unavailable", triggering full re-downloads from remote repositories. In CI environments and integration tests that use fake/file-based repositories (like maven-integration-testing), this breaks resolution entirely — see [apache/maven#13078 (comment)](apache/maven#13078 (comment)). ## Root Cause The tracking key change from `nid` to `nid_hurl` (finding f013 in the security audit) makes existing `_remote.repositories` entries invisible: - **Old entry**: `maven-core-3.8.6.pom>central=` - **New lookup**: `maven-core-3.8.6.pom>central-<sha1 of repo URL>=` → no match The `isTracked()` check finds the old `central=` entry, so the artifact is treated as "tracked but not for the current repo" rather than "untracked" — the untracked inter-op fallback does not apply, and the artifact is rejected. ## Fix Add a backward-compatible fallback in `applyTracking()`: when the URL-qualified (`nid_hurl`) lookup misses, try the system-wide key function (`nid` by default) as a fallback. If that matches, accept the artifact and log a debug message. The legacy entry will be upgraded to the new key format on the next download. This preserves the f013 security improvement (URL-qualified tracking prevents same-id-different-URL repository poisoning for *new* entries) while providing a smooth upgrade path from older resolvers. ## Tests - Updated `testUrlQualifiedTrackingTreatsLegacyIdOnlyEntriesAsStale` → `testUrlQualifiedTrackingAcceptsLegacyIdOnlyEntriesViaFallback`: legacy entries from a matching repo are now accepted - Added `testUrlQualifiedTrackingRejectsLegacyIdOnlyEntriesFromDifferentRepo`: legacy entries from a *different* repo are still rejected - `testUrlQualifiedTrackingDistinguishesSameIdDifferentUrl`: unchanged — URL-qualified entries still prevent same-id-different-URL poisoning

gnodet · 2026-09-08T16:14:59Z

Updated the resolver fix PR based on @cstamas's analysis. The first fix only handled legacy nid → nid_hurl migration, but the real problem is different:

The ITs override central to file:target/null to prevent remote access. With nid_hurl, the tracking key includes a hash of the repo URL: central-<sha1(https://repo.maven.apache.org/maven2)> ≠ central-<sha1(file:target/null)>. Same repo ID, different URL hash → tracking key mismatch → artifact treated as "present but unavailable" → tries to download from file:target/null → 💥

The fix now has a two-stage fallback:

Stage 1: ID-only match (handles legacy nid entries from older resolvers)
Stage 2: repo-ID prefix match (handles same-id-different-URL nid_hurl entries — the IT scenario)

This effectively relaxes f013 to recording-only: entries are written with URL-qualified keys, but lookups fall back to repo-ID matching. All 585 resolver tests pass.

Fix PR: apache/maven-resolver#2133 (force-pushed with the updated fix)

ascheman · 2026-09-09T07:26:57Z

@cstamas — the SNAPSHOT and IT-explosion questions are resolved on my side (thanks, and to @gnodet for #2133). One code point from the validation consolidation itself, independent of the IT fix:

The removed DefaultMetadataReader.validateMetadata() validated both plugin.getArtifactId() and plugin.getPrefix(), but the consolidated ValidatingMetadataXpp3Reader.validate() adds only plugin.getArtifactId() — getPrefix() is no longer validated. Since the plugin prefix is carried forward as a coordinate/path component (the old reader's comment flagged prefixes as such), is dropping its validation intentional, or should it get a PathUtils.validatePathComponent(plugin.getPrefix(), "plugin/prefix") alongside the artifactId?

Two minors: validate() went public → private (fine — internal package, no external caller left), and there's no test yet exercising the plugin-token validation path.

gnodet

Review: validation consolidation looks correct, one gap to fix

The consolidation from two separate validation sites (DefaultMetadataReader.validateMetadata() and DefaultRepositoryMetadataManager.validateVersioning()) into the single ValidatingMetadataXpp3Reader.validate() is clean — it eliminates the duplicate validateVersioning() call and the now-redundant per-reader validation logic.

The new validate() method correctly covers versioning tokens (latest, release, versions[], snapshotVersions[].version, snapshot.timestamp) and plugin.getArtifactId(). The snapshot timestamp validation and metadata.getVersion() validation are new additions not present in either old reader — good.

However, as @ascheman noted: plugin.getPrefix() validation was dropped. The removed DefaultMetadataReader.validateMetadata() explicitly validated both plugin.getArtifactId() and plugin.getPrefix(), with its Javadoc stating these "get carried forward by callers as if they were already-validated path and coordinate components." The consolidated reader only validates plugin.getArtifactId().

Fix: add PathUtils.validatePathComponent(plugin.getPrefix(), "plugin/prefix"); alongside the artifactId validation in ValidatingMetadataXpp3Reader.validate().

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of @gnodet

gnodet · 2026-09-09T14:57:33Z

            PathUtils.validatePathComponent(metadata.getVersion(), "version");
+            for (Plugin plugin : metadata.getPlugins()) {
+                PathUtils.validatePathComponent(plugin.getArtifactId(), "plugin/artifactId");
+            }


@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");

Resolver 2.0.23-SNAPSHOT and validation cleanup

fa77334

We had overlapping changes in Resolver and Maven, align them.

cstamas added this to the 3.10.0 milestone Sep 8, 2026

cstamas requested review from ascheman, gnodet and slachiewicz September 8, 2026 10:01

cstamas self-assigned this Sep 8, 2026

cstamas added the dependencies Pull requests that update a dependency file label Sep 8, 2026

gnodet mentioned this pull request Sep 8, 2026

fix: accept locally cached artifacts via same-id fallback when tracking URL changes apache/maven-resolver#2133

Merged

gnodet mentioned this pull request Sep 8, 2026

fix: extend tracking fallback to accept same-id-different-URL entries apache/maven-resolver#2134

Open

slachiewicz removed their request for review September 9, 2026 14:53

gnodet reviewed Sep 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

Resolver 2.0.23-SNAPSHOT and validation cleanup - #13078

Resolver 2.0.23-SNAPSHOT and validation cleanup#13078
cstamas wants to merge 1 commit into
apache:maven-3.10.xfrom
cstamas:maven-3.10.x-resolver-2023

cstamas commented Sep 8, 2026 •

edited

Loading

Uh oh!

ascheman commented Sep 8, 2026

Uh oh!

cstamas commented Sep 8, 2026

Uh oh!

cstamas commented Sep 8, 2026 •

edited

Loading

Uh oh!

gnodet commented Sep 8, 2026

Uh oh!

gnodet commented Sep 8, 2026

Uh oh!

ascheman commented Sep 9, 2026

Uh oh!

gnodet left a comment

Uh oh!

gnodet Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

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

Uh oh!

Conversation

cstamas commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ascheman commented Sep 8, 2026

Uh oh!

cstamas commented Sep 8, 2026

Uh oh!

cstamas commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnodet commented Sep 8, 2026

Uh oh!

gnodet commented Sep 8, 2026

Uh oh!

ascheman commented Sep 9, 2026

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

gnodet Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cstamas commented Sep 8, 2026 •

edited

Loading

cstamas commented Sep 8, 2026 •

edited

Loading