New transport: apache5x by cstamas · Pull Request #742 · apache/maven-resolver

cstamas · 2025-06-09T12:39:48Z

That is using ASF httpclient5x (5.5 currently). Took existing "apache" transport and migrated it to "classic" of 5.5 for now. Tests are not passing as I probably introduced some migration issues (or just overlooked something).

WIP

That is using ASF httpclient5x (5.5 currently).

cstamas · 2025-06-09T12:41:45Z

Current status:

[ERROR] Tests run: 85, Failures: 4, Errors: 10, Skipped: 1

cstamas · 2025-06-09T12:47:24Z

This also raises following question about Maven core:

it contains Wagon (to support legacy) and needs httpClient 4.x
it contains "apache" transport with httpClient 4.x
how does fit potentially now this new "apache5x" transport?

cstamas · 2025-06-10T19:45:50Z

@michael-o can you take a peek please? Also, I copied "apache" transport that was written in very specific way (ie globally shared state etc), that may not be needed (or will not work anymore) for httpclient 5.5.x...

michael-o · 2025-06-10T20:01:04Z

@michael-o can you take a peek please? Also, I copied "apache" transport that was written in very specific way (ie globally shared state etc), that may not be needed (or will not work anymore) for httpclient 5.5.x...

Will do tomorrow

cstamas · 2025-06-10T20:29:38Z

Migrating to 5.5.x "classic" API is IMO completely enough, unless you think we should do something else.

michael-o · 2025-06-13T15:14:36Z

+    <dependency>
+      <groupId>org.apache.httpcomponents.core5</groupId>
+      <artifactId>httpcore5-h2</artifactId>
+      <version>5.3.4</version>


Use a property

michael-o · 2025-06-13T15:15:52Z

+
+  <artifactId>maven-resolver-transport-apache5x</artifactId>
+
+  <name>Maven Artifact Resolver Transport Apache 5.x</name>


That is a bad name. Apache is, unfortunately, used synonymously for Apache HTTPd. Use the full name.

michael-o · 2025-06-13T15:15:59Z

  <artifactId>maven-resolver-transport-apache</artifactId>

-  <name>Maven Artifact Resolver Transport Apache</name>
+  <name>Maven Artifact Resolver Transport Apache 4.x</name>


michael-o · 2025-06-13T15:16:14Z

+    <version>2.0.10-SNAPSHOT</version>
+  </parent>
+
+  <artifactId>maven-resolver-transport-apache5x</artifactId>


apache-httpclient5

michael-o · 2025-06-13T15:16:34Z

 | `"aether.transport.apache.retryHandler.name"` | `String` | The name of retryHandler, supported values are “standard”, that obeys RFC-2616, regarding idempotent methods, and “default” that considers requests w/o payload as idempotent. |  `"standard"`  | 2.0.0 |  Yes  | Session Configuration |
 | `"aether.transport.apache.retryHandler.requestSentEnabled"` | `Boolean` | Set to true if it is acceptable to retry non-idempotent requests, that have been sent. |  `false`  | 2.0.0 |  Yes  | Session Configuration |
 | `"aether.transport.apache.useSystemProperties"` | `Boolean` | If enabled, underlying Apache HttpClient will use system properties as well to configure itself (typically used to set up HTTP Proxy via Java system properties). See HttpClientBuilder for used properties. This mode is not recommended, better use documented ways of configuration instead. |  `false`  | 2.0.0 |  Yes  | Session Configuration |
+| `"aether.transport.apache5x.followRedirects"` | `Boolean` | If enabled, Apache HttpClient will follow HTTP redirects. |  `true`  | 2.0.10 |  Yes  | Session Configuration |


Why x actually? Isn't 5 enough?

michael-o · 2025-06-13T15:17:11Z

+
+
+<site xmlns="http://maven.apache.org/SITE/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+      xsi:schemaLocation="http://maven.apache.org/SITE/2.0.0 https://maven.apache.org/xsd/site-2.0.0.xsd" name="Transport Apache">


Ditto for name

michael-o · 2025-06-13T15:18:07Z

+        }
+        return false;
+    }
+}


The client package or even core might include UriUtils. Did you check?

michael-o · 2025-06-13T15:19:25Z

+        }
+    }
+
+    public Boolean getWebDav() {


Why boolean object?

michael-o · 2025-06-13T15:23:42Z

+ */
+@Named(ApacheTransporterFactory.NAME)
+public final class ApacheTransporterFactory implements HttpTransporterFactory {
+    public static final String NAME = "apache5x";


michael-o · 2025-06-13T15:25:38Z

@arturobernalg @ok2c @garydgregory: Please have a look.

ok2c · 2025-06-13T17:24:00Z

@arturobernalg @ok2c @garydgregory: Please have a look.

@michael-o Sure. I will do a pass

cstamas · 2025-06-14T20:44:12Z

This was a misunderstanding. This PR is not "done-done". I asked @michael-o to "take over", if possible.

michael-o · 2025-06-14T20:46:11Z

This was a misunderstanding. This PR is not "done-done". I asked @michael-o to "take over", if possible.

Unfortuntely, my exprience with 5.x is basically zero. I do even maintain 4.x based client code at work. I'll be of very little help here.

gnodet

AI-Assisted Review — PR #742 (New transport: apache5x)

Thanks for tackling the HC 4.x → 5.x transport migration — this is a substantial and important piece of work (22 files, new module). The overall structure is clean, and the approach of creating a separate maven-resolver-transport-apache5x module rather than modifying the existing transport in-place is a sound strategy that allows both to coexist during the transition.

Verified findings

1. NPE on null password in HC 5.x migration (Bug — new in this PR)

In DeferredCredentialsProvider.BasicFactory.newCredentials(), password can be null when AuthenticationContext.get(AuthenticationContext.PASSWORD) returns null. The code calls password.toCharArray() without a null check. The old HC 4.x UsernamePasswordCredentials(String, String) accepted null passwords, but HC 5.x requires char[]:

// Current code — NPE when password is null:
new UsernamePasswordCredentials(username, password.toCharArray())
// Suggested fix:
new UsernamePasswordCredentials(username, password != null ? password.toCharArray() : new char[0])

2. Double status-code in error messages (Cosmetic — pre-existing)

ApacheRFC9457Reporter.getReasonPhrase() appends " (" + statusCode + ")" to the reason phrase, then handleStatus() appends it again, producing messages like "Not Found (404) (404)" for non-RFC 9457 responses. This is actually a pre-existing bug copied from the old transport (maven-resolver-transport-apache), so it's not a regression — but this migration is a good opportunity to fix it.

Noted (not issues)

NTLM/SPNEGO/Kerberos not registered: The old transport registered these schemes, but HC 5.3+ officially deprecated NTLM (and GSS-based schemes) and disables them by default. Dropping them aligns with HC 5.x's direction. A brief comment in the code or release notes noting the intentional omission would be helpful for users who relied on NTLM.
SharingAuthCache null handling: The get() method's behavior of putting null into the HashMap is correctly handled — HashMap.get() returns null for both absent keys and null-valued entries, so subsequent calls correctly re-query the state.

Note: This is a draft PR with merge conflicts and an existing review. The above findings focus on the code correctness aspects.

🤖 This review was generated with AI assistance (reviewer + independent verifier pattern). Findings were independently verified before posting.

New transport: apache5x

df9537d

That is using ASF httpclient5x (5.5 currently).

cstamas self-assigned this Jun 9, 2025

cstamas requested review from gnodet, michael-o and slawekjaranowski June 9, 2025 12:40

cstamas added 3 commits June 9, 2025 14:48

Move package apache -> apache5x

aea837b

Do reuse, not only if property present

50fd65e

Proper override

4b2b4de

cstamas added 3 commits June 10, 2025 21:53

Merge remote-tracking branch 'upstream/master' into apache5x

c0c2506

Fix generated doco

563706f

Fix again

db1a23d

michael-o requested changes Jun 13, 2025

View reviewed changes

kwin mentioned this pull request Oct 7, 2025

Undo JDK transport demote #1608

Closed

gnodet reviewed Jul 13, 2026

View reviewed changes

kwin mentioned this pull request Jul 16, 2026

Bump coordinates of httpcomponents #1954

Open


		<artifactId>maven-resolver-transport-apache5x</artifactId>

		<name>Maven Artifact Resolver Transport Apache 5.x</name>



		<site xmlns="http://maven.apache.org/SITE/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
		xsi:schemaLocation="http://maven.apache.org/SITE/2.0.0 https://maven.apache.org/xsd/site-2.0.0.xsd" name="Transport Apache">

Uh oh!

Conversation

cstamas commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cstamas commented Jun 9, 2025

Uh oh!

cstamas commented Jun 9, 2025

Uh oh!

cstamas commented Jun 10, 2025

Uh oh!

michael-o commented Jun 10, 2025

Uh oh!

cstamas commented Jun 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michael-o commented Jun 13, 2025

Uh oh!

ok2c commented Jun 13, 2025

Uh oh!

cstamas commented Jun 14, 2025

Uh oh!

michael-o commented Jun 14, 2025

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

AI-Assisted Review — PR #742 (New transport: apache5x)

Verified findings

Noted (not issues)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cstamas commented Jun 9, 2025 •

edited

Loading