From 2c529077bb1766f137cb6bb08e08f4a833011f41 Mon Sep 17 00:00:00 2001 From: David <3dgiordano@gmail.com> Date: Tue, 25 Aug 2026 05:15:48 -0300 Subject: [PATCH] H2C Fallback to HTTP/1.1 support Only when H2C not is supported --- .../jmeter/http2/core/HTTP2JettyClient.java | 37 +++++++- .../core/HTTP2JettyClientH2cFallbackTest.java | 89 +++++++++++++++++++ 2 files changed, 122 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java b/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java index a35d292e..20d1c418 100644 --- a/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java +++ b/src/main/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClient.java @@ -189,6 +189,17 @@ public class HTTP2JettyClient { private static final String ATTR_HTTP3_ATTEMPTED = "bzm.http3.attempted"; private static final String ATTR_H2C_FALLBACK_ATTEMPTED = "bzm.h2cFallbackAttempted"; private static final String ATTR_SKIP_H2C_UPGRADE = "bzm.skipH2cUpgrade"; + /** + * Statuses that answer an {@code Upgrade: h2c} attempt by refusing it rather than by serving the + * request: a bad request, an upgrade demanded or not implemented, a version not supported. Only + * these are worth sending again without the upgrade headers - see + * {@link #shouldRetryAfterFailedH2cUpgrade}. + */ + private static final Set H2C_UPGRADE_REFUSED_STATUSES = Set.of( + HttpStatus.BAD_REQUEST_400, + HttpStatus.UPGRADE_REQUIRED_426, + HttpStatus.NOT_IMPLEMENTED_501, + HttpStatus.HTTP_VERSION_NOT_SUPPORTED_505); private static final String ATTR_ORIGIN_KEY = "bzm.http3.origin"; private static final String ATTR_REQUEST_HEADERS_SERIALIZED = "bzm.request.headers.serialized"; /** @@ -1450,9 +1461,20 @@ private boolean wasH2cUpgradeAttempt(Request request) { } /** - * The server answered (no timeout/error) but never actually negotiated HTTP/2 despite our - * {@code Upgrade: h2c} attempt - e.g. it silently ignored the header, as a compliant HTTP/1.1 - * server that doesn't support h2c is allowed to do. Retry once with a plain HTTP/1.1 request. + * Whether the {@code Upgrade: h2c} attempt has to be made again as a plain HTTP/1.1 request. + * + *

Only when the answer says the upgrade attempt itself was refused. A compliant HTTP/1.1 + * server that does not speak h2c ignores the header and serves the request over HTTP/1.1, and + * that response is the answer to this request: re-sending it would put the same request + * on the wire twice - twice the load, and twice the side effect for anything that is not a GET - + * and would report only the second attempt's time. What is left of the failed negotiation is + * remembered by {@link #updateHttp1OnlyCache}, so the requests after it skip the upgrade instead + * of paying for it again. + * + *

A server or proxy that answers the upgrade headers with one of + * {@link #H2C_UPGRADE_REFUSED_STATUSES} did not serve the request, it rejected the attempt, and + * that is a failure this client caused by adding headers the test plan never asked for. Those are + * retried once, without the headers. */ private boolean shouldRetryAfterFailedH2cUpgrade(Request request, ContentResponse response) { if (!enableHttp1 || !http1UpgradeRequired || request == null || response == null) { @@ -1469,7 +1491,8 @@ private boolean shouldRetryAfterFailedH2cUpgrade(Request request, ContentRespons if (!wasH2cUpgradeAttempt(request)) { return false; } - return response.getVersion() != HttpVersion.HTTP_2; + return response.getVersion() != HttpVersion.HTTP_2 + && H2C_UPGRADE_REFUSED_STATUSES.contains(response.getStatus()); } private void markCleartextHttp1Only(URI uri) { @@ -4196,8 +4219,14 @@ private void setHeaders(Request request, URL url, HeaderManager headerManager) { // 1. The connection is already HTTP/2 (negotiated via ALPN) // 2. Upgrade headers are for cleartext HTTP, not HTTPS // 3. It violates the HTTP/2 protocol (RFC 7540) + // An origin already known to answer HTTP/1.1 is not asked to upgrade again: the attempt costs + // three headers and Jetty's upgrade machinery on every request, and the negotiation it asks for + // is one this client already watched fail. This is the "later requests skip the futile upgrade + // attempt" the HTTP/1.1-only cache is written for - until now the cache only steered which + // client was used, and the headers went out regardless. if (http1UpgradeRequired && enableHttp2 && !"https".equalsIgnoreCase(url.getProtocol()) && !shouldUseH2cPriorKnowledge(request.getURI()) + && !isHttp1Only(request.getURI()) && !Boolean.TRUE.equals(request.getAttributes().get(ATTR_SKIP_H2C_UPGRADE))) { Mutable headers = ((Mutable) request.getHeaders()); addHeaderIfMissing(HttpHeader.UPGRADE, "h2c", headers); diff --git a/src/test/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClientH2cFallbackTest.java b/src/test/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClientH2cFallbackTest.java index d1d203e6..c36e05e0 100644 --- a/src/test/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClientH2cFallbackTest.java +++ b/src/test/java/com/blazemeter/jmeter/http2/core/HTTP2JettyClientH2cFallbackTest.java @@ -1,6 +1,8 @@ package com.blazemeter.jmeter.http2.core; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import com.blazemeter.jmeter.http2.HTTP2TestBase; import com.blazemeter.jmeter.http2.core.ServerBuilder.TeardownableServer; @@ -10,7 +12,9 @@ import java.net.URL; import java.nio.channels.ClosedChannelException; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.jmeter.protocol.http.sampler.HTTPSampleResult; +import org.eclipse.jetty.client.ContentResponse; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.Request; import org.eclipse.jetty.http.HttpFields; @@ -81,6 +85,81 @@ public void secondRequestToSameOriginSkipsRepeatedFailedH2cUpgrade() throws Exce } } + /** + * A server that ignores {@code Upgrade: h2c} and answers the request over HTTP/1.1 has answered + * it: that response is the sample. Re-sending would hit the server twice for one sampler, which + * for anything other than a GET means the side effect happens twice, and would report only the + * second attempt's time. + */ + @Test + public void anUpgradeAttemptAnsweredOverHttp11PutsOneRequestOnTheWire() throws Exception { + AtomicInteger requests = new AtomicInteger(); + int port = startHttp1OnlyServer(requests); + // http1UpgradeRequired=true is what makes this request carry the h2c upgrade headers. + HTTP2JettyClient client = new HTTP2JettyClient(true, "h2c-single-request-test"); + client.start(); + try { + HTTPSampleResult result = sampleGet(client, port); + + assertThat(result.isSuccessful()).isTrue(); + assertThat(result.getResponseCode()).isEqualTo("200"); + assertThat(requests.get()).as("requests that reached the server").isEqualTo(1); + } finally { + client.stop(); + } + } + + /** And the origin is remembered, so the requests after it are not upgrade attempts either. */ + @Test + public void furtherRequestsToAnHttp11OriginPutOneRequestEachOnTheWire() throws Exception { + AtomicInteger requests = new AtomicInteger(); + int port = startHttp1OnlyServer(requests); + HTTP2JettyClient client = new HTTP2JettyClient(true, "h2c-single-request-cache-test"); + client.start(); + try { + sampleGet(client, port); + sampleGet(client, port); + sampleGet(client, port); + + assertThat(requests.get()).as("requests that reached the server for three samplers") + .isEqualTo(3); + } finally { + client.stop(); + } + } + + /** + * The other half of the rule: a server or proxy that answers the upgrade headers by refusing them + * did not serve the request, and that failure is one this client caused by adding headers the test + * plan never asked for. Those are still sent again without them. + */ + @Test + public void onlyAnAnswerThatRefusesTheUpgradeIsSentAgain() throws Exception { + HTTP2JettyClient client = new HTTP2JettyClient(true, "h2c-retry-decision-test"); + Request upgradeAttempt = newProbeRequest("http://example.invalid/") + .headers(h -> h.put(HttpHeader.UPGRADE, "h2c")); + + for (int refused : new int[] {400, 426, 501, 505}) { + assertThat(shouldRetry(client, upgradeAttempt, refused)) + .as("status %s refuses the upgrade, so the request must be sent again", refused) + .isTrue(); + } + for (int served : new int[] {200, 201, 204, 301, 401, 403, 404, 500, 503}) { + assertThat(shouldRetry(client, upgradeAttempt, served)) + .as("status %s is an answer to the request, so it must not be sent again", served) + .isFalse(); + } + } + + private static boolean shouldRetry(HTTP2JettyClient client, Request request, int status) + throws Exception { + ContentResponse response = mock(ContentResponse.class); + when(response.getVersion()).thenReturn(HttpVersion.HTTP_1_1); + when(response.getStatus()).thenReturn(status); + return (Boolean) invokePrivate(client, "shouldRetryAfterFailedH2cUpgrade", + new Class[] {Request.class, ContentResponse.class}, request, response); + } + @Test public void wasH2cUpgradeAttemptDetectsUpgradeHeader() throws Exception { HTTP2JettyClient client = newClientForReflection(); @@ -132,7 +211,17 @@ public void isClosedChannelFailureFindsItAnywhereInCauseChain() throws Exception } private int startHttp1OnlyServer() throws Exception { + return startHttp1OnlyServer(null); + } + + /** + * @param requestCounter counts every request the server answered, or {@code null} to not count + */ + private int startHttp1OnlyServer(AtomicInteger requestCounter) throws Exception { server = new ServerBuilder().withHTTP1().buildServer(); + if (requestCounter != null) { + server.setRequestLog((request, response) -> requestCounter.incrementAndGet()); + } server.start(); return ((ServerConnector) server.getConnectors()[0]).getLocalPort(); }