Skip to content

Add HttpClient5 sampler implementation with HTTP/2 support - #6742

Open
andreaslind01 wants to merge 27 commits into
apache:masterfrom
andreaslind01:httpclient5_http2
Open

Add HttpClient5 sampler implementation with HTTP/2 support#6742
andreaslind01 wants to merge 27 commits into
apache:masterfrom
andreaslind01:httpclient5_http2

Conversation

@andreaslind01

Copy link
Copy Markdown
Contributor

Description

This PR adds a new HTTP sampler implementation, HttpClient5, based on Apache HttpComponents HttpClient 5.x, and introduces a configurable HTTP Version setting (HTTP/1.1 / HTTP/2) for both the HttpClient5 and the Java implementation.

Main changes:

  • New HTTPHC5Impl (HTTPSamplerFactory.IMPL_HTTP_CLIENT5, selectable as HttpClient5 in the GUI and in JMX files):
    • Classic (blocking) client for HTTP/1.1 and an async H2 client for HTTP/2, selected via HttpVersionPolicy (FORCE_HTTP_1 / NEGOTIATE), with automatic fallback to HTTP/1.1 when the server does not offer h2 via ALPN.
    • Per-thread client caching keyed by target/proxy/timeouts/local address/version policy.
    • Support for connect & response timeouts, proxies (incl. proxy authentication), AuthManager (BASIC/DIGEST, pre-emptive BASIC), CacheManager (conditional requests via If-Modified-Since / If-None-Match), CookieManager, DNSCacheManager, response decompression (gzip/deflate/brotli), and retry handling.
    • Correct population of SampleResult metrics: sentBytes, connectTime (measured for both HTTP/1.1 and HTTP/2, including TLS), latency, headers and response code/message.
  • HTTPJavaImpl: HTTP/2 support via the JDK java.net.http.HttpClient when HTTP/2 is selected, including caching, proxies, user authentication, sentBytes accounting, connect-time measurement, reason-phrase derivation (HTTP/2 has no reason phrase) and preservation of Authorization / Proxy-Authorization headers.
  • New sampler property HTTPSampler.httpVersion (HTTPSamplerBaseSchema.httpVersion, getter/setter on HTTPSamplerBase) with a new combo box in HTTP Request and HTTP Request Defaults (http_version resource key added to all messages_*.properties).
  • CacheManager: new overloads for HC5 (ClassicHttpRequest / ClassicHttpResponse / org.apache.hc.core5.http.Header[]) and for the JDK java.net.http.HttpResponse.
  • Property httpclient.version re-purposed as the default HTTP version (HTTP/1.1 | HTTP/2) used when the sampler's HTTP Version field is empty.
  • Dependencies: httpclient5 and httpcore5 added to src/protocol/http and to the third-party BOM (httpcore5:5.3.4).
  • Documentation updated: component_reference.xml, properties_reference.xml, get-started.xml, bin/jmeter.properties.

Motivation and Context

JMeter's HTTP samplers currently only support HTTP/1.1: the HttpClient4 implementation is built on the HttpComponents 4.x line, which will not receive HTTP/2 support, and the Java implementation used the legacy HttpURLConnection. Modern web applications and APIs are increasingly served over HTTP/2, so load tests against them either could not be executed at all or did not represent realistic client behaviour (multiplexing, HPACK header compression, single connection per origin).

This change gives users a supported migration path to HttpComponents 5.x and makes it possible to run load tests over HTTP/2 — either with the fully featured HttpClient5 implementation or, for lightweight scenarios, with the JDK client in the Java implementation. Existing test plans are unaffected: HttpClient4 remains the default and an empty HTTP Version falls back to the previous HTTP/1.1 behaviour.

Fixes:

How Has This Been Tested?

  • New unit/integration tests (34 tests, all green):
    • TestHTTPHC5Features (16 tests): version selection and precedence (sampler value vs. httpclient.version vs. unsupported value), HTTP/2 usage, fallback to HTTP/1.1 when the server does not support h2, HTTP/2 via proxy, sentBytes for GET/POST, conditional requests through CacheManager, BASIC credentials from AuthManager, proxy authentication, and connectTime for HTTP/1.1 and HTTP/2.
    • TestHTTPJavaFeatures (~16 tests): version selection, HTTP/2 requests (incl. via proxy), response message / reason-phrase handling for HTTP/2, sentBytes for GET/POST in both versions, Authorization header from the HeaderManager, and connectTime for HTTP/1.1, HTTP/2 plaintext and HTTP/2 over TLS.
    • TestHTTPSamplerFactory: creation and lookup of the new HttpClient5 implementation, plus the unchanged behaviour for the existing aliases.
    • The tests run against locally started embedded HTTP/1.1, HTTP/2 (h2c and h2 over TLS) and proxy servers, so no external services are required.
  • ./gradlew classes style — compiles cleanly and reports no style/checkstyle/autostyle violations.
  • The existing src:protocol:http test suite (including JMeterTest, extended by httpVersion in the ignored-properties list) still passes.
  • Manual verification in the JMeter GUI: the new HTTP Version combo box in HTTP Request and HTTP Request Defaults is saved/restored correctly in JMX files, and requests against an HTTP/2 endpoint are reported as HTTP/2 in the View Results Tree.
  • Test environment: Windows, JDK 21 toolchain, Gradle build JMeter 6.0.0-SNAPSHOT.

Screenshots (if appropriate):

image

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.

…ng, caching, proxies, and user authentication
…and improve fallback handling with new tests
…ts, including HTTP/2, with corresponding unit tests
…ers (`Authorization`, `Proxy-Authorization`) and adding unit tests for validation

@milamberspace milamberspace left a comment

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.

Not a full review of this PR — just a scoped, timely note on the dependency versions.

Apache HttpComponents Client just released 5.6.4: "Corrects application of SSL parameters in the async TLS upgrade method" (RELEASE_NOTES-5.6.x.txt). This PR pins httpclient5:5.5.1, and HTTPHC5Impl is exactly the kind of code that exercises that path — it uses the async H2 client with HttpVersionPolicy.NEGOTIATE for HTTP/2-over-TLS via ALPN, i.e. an async TLS upgrade. Worth pulling in the fix before this lands, rather than shipping the new HTTP/2 sampler with a known bug in SSL-parameter application during that exact upgrade.

See inline comment for the concrete version bump (and the matching httpcore5 pairing, since httpclient5:5.6.4 is built/tested against httpcore5:5.4.3, not 5.3.4).

This review was drafted by an AI-assisted tool and confirmed by an Apache JMeter maintainer.

Comment thread src/bom-thirdparty/build.gradle.kts Outdated
@@ -107,6 +107,8 @@ dependencies {
because("User might still rely on commons-text")
}
api("org.apache.httpcomponents.client5:httpclient5:5.5.1")

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.

Since HTTPHC5Impl (this PR) exercises HttpClient5's async TLS upgrade path for HTTP/2, worth bumping both of these before merge:

  • httpclient5: 5.5.15.6.4 — fixes "SSL parameter application in the async TLS upgrade strategy" (release notes)
  • httpcore5 / httpcore5-h2: 5.3.45.4.3 — the version httpclient5:5.6.4 is actually built and tested against (per its parent POM's httpcore.version property), so bumping only httpclient5 and leaving httpcore5 at 5.3.4 would be an incoherent pairing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks — good catch. Bumped httpclient5 to 5.6.4 and httpcore5/httpcore5-h2 to 5.4.3. Confirmed the pairing you flagged: httpclient5-parent-5.6.4.pom sets <httpcore.version>5.4.3</httpcore.version>.

Not a drop-in bump though — it surfaced two real issues:

1. HTTPS/HTTP-2 handshakes broke (SSLHandshakeException: No name matching localhost found). Now that SSL parameters are actually applied on the async path, JSSE endpoint identification runs, and ClientTlsStrategyBuilder defaults to BOTH when a hostnameVerifier is set, silently overriding our NoopHostnameVerifier + TrustAllStrategy. On 5.5.1 that half was a no-op because of the bug. Fixed with .setHostVerificationPolicy(HostnameVerificationPolicy.CLIENT). This would have hit anyone testing HTTPS with a self-signed cert, so good that this landed before the sampler shipped.

2. NoClassDefFoundError in HTTPHC5Impl's static initializer — 5.6 rewrote BrotliInputStreamFactory to use the optional brotli4j, which we don't ship. Fixed by decoding br via org.brotli:dec, already a direct dependency and what HTTPHC4Impl uses.

Also switched deprecated build()buildAsync() and suppressed the new deprecation warnings (-Werror). Didn't migrate to the suggested ContentCodecRegistry — it's @Internal. Happy to revisit.

classes style clean; :src:protocol:http:test 977 passed / 0 failed.

….4.3 respectively for improved SSL parameter handling in HTTP/2
… and ensuring consistent header reporting across transports

@milamberspace milamberspace left a comment

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.

Found a real bug while manually testing the HTTP/2 sampler against a long-running server: intermittent

java.io.IOException: Could not execute HTTP/2 request
Caused by: org.apache.hc.core5.http2.impl.nio.ConnectionClosedException: Connection is closed
	at org.apache.hc.core5.http2.impl.nio.H2Streams.shutdownAndReleaseAll(H2Streams.java:149)
	at org.apache.hc.core5.http2.impl.nio.AbstractH2StreamMultiplexer.onOutput(...)
	...

Root cause

createHttp2Client() never configures ConnectionConfig.validateAfterInactivity on the PoolingAsyncClientConnectionManagerBuilder, and ConnectionConfig.DEFAULT documents it as null (undefined). In PoolingAsyncClientConnectionManager#lease(), the re-validation step (an HTTP/2 PING before handing out a pooled connection, StaleCheckCommand for HTTP/1.1) is gated by:

final TimeValue timeValue = connectionConfig.getValidateAfterInactivity();
if (connection.isOpen() && TimeValue.isNonNegative(timeValue)) { ... }

TimeValue.isNonNegative(null) is false, so with the default config this block never runs — pooled async connections are leased straight out of the pool with zero liveness check.

Concretely: HTTP_2_CLIENTS caches the async client per JMeter thread and reuses it across iterations. If the server (or an idle load balancer/NAT) closes an idle pooled HTTP/2 connection between two samples, the next sample picks it from the pool as-is; the I/O reactor only discovers it's dead when it tries to write to it, surfacing as ConnectionClosedException deep in H2Streams.

This is made worse by disableAutomaticRetries() (called on both the classic and async builders) — correctly disabled so JMeter doesn't silently mask real server behavior/timing from the sample result, but it also removes HttpClient5's own safety net for exactly this failure mode. Without proactive pool validation, there's nothing left to catch it, and it surfaces as a hard sampler failure instead of a transparent retry.

Suggested fix

See inline comment — add .setValidateAfterInactivity(...) to the ConnectionConfig built in createHttp2Client() (worth doing for createClient()'s classic-transport config too, same gap applies there).

This review was drafted by an AI-assisted tool and confirmed by an Apache JMeter maintainer.

if (key.dnsCacheManager != null) {
connectionManagerBuilder.setDnsResolver(createDnsResolver(key.dnsCacheManager));
}
if (key.connectTimeout > 0) {

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.

Worth folding a setValidateAfterInactivity into this ConnectionConfig (unconditionally, not just under the connectTimeout > 0 guard) so pooled HTTP/2 connections get an HTTP/2 PING liveness check before reuse instead of being handed out straight from the pool:

ConnectionConfig.Builder connectionConfig = ConnectionConfig.custom()
        .setValidateAfterInactivity(TimeValue.ofSeconds(2));
if (key.connectTimeout > 0) {
    connectionConfig.setConnectTimeout(Timeout.ofMilliseconds(key.connectTimeout));
}
connectionManagerBuilder.setDefaultConnectionConfig(connectionConfig.build());

Without it, ConnectionConfig.DEFAULT.getValidateAfterInactivity() is null, and PoolingAsyncClientConnectionManager#lease() skips its re-validation step entirely (TimeValue.isNonNegative(null) is false), so a connection the server already closed gets reused as-is and fails mid-write with ConnectionClosedException instead of being transparently discarded and replaced.

@milamberspace

Copy link
Copy Markdown
Contributor

Following up on the stale-connection bug above with a concrete repro I ran manually against a real server (own domain, browsing-style scenario: 5 threads, 3 loops, ~800–2800ms think time between transactions on the same reused HTTP/2 connection).

Worth a regression test to prove validateAfterInactivity actually catches this rather than just proving it's configured. One nuance that matters for how the test is shaped: it's not the request count that triggers this, it's the idle gap between two requests on the same pooled connection. A tight rapid-fire loop with no pauses almost certainly won't reproduce it — my manual repro needed nothing more exotic than ~1–3s of think time between transactions, which was enough for the server side to close the idle connection before the next request picked it back up from the pool.

Suggested shape for the test:

  1. Spin up an embedded HTTP/2-over-TLS server the test controls directly (not WireMock's black-box backend — this needs to forcibly close an already-accepted connection without stopping the whole server, which WireMock doesn't expose).
  2. Fire request Expand / Collapse all buttons #1 through HTTPHC5Impl with HTTP/2 selected, get a 200.
  3. Forcibly close the underlying TCP connection from the server side right after responding (server keeps listening for new connections on the same port — just this one connection dies), simulating an idle server-side timeout.
  4. Sleep a little longer than whatever validateAfterInactivity ends up configured to (e.g. 1.1s if it's set to 1s).
  5. Fire request BUG-49753 Enhancement implemented #2 from the same HTTPSamplerBase instance / same thread, so it goes through the cached HTTP_2_CLIENTS entry and reuses the pool.
  6. Assert request BUG-49753 Enhancement implemented #2 succeeds (200, no ConnectionClosedException) — and ideally assert it would fail without the fix, e.g. by running the same test body against a build with validateAfterInactivity stripped out, or documenting in the test comment that it reproduces apache/jmeter#<this-PR>'s manual repro.

Happy to be wrong about the exact mechanics of forcing step 3 cleanly with whatever test HTTP/2 server this project already has infrastructure for (TestHTTPHC5Features already stands up embedded h2/h2c servers per the PR description) — but the idle-gap-not-volume framing is the part I'd want the test to actually exercise, since that's what makes it a faithful regression test rather than one that happens to pass for unrelated reasons.

@andreaslind01

Copy link
Copy Markdown
Contributor Author

Thanks @milamberspace - fixed and covered by a regression test.

Fix: setValidateAfterInactivity is now applied unconditionally (not under the connectTimeout > 0 guard) to the ConnectionConfig of both createHttp2Client() and createClient(), exactly as suggested. The interval is configurable via a new httpclient5.validate_after_inactivity property (ms, default 2000, -1 disables), documented in jmeter.properties, properties_reference.xml and changes.xml.

One correction on the mechanics, which changed how the test had to be shaped. Tracing httpclient5 5.6.4 / httpcore5 5.4.3: a cleanly closed connection isn't what fails. LaxConnPool.getAvailableEntry does hand out the dead entry, but AsyncConnectExec then checks isEndpointConnected() and transparently reconnects - and AbstractH2StreamMultiplexer.isOpen() is connState == ACTIVE, so once the reactor has processed the FIN (or a GOAWAY) the endpoint reports "not connected" and recovers on its own. Step 3 as suggested would therefore have passed with and without the fix. The failure needs the client to still believe the connection is usable when the request is submitted - that's the H2Streams.shutdownAndReleaseAll path in the trace.

What the test does instead, keeping the idle-gap framing:

A raw TCP relay sits in front of WireMock's h2/TLS port, so the test owns the client-facing socket. It can mark a connection doomed: it stays open, but is dropped as soon as the client writes to it again - what an idle server/LB timeout looks like to a client that hasn't noticed yet.

  1. Request 1 → 200, connection pooled.
  2. Sleep validateAfterInactivity + 500ms - the idle gap, not the request count, arms the check.
  3. Doom the pooled connection.
  4. Request 2 from the same sampler/thread, reusing the cached HTTP_2_CLIENTS entry and pool.
  5. Assert 200 and that the relay accepted exactly 2 connections - proving the stale one was replaced, not that it passed for unrelated reasons.

On asserting it fails without the fix: verified directly by forcing the default to -1, which reproduces the reported error verbatim (Could not execute HTTP/2 request / Non HTTP response code: java.io.IOException). Since that can't be asserted in one build, a companion test doesNotRevalidatePooledHttp2ConnectionWithoutAnIdleGap runs the same body with no idle gap, where the check is legitimately skipped and the sample fails - pinning the mechanism from the other side.

./gradlew classes style clean, full :src:protocol:http:test (983 tests) green, HC5 tests run 3× without flakiness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants