JCR-5255: Migrate to Apache HttpClient 5 - #367
Open
slachiewicz wants to merge 4 commits into
Open
Conversation
…n parent Concurrent logins of the same user each use their own session but add their token node below the same .tokens parent. A concurrent commit below that parent can invalidate this session's pending changes, so that the token node can neither be saved (InvalidItemStateException from validateTransientItems) nor resolved afterwards (ItemNotFoundException while building its path). Either one failed the whole login. Wrap the token node creation in a bounded retry that discards the doomed transient state via session.refresh(false) and re-reads the token parent, mirroring the conflict handling that getTokenParent already performs for the concurrent creation of the token store itself. The original exception is rethrown once the attempts are exhausted, so behaviour on persistent failures is unchanged. This makes token creation tolerate the conflict but does not remove the underlying race in the transient state handling.
Replace Apache HttpClient 4.5.14 / HttpCore 4.4.16 with HttpClient 5.6.1 and HttpCore 5.4.2 across jackrabbit-webdav, jackrabbit-spi2dav, jackrabbit-jcr-server and jackrabbit-it-osgi. The httpmime artifact is dropped; its classes now live in httpclient5 as org.apache.hc.client5.http.entity.mime. This is a breaking API change. The exported package org.apache.jackrabbit.webdav.client.methods goes from 2.0.0 to 3.0.0: * BaseDavRequest extends HttpUriRequestBase instead of HttpEntityEnclosingRequestBase, and takes the request method as its first constructor argument, since HttpClient 5 has no no-arg base constructor plus setURI. Subclasses no longer override getMethod(). * The response accessors take ClassicHttpResponse in place of HttpResponse. * XmlEntity.create returns org.apache.hc.core5.http.HttpEntity. In jackrabbit-spi2dav, RepositoryServiceImpl.executeRequest returns ClassicHttpResponse, initMethod takes the HttpClient 5 HttpUriRequest, and ExceptionConverter.generate takes HttpUriRequestBase. The public ConnectionOptions API is unchanged. Behaviour that needed explicit handling rather than a mechanical rename: * Connect and socket timeouts moved from RequestConfig to ConnectionConfig. The ConnectionOptions value -1 means "not configured", which was infinite under HttpClient 4; it now maps to Timeout.DISABLED, because omitting the setter would silently pick up the HttpClient 5 default of three minutes and break long-running polls and batches. * HttpClient 5 does not pre-authenticate from a BasicScheme in the AuthCache unless it has been primed, so initPreemptive is now called. * HttpClient 5 follows redirects for every method, where HttpClient 4 redirected only GET and HEAD. GetHeadRedirectStrategy restores the old rules so that a redirected MOVE, COPY, PUT or DELETE surfaces to the caller instead of silently retargeting a write. * ProxyAuthenticationStrategy is gone; proxy authentication is handled by the shared authentication strategy. * HttpHost takes (scheme, host, port) rather than (host, port, scheme), and credentials take a char[] password. * releaseConnection() maps to reset(); in HttpClient 4 releaseConnection() was defined as reset(). * ContentType.get(HttpEntity) is gone. The replacement is guarded, since the HttpClient 4 method returned null for a null entity. * Entities are immutable, so content encoding moves into the constructor. * HttpMultipartMode.RFC6532 is now EXTENDED. HttpComponents publishes no OSGi bundles for the 5.x line: httpclient5-osgi and httpcore5-osgi stop at 5.0-beta, and the plain JARs carry no Bundle-SymbolicName. jackrabbit-it-osgi therefore repacks them as bundles for the test container. The retry behaviour is left at the HttpClient 5 default, which retries once on 429 and 503 where HttpClient 4 never retried on a status code. Two changes to the WebDAV test harness were needed, neither affecting main code: * HttpClient 5 wraps the TLS upgrade in the per-address retry block of DefaultHttpClientConnectionOperator, so an SSLHandshakeException on the first resolved address is logged at DEBUG and the next address is tried; the caller sees whatever the last address produced. HttpClient 4 only retried on ConnectException, NoRouteToHostException and SocketTimeoutException. Where localhost resolves to both 127.0.0.1 and ::1 and the server binds one family, this turned the handshake failure that HttpsSelfSignedTest asserts on into a connection failure, so the HTTPS connector and its URI are now pinned to 127.0.0.1. * HttpClient 5 keeps a pooled connection leased until the response is closed or its entity consumed, including for status-only responses that HttpClient 4 released automatically. The tests read status codes without closing responses, which exhausted the default pool of five per route and then blocked for the three minute lease timeout. The test pool is now sized for the busiest test class, with a short lease timeout so exhaustion fails fast instead of stalling. Main code is unaffected, since it releases via request.reset() in a finally.
- restore HttpClient 4 semantics for connection options: null proxy password, requestTimeoutMs=0 as infinite lease wait, and negative connect/socket timeouts as disabled - copy the SimpleCredentials password instead of aliasing the caller's array - send the absolute request URI in the Referer header again - build TLS on the supported TlsSocketStrategy API instead of the deprecated SSLConnectionSocketFactory/TrustSelfSignedStrategy - use a token-only multipart boundary and set Content-Disposition explicitly so no RFC 5987 filename* parameter is emitted for non-Latin-1 JCR paths - fail fast in Rfc6532MultipartEntity.getContent() like HttpClient 4 did and measure the framing without buffering it - avoid NPEs on bodyless error responses, responses without a Content-Type charset, and CHECKIN responses without a Location header
DefaultClientTlsStrategy defaults to HostnameVerificationPolicy.BOTH, so the JSSE built-in endpoint identification still ran even when a NoopHostnameVerifier was passed, making ConnectionOptions.disableHostnameVerification ineffective. Pass HostnameVerificationPolicy.CLIENT along with the noop verifier. Also relax the ConnectionTest assertion for the enabled-verification case: the built-in check rejects the host name during the handshake with an SSLHandshakeException instead of the SSLPeerUnverifiedException thrown by the client-side verifier of HttpClient 4.
slachiewicz
force-pushed
the
httpclient5-migration
branch
from
August 8, 2026 11:28
1fc14a3 to
1697ce8
Compare
Contributor
|
@slachiewicz - I'm very reluctant to merge a PR that affects 66 files, in particular when it's different from what we were planning to do. And yes, both proposed changes are going to be breaking, unless we use new packages that can live side by side for some time. @kwin ? |
Member
|
Given that Apache HTTP Components no longer provide OSGi bundles for version 5 and our main consumers leverage OSGi I would focus on Java HttpClient. The missing support for evaluating 1xx responses is IMHO no blocker. |
Member
Author
|
I'm ok about that. It was side effect of working on maven wagon component where jackrabbit was dependency blocking upgrade for us. |
Member
Author
|
Later I've found also ticket related to completely remove ht4 and relay on Java. For Maven we found that this was a bit limiting and problematic but @kwin should be already aware about that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrates the WebDAV/spi2dav client stack from Apache HttpClient 4.5.x to HttpClient 5.6.1 (httpcore 5.4.2).
Migration highlights
org.apache.hc.client5/org.apache.hc.core5APIs; method constants, redirect handling and connection release (releaseConnection()->reset()) reviewed for behavioral parity.GetHeadRedirectStrategyrestricts this to the safe methods HttpClient 4 redirected.ConnectionOptionsvalues are mapped so-1/0keep their documented HttpClient 4 meaning (infinite) instead of silently picking up the new defaults.HttpMultipartMode.RFC6532, so a smallRfc6532MultipartEntitydoes the framing itself;Rfc6532MultipartEntityTestpins the wire format byte for byte against HttpClient 4.5.14 output, including quoting/escaping and non-Latin-1 paths (no RFC 5987filename*parameter).TlsSocketStrategy/DefaultClientTlsStrategyAPI rather than the deprecatedSSLConnectionSocketFactory.SimpleCredentialspassword copied at login time, absolute request URI in theRefererheader,getContent()failing fast withContentTooLongExceptionfor large/unknown-length multipart bodies, and null-safe handling of bodyless error responses, missingContent-Typecharsets, and CHECKIN responses without aLocationheader.Known limitation
HttpComponents 5.x jars ship no OSGi manifests (upstream dropped OSGi support), so the
org.apache.hc.*imports of thejackrabbit-webdav/jackrabbit-spi2davbundles cannot resolve in an OSGi container out of the box. The OSGi IT wraps the jars locally (TestBundles); how to serve real OSGi consumers (embedding, wrapper bundles, or upstream metadata) is left as a follow-up decision.Testing
jackrabbit-webdavandjackrabbit-spi2davunit tests pass.