Honor system proxy credentials in Apache transport - #2148
goutamadwant wants to merge 3 commits into
Conversation
Use matching HTTP or HTTPS proxy credentials under the existing system-properties opt-in while preserving explicit Resolver authentication precedence. Keep fallback credentials scoped to proxy challenges and clear their authentication state when a redirect changes the proxy route. Add native transfer, CONNECT, precedence, origin-isolation and redirect regression tests. Related to apache#2139.
gnodet-bot
left a comment
gnodet-bot
left a comment
There was a problem hiding this comment.
Solid bug fix with comprehensive tests. The credential scoping via child HttpContext in select() is correct and the http-first / https-second fallback logic is intentional and works correctly with the test harness.
Two findings below.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
| /** | ||
| * Adds system proxy credentials only while selecting authentication for the current route's proxy. | ||
| */ | ||
| final class SystemProxyAuthenticationStrategy extends ProxyAuthenticationStrategy implements HttpRequestInterceptor { |
There was a problem hiding this comment.
Nit: Missing @since tag. OriginScopedHeadersInterceptor (added in the same 2.0.23 release cycle, same package) sets the precedent — even package-private classes carry @since here.
| final class SystemProxyAuthenticationStrategy extends ProxyAuthenticationStrategy implements HttpRequestInterceptor { | |
| * Adds system proxy credentials only while selecting authentication for the current route's proxy. | |
| * | |
| * @since 2.0.23 | |
| */ |
| credentials.setCredentials( | ||
| new AuthScope(proxy, AuthScope.ANY_REALM, AuthSchemes.NTLM), | ||
| new NTCredentials(username, password, null, System.getProperty("http.auth.ntlm.domain"))); | ||
| return credentials; |
There was a problem hiding this comment.
Informational: http.auth.ntlm.domain is used regardless of protocol. Apache HC4 only defines this one property (no https.auth.ntlm.domain counterpart), so this is consistent with library conventions — but a short inline comment would save the next maintainer from wondering whether the hard-coded "http." prefix is a bug.
Signed-off-by: goutamadwant <workwithgoutam@gmail.com>
Signed-off-by: goutamadwant <workwithgoutam@gmail.com>
2 participants
Related to #2139.
With
aether.connector.http.useSystemProperties=true, the Apache transport uses the configured proxy but ignores its username and password, causing HTTP 407. Use matching HTTP or HTTPS system proxy credentials when explicit Resolver authentication is unavailable.Keep fallback credentials scoped to the actual proxy, separate from repository authentication. Clear their authentication state when a redirect changes proxy or becomes direct. The opt-in remains disabled by default; this adds no public API or runtime dependency.
Tests cover GET/HEAD/PUT, HTTPS CONNECT, explicit credential precedence, missing or mismatched properties, origin isolation, and redirects. The native regression fails with HTTP 407 on unchanged master.
Validation on Java 21 with Maven 3.9.16:
mvn verify: all 31 modules passed; 2,082 tests, zero failures/errors, 87 existing skips.mvn -Prun-its verify: passed, including 107 integration tests and one Invoker build.This targets Resolver 2.x. The corresponding 1.9.x backport covers the lineage used by Maven 3.9.16.
mvn verifypassed.mvn -Prun-its verifypassed.