Skip to content

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

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

Add HttpClient5 sampler implementation with HTTP/2 support#6742
andreaslind01 wants to merge 33 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.

@milamberspace

Copy link
Copy Markdown
Contributor

Thanks @andreaslind01 — manual testing against the built distribution (browsing-style scenario, HTTP/2, same idle-gap pattern as the original repro) is conclusive: the stale-connection failure is gone. Great fix, and a genuinely clever regression test.

Two smaller, non-blocking points from going through the GUI while testing:

1. The HTTP Version combo doesn't reflect what each implementation actually supports

In HTTP RequestAdvanced (and HTTP Request Defaults), the Implementation combo offers HttpClient4 / HttpClient5 / Java / (default), and HTTP Version offers HTTP/1.1 / HTTP/2 / (empty) — independently of each other. HttpClient4 and HTTP/2 can both be selected together, but HTTPHC4Impl never reads the httpVersion property at all, so the request silently runs as HTTP/1.1 regardless — nothing in the UI signals that the combination is a no-op.

Would be worth either:

  • disabling/graying out HTTP/2 in the HTTP Version combo when HttpClient4 is the selected implementation, and/or
  • relabeling the item to make the fallback explicit, e.g. HTTP/2.0 (back to HTTP/1.1) when the active implementation doesn't support it.

Relevant: httpImplementation/httpVersion combos in HttpTestSampleGui.java and HttpDefaultsGui.java — currently just two independent JComboBoxes with no listener tying one to the other's state/labels.

2. Advanced-tab screenshot is stale

xdocs/images/screenshots/http-request-advanced-tab.png (referenced from component_reference.xml) predates this PR by several years and doesn't show the new HTTP Version field or the HttpClient5 implementation choice. Worth refreshing it as part of this PR (ideally with the Metal look-and-feel, to match the rest of the JMeter docs' screenshots) so the manual covers the feature it now documents.

@milamberspace

Copy link
Copy Markdown
Contributor

A few notes on xdocs/changes.xml:

Missing <pr>6742</pr> reference

None of the entries this PR adds carry a <pr> tag, unlike their neighbors in the same lists (e.g. <pr>6268</pr>, <pr>6620</pr>). Worth adding <pr>6742</pr> to:

  • the four new bullets under Changes → HTTP Samplers and Test Script Recorder (HTTP/2 multiplexing for HttpClient5 and Java, default User-Agent for both),
  • the HttpClient5/HttpCore5 version-bump bullet under Changes → Non-functional changes.

Maybe group the HttpClient5/HTTP2 entries under one heading

The four new HTTP Samplers bullets (HTTP/2 multiplexing ×2, default User-Agent ×2) currently sit flat in the same list as older, unrelated entries (IE conditional comments, argument enable/disable, multipart charset, redirect method preservation…). They're really one coherent piece of work — might read better with a short lead-in grouping them, e.g. a one-line "HTTP/2 support for the HttpClient5 and Java sampler implementations:" before the four bullets, so a reader scanning the changelog sees it as one feature rather than four scattered items. No strong opinion on the exact markup — this file doesn't have a <h4> sub-heading precedent elsewhere, so whatever's lightest.

The pooled-connection re-validation entry shouldn't be in Bug fixes

Re-validate pooled connections of the HttpClient5 sampler implementation after they have been idle...

This fixes a bug introduced and fixed entirely within this same unreleased PR — it never shipped in any JMeter release, so it isn't a "bug fix" from the changelog reader's perspective (there's nothing between two releases for them to have hit). Suggest dropping this bullet from Bug fixes → HTTP Samplers and Test Script Recorder entirely, since the fixed behavior is just folded into the feature as it ships. If it's worth keeping any trace of it at all, httpclient5.validate_after_inactivity could just be mentioned in passing in one of the Changes bullets above instead — but a dedicated "bug fix" entry for a bug the released code never had reads as noise.

Thanks section

Missing Andreas Lind (github.com/andreaslind01) in the Thanks list at the bottom.

@andreaslind01

Copy link
Copy Markdown
Contributor Author

Thanks @andreaslind01 — manual testing against the built distribution (browsing-style scenario, HTTP/2, same idle-gap pattern as the original repro) is conclusive: the stale-connection failure is gone. Great fix, and a genuinely clever regression test.

Two smaller, non-blocking points from going through the GUI while testing:

1. The HTTP Version combo doesn't reflect what each implementation actually supports

In HTTP RequestAdvanced (and HTTP Request Defaults), the Implementation combo offers HttpClient4 / HttpClient5 / Java / (default), and HTTP Version offers HTTP/1.1 / HTTP/2 / (empty) — independently of each other. HttpClient4 and HTTP/2 can both be selected together, but HTTPHC4Impl never reads the httpVersion property at all, so the request silently runs as HTTP/1.1 regardless — nothing in the UI signals that the combination is a no-op.

Would be worth either:

  • disabling/graying out HTTP/2 in the HTTP Version combo when HttpClient4 is the selected implementation, and/or
  • relabeling the item to make the fallback explicit, e.g. HTTP/2.0 (back to HTTP/1.1) when the active implementation doesn't support it.

Relevant: httpImplementation/httpVersion combos in HttpTestSampleGui.java and HttpDefaultsGui.java — currently just two independent JComboBoxes with no listener tying one to the other's state/labels.

2. Advanced-tab screenshot is stale

xdocs/images/screenshots/http-request-advanced-tab.png (referenced from component_reference.xml) predates this PR by several years and doesn't show the new HTTP Version field or the HttpClient5 implementation choice. Worth refreshing it as part of this PR (ideally with the Metal look-and-feel, to match the rest of the JMeter docs' screenshots) so the manual covers the feature it now documents.

Thanks for the testing and the detailed review @milamberspace.

Regarding the HTTP Version selector: I actually tend towards removing it rather than introducing implementation-specific enable/disable logic. The selected implementation already determines the effective protocol behavior (HttpClient4 → HTTP/1.1, HttpClient5/Java → HTTP/2 with HTTP/1.1 fallback).

A tooltip explaining this behavior would likely be clearer than allowing users to choose combinations that are effectively ignored. For the few cases where HTTP/1.1 must be enforced, I would prefer dedicated properties in jmeter.properties rather than additional GUI complexity.

@milamberspace

Copy link
Copy Markdown
Contributor

Following up on removing the HTTP Version field — I'd argue the opposite: keep it, but make its options implementation-aware instead of static. Removing it loses something a load-testing tool specifically benefits from.

Why keep it: comparative testing on the same target

The whole point of a field like this in JMeter is to let a single test plan run the same request against the same target under different explicit protocol conditions — e.g. HttpClient5 forced to HTTP/1.1 vs HttpClient5 negotiating HTTP/2 vs HttpClient5 strictly requiring HTTP/2, all as separate samplers/thread groups in one scenario, to compare latency, throughput or behavior side by side. Or Java HTTP/1.1 vs Java HTTP/2 Negotiate. That's a legitimate, common load-testing use case (validating an HTTP/2 migration, quantifying its actual performance benefit, or just making sure a "should be HTTP/2" test really is one — which is exactly the failure mode my own manual test just ran into: NEGOTIATE silently testing HTTP/1.1 with nothing in the GUI making that obvious). A global jmeter.properties default can't do that — it's one value for the whole run, not a per-sampler choice.

Losing per-sampler control to gain a tooltip trades away a real capability for a cosmetic simplification.

Proposal: implementation-aware options, not a removed field

Rather than a static 3-item combo shared by all implementations, populate it based on the currently selected Implementation, dynamically (an ItemListener on httpImplementation refreshing httpVersion's model — this also directly answers the concern about combinations that are silently ignored, since the no-op ones simply wouldn't be offered anymore):

  • HttpClient4: (default) / HTTP/1.1 — no HTTP/2 entry at all, since HTTPHC4Impl never reads httpVersion; today's silent no-op combination becomes structurally impossible instead of documented-away.
  • Java: (default) / HTTP/1.1 / HTTP/2 Negotiate — matches what java.net.http.HttpClient can actually do (no strict mode exists in the JDK's public API, so don't offer one).
  • HttpClient5: (default) / HTTP/1.1 / HTTP/2 Negotiate / HTTP/2 Strict — HttpClient5 is the one implementation where the underlying library can genuinely do all three, so expose all three rather than the two the current PR limits it to.

HTTP/2 Strict for HttpClient5 needs one more code change

Right now getHttpVersionPolicy(String, String, String scheme) only ever returns FORCE_HTTP_2 for plaintext http:// with prior knowledge — for https://, HTTP/2 always resolves to NEGOTIATE, with silent ALPN fallback to HTTP/1.1 if the server doesn't offer h2:

static HttpVersionPolicy getHttpVersionPolicy(String samplerHttpVersion, String defaultHttpVersion, String scheme) {
    HttpVersionPolicy policy = getHttpVersionPolicy(samplerHttpVersion, defaultHttpVersion);
    if (policy == HttpVersionPolicy.NEGOTIATE && HTTP_2_PRIOR_KNOWLEDGE
            && !HTTPConstants.PROTOCOL_HTTPS.equalsIgnoreCase(scheme)) {
        return HttpVersionPolicy.FORCE_HTTP_2;
    }
    return policy;
}

ClientTlsStrategyBuilder/TlsConfig.setVersionPolicy(FORCE_HTTP_2) offers only h2 in the ALPN extension, so the TLS handshake itself fails when the server doesn't support it — exactly the "strict" behavior. Worth a distinct sampler-level value (not reusing the existing "HTTP/2" string, to keep old JMX files negotiating as they do today) that maps to FORCE_HTTP_2 regardless of scheme, surfaced as HTTP/2 Strict in the combo.

Happy to be told this is more scope than this PR should carry and belongs in a follow-up — just wanted to lay out the full shape of it while the HTTP Version field is under discussion, since the three-tier HttpClient5 combo and the field's removal are mutually exclusive decisions.

@andreaslind01

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed feedback and for laying out the use case so clearly. I agree that removing the field entirely would sacrifice a legitimate and valuable load-testing capability, particularly for side-by-side protocol comparisons within a single test plan.

I've implemented the proposed direction and the changes are now available:

  • The HTTP Version field has been retained.
  • The available options are now implementation-aware and dynamically updated based on the selected HTTP implementation, preventing unsupported combinations from being selected.
  • HttpClient4, Java, and HttpClient5 each expose only the protocol modes that are actually supported by their underlying implementation.
  • Support for HTTP/2 Strict has been added for HttpClient5, mapping to FORCE_HTTP_2 independently of the scheme, while preserving the existing behavior of legacy HTTP/2 configurations for backward compatibility.

This keeps per-sampler protocol control available while eliminating the previous silent no-op combinations that motivated the original discussion.

Thanks again for the suggestion. I think this results in a clearer UI without reducing functionality.

@milamberspace

Copy link
Copy Markdown
Contributor
http-request-advanced-tab

@milamberspace

Copy link
Copy Markdown
Contributor

Manual testing against the latest build is OK on these points:

  • The HTTP Version combo now correctly restricts itself to what each implementation supports
  • HTTP/2 Strict for HttpClient5 behaves exactly as intended: pointed at a server that only speaks HTTP/1.1, the sample fails as expected

Also did a clean :src:dist:assemble build from this commit to get a good distributable archive (using for my tests)

The last change is to update ./xdocs/images/screenshots/http-request-advanced-tab.png with the capture posted on this discussion (and change the component_reference.xml with the good dimension of screenshot: width="1734" height="669")

Thanks again for you PR / work.

@milamberspace
milamberspace requested a review from vlsi August 14, 2026 04:45
@andreaslind01

Copy link
Copy Markdown
Contributor Author

Thanks a lot for the thorough testing.

Both screenshots are updated: http-request-advanced-tab.png and — for consistency, since it shows the same new HTTP Version combo — http-config/http-request-defaults-advanced-tab.png.

I exported them at 950×391 (and set component_reference.xml accordingly), so they match the other figures in that file (e.g. graphql-http-request.png at 950×618) and don't get scaled by the browser.

@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.

@andreaslind01 Hi, the constants review. Thanks

* implementation supports. The items are the values stored in the {@code HTTPSampler.httpVersion}
* property, the rendering spells out how HTTP/2 is applied.
*
* @since 5.7

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.

change for @SInCE 6.0

private static Object getLabel(Object value) {
if (HTTPConstants.HTTP_VERSION_2.equals(value)) {
// Spelled out, as HTTP/2 falls back to HTTP/1.1 when the server does not support it
return "HTTP/2 Negotiate";

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.

Move to constants variable


private static String getEntityPreview(HttpEntity entity, String contentEncoding) throws IOException {
if (!entity.isRepeatable()) {
return "<Entity was not repeatable, cannot view what was sent>";

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.

Constant or translate string ?

if (HTTPConstants.HTTP_VERSION_2_STRICT.equalsIgnoreCase(httpVersion)) {
return HttpVersionPolicy.FORCE_HTTP_2;
}
return HTTPConstants.HTTP_VERSION_2.equals(httpVersion) || "2".equals(httpVersion)

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.

Where the httpVersion can have the value "2"?

static boolean isHttp2(String samplerHttpVersion, String defaultHttpVersion) {
String httpVersion = StringUtilities.isBlank(samplerHttpVersion) ? defaultHttpVersion : samplerHttpVersion;
// java.net.http.HttpClient always negotiates, so a strict HTTP/2 request is negotiated as well
return HTTPConstants.HTTP_VERSION_2.equalsIgnoreCase(httpVersion) || "2".equalsIgnoreCase(httpVersion)

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.

Same question about the value "2"

cacheManager.saveDetails(response, res);
}

res.setSentBytes(calculateSentBytes(url, method, "HTTP/2",

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.

Constant for "HTTP/2" here

if (res.getEndTime() == 0) {
res.sampleEnd();
}
res.setSentBytes(calculateSentBytes(url, method, "HTTP/2",

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.

Constant


private static String getResponseHeaders(HttpResponse<?> response) {
StringBuilder headerBuf = new StringBuilder();
String versionStr = (response.version() == HttpClient.Version.HTTP_2) ? "HTTP/2" : "HTTP/1.1"; // $NON-NLS-1$ $NON-NLS-2$

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.

Constants

uri = "";
}
org.apache.hc.core5.http.ProtocolVersion version = request.getVersion();
String versionStr = version != null ? version.toString() : "HTTP/1.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.

Constant pls

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