Skip to content

Commit

Permalink
feat: clean HTTP 1.1 to 2 upgrade headers
Browse files Browse the repository at this point in the history
we don't want to upgrade to HTTP/2 if Carapace supports it, but the backend doesn't

fixes #484
  • Loading branch information
NiccoMlt authored and dmercuriali committed Dec 3, 2024
1 parent 79dc901 commit 884cb0e
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -437,8 +438,25 @@ public Publisher<Void> 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<String> 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<String> 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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <b>works</b> !!".length()))
.withBody("it <b>works</b> !!"))
);
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));
}
}
}

0 comments on commit 884cb0e

Please sign in to comment.