From 884cb0e0a56d7882fd586a94078d32cdd162f16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niccol=C3=B2=20Maltoni?= Date: Mon, 2 Dec 2024 13:10:49 +0100 Subject: [PATCH] feat: clean HTTP 1.1 to 2 upgrade headers we don't want to upgrade to HTTP/2 if Carapace supports it, but the backend doesn't fixes #484 --- .../core/ProxyRequestsManager.java | 22 ++++- .../carapaceproxy/core/Http2HeadersTest.java | 91 +++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 carapace-server/src/test/java/org/carapaceproxy/core/Http2HeadersTest.java diff --git a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java index 507735b5d..ac70c37cd 100644 --- a/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java +++ b/carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java @@ -37,6 +37,7 @@ import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpVersion; +import io.netty.handler.codec.http2.Http2CodecUtil; import io.netty.handler.timeout.ReadTimeoutException; import io.prometheus.client.Counter; import io.prometheus.client.Gauge; @@ -437,8 +438,25 @@ public Publisher forward(final ProxyRequest request, final boolean cache, return forwarder.request(request.getMethod()) .uri(request.getUri()) .send((req, out) -> { - // client request headers - req.headers(request.getRequestHeaders().copy()); + // we don't want to upgrade to HTTP/2 if Carapace supports it, but the backend doesn't + final HttpHeaders copy = request.getRequestHeaders().copy(); + if (copy.contains(HttpHeaderNames.UPGRADE)) { + final List upgrade = copy.getAll(HttpHeaderNames.UPGRADE); + if (upgrade.contains("HTTP/2")) { + // we drop connection and upgrade only if the target upgrade is HTTP/2.0; + // else, we want to preserve upgrades to HTTPS or similar + final List connection = copy.getAll(HttpHeaderNames.CONNECTION); + connection.removeIf("upgrade"::equalsIgnoreCase); + copy.remove(HttpHeaderNames.CONNECTION); + copy.add(HttpHeaderNames.CONNECTION, connection); + + upgrade.remove("HTTP/2"); + copy.remove(HttpHeaderNames.UPGRADE); + copy.add(HttpHeaderNames.UPGRADE, upgrade); + } + } + copy.remove(Http2CodecUtil.HTTP_UPGRADE_SETTINGS_HEADER); + req.headers(copy); // netty overrides the value, we need to force it req.header(HttpHeaderNames.HOST, request.getRequestHeaders().get(HttpHeaderNames.HOST)); return out.send(request.getRequestData()); // client request body diff --git a/carapace-server/src/test/java/org/carapaceproxy/core/Http2HeadersTest.java b/carapace-server/src/test/java/org/carapaceproxy/core/Http2HeadersTest.java new file mode 100644 index 000000000..3c6d68946 --- /dev/null +++ b/carapace-server/src/test/java/org/carapaceproxy/core/Http2HeadersTest.java @@ -0,0 +1,91 @@ +package org.carapaceproxy.core; + +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.core.Options.DYNAMIC_PORT; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_FORWARDED_STRATEGY; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_KEEP_ALIVE; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_KEEP_ALIVE_COUNT; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_KEEP_ALIVE_IDLE; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_KEEP_ALIVE_INTERVAL; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_MAX_KEEP_ALIVE_REQUESTS; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_SO_BACKLOG; +import static org.carapaceproxy.server.config.NetworkListenerConfiguration.DEFAULT_SSL_PROTOCOLS; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import io.netty.handler.codec.http.HttpHeaderNames; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.HttpVersion; +import java.io.IOException; +import java.util.Set; +import org.carapaceproxy.server.config.ConfigurationNotValidException; +import org.carapaceproxy.server.config.NetworkListenerConfiguration; +import org.carapaceproxy.utils.TestEndpointMapper; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import reactor.netty.http.HttpProtocol; +import reactor.netty.http.client.HttpClient; +import reactor.netty.http.client.HttpClientResponse; + +public class Http2HeadersTest { + + @Rule + public WireMockRule wireMockRule = new WireMockRule(options().dynamicPort()); + + @Rule + public TemporaryFolder tmpDir = new TemporaryFolder(); + + @Test + public void test() throws IOException, ConfigurationNotValidException, InterruptedException { + stubFor(get(urlEqualTo("/index.html")) + .willReturn(aResponse() + .withStatus(HttpResponseStatus.OK.code()) + .withHeader("Content-Type", "text/html") + .withHeader("Content-Length", String.valueOf("it works !!".length())) + .withBody("it works !!")) + ); + final var mapper = new TestEndpointMapper("localhost", wireMockRule.port()); + try (final var server = new HttpProxyServer(mapper, tmpDir.newFolder())) { + server.addListener(new NetworkListenerConfiguration( + "localhost", + DYNAMIC_PORT, + false, + null, + null, + DEFAULT_SSL_PROTOCOLS, + DEFAULT_SO_BACKLOG, + DEFAULT_KEEP_ALIVE, + DEFAULT_KEEP_ALIVE_IDLE, + DEFAULT_KEEP_ALIVE_INTERVAL, + DEFAULT_KEEP_ALIVE_COUNT, + DEFAULT_MAX_KEEP_ALIVE_REQUESTS, + DEFAULT_FORWARDED_STRATEGY, + Set.of(), + Set.of(HttpProtocol.H2C.name(), HttpProtocol.HTTP11.name()) + )); + + server.start(); + final var port = server.getLocalPort(); + final HttpClientResponse response = HttpClient.create() + .protocol(HttpProtocol.H2C, HttpProtocol.HTTP11) + .headers(headers -> headers + .add(HttpHeaderNames.CONNECTION, "keep-alive") + .add(HttpHeaderNames.CONNECTION, "upgrade") + .add(HttpHeaderNames.UPGRADE, "HTTP/2.0") + ) + .get() + .uri("http://localhost:" + port + "/index.html") + .response() + .block(); + assertThat(response, is(notNullValue())); + assertThat(response.status(), is(HttpResponseStatus.OK)); + assertThat(response.version(), is(HttpVersion.HTTP_1_1)); + } + } +}