From 2749bc2bbe9e4d0622d35525261b52995f0d924e Mon Sep 17 00:00:00 2001 From: Jorge Bescos Gascon Date: Tue, 1 Aug 2023 15:21:19 +0200 Subject: [PATCH] HTTP 1 parser, CONNECT host:port fix, HTTP2 support tests Signed-off-by: Jorge Bescos Gascon --- .../integration/webclient/webclient/pom.xml | 4 + .../integration/webclient/HttpProxy.java | 24 ++--- .../integration/webclient/HttpProxyTest.java | 102 +++++++++++++----- .../integration/webclient/HttpsProxyTest.java | 83 +++++++++----- .../webclient/http1/Http1CallChainBase.java | 25 +++-- .../webclient/http1/Http1StatusParser.java | 14 +-- 6 files changed, 168 insertions(+), 84 deletions(-) diff --git a/nima/tests/integration/webclient/webclient/pom.xml b/nima/tests/integration/webclient/webclient/pom.xml index f1fbce70400..570b0a31792 100644 --- a/nima/tests/integration/webclient/webclient/pom.xml +++ b/nima/tests/integration/webclient/webclient/pom.xml @@ -32,6 +32,10 @@ io.helidon.nima.webclient helidon-nima-webclient + + io.helidon.nima.http2 + helidon-nima-http2-webclient + io.helidon.nima.webserver helidon-nima-webserver diff --git a/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxy.java b/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxy.java index ec199690bdf..3e15e6dc77c 100644 --- a/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxy.java +++ b/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxy.java @@ -24,6 +24,7 @@ import java.net.Socket; import java.util.Arrays; import java.util.Base64; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -53,7 +54,8 @@ class HttpProxy { this(port, null, null); } - boolean start() { + void start() { + CountDownLatch ready = new CountDownLatch(1); executor.submit(() -> { try (ServerSocket server = new ServerSocket(port)) { this.connectedPort = server.getLocalPort(); @@ -62,6 +64,7 @@ boolean start() { Socket origin = server.accept(); LOGGER.log(Level.DEBUG, "Open: " + origin); counter.incrementAndGet(); + ready.countDown(); origin.setSoTimeout(TIMEOUT); Socket remote = new Socket(); remote.setSoTimeout(TIMEOUT); @@ -82,9 +85,10 @@ boolean start() { try (Socket socket = new Socket()) { socket.connect(new InetSocketAddress(connectedPort), 10000); responding = true; - } catch (IOException e) {} + // Wait for counter is set to 0 + ready.await(5, TimeUnit.SECONDS); + } catch (IOException | InterruptedException e) {} } - return responding; } int counter() { @@ -112,7 +116,6 @@ private class MiddleCommunicator { private static final System.Logger LOGGER = System.getLogger(MiddleCommunicator.class.getName()); private static final int BUFFER_SIZE = 1024 * 1024; - private static final String HOST = "HOST: "; private final ExecutorService executor; private final Socket readerSocket; private final Socket writerSocket; @@ -169,7 +172,7 @@ public void run() { if (originInfo.respondOrigin()) { if (authenticate(originInfo)) { // Respond origin - String response = "HTTP/1.1 200 Connection established\r\n\r\n"; + String response = "HTTP/1.0 200 Connection established\r\n\r\n"; writerSocket.connect(new InetSocketAddress(originInfo.host, originInfo.port)); LOGGER.log(Level.DEBUG, "Open: " + writerSocket); readerSocket.getOutputStream() @@ -180,7 +183,7 @@ public void run() { } else { LOGGER.log(Level.WARNING, "Invalid " + originInfo.user + ":" + originInfo.password); originInfo = null; - String response = "HTTP/1.1 401 Unauthorized\r\n\r\n"; + String response = "HTTP/1.0 401 Unauthorized\r\n\r\n"; readerSocket.getOutputStream().write(response.getBytes()); readerSocket.getOutputStream().flush(); readerSocket.close(); @@ -222,8 +225,6 @@ private OriginInfo getOriginInfo(byte[] buffer, int read) throws MalformedURLExc for (String line : lines) { if (line.startsWith(OriginInfo.CONNECT)) { request.parseFirstLine(line); - } else if (line.toUpperCase().startsWith(HOST)) { - request.parseHost(line); } else if (line.toUpperCase().startsWith(OriginInfo.AUTHORIZATION)) { request.parseAuthorization(line); } @@ -261,12 +262,7 @@ private void parseFirstLine(String line) { String[] parts = line.split(" "); this.method = parts[0].trim(); this.protocol = parts[2].trim(); - } - - // Host: host:port - private void parseHost(String line) { - line = line.substring(HOST.length()).trim(); - String[] hostPort = line.split(":"); + String[] hostPort = parts[1].split(":"); this.host = hostPort[0]; if (hostPort.length > 1) { this.port = Integer.parseInt(hostPort[1]); diff --git a/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxyTest.java b/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxyTest.java index 237825cc736..c534cf6f7b0 100644 --- a/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxyTest.java +++ b/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpProxyTest.java @@ -24,12 +24,15 @@ import java.net.ProxySelector; import io.helidon.common.http.Http; +import io.helidon.nima.http2.webclient.Http2Client; import io.helidon.nima.testing.junit5.webserver.ServerTest; import io.helidon.nima.testing.junit5.webserver.SetUpRoute; +import io.helidon.nima.webclient.api.HttpClient; +import io.helidon.nima.webclient.api.HttpClientResponse; import io.helidon.nima.webclient.api.Proxy; import io.helidon.nima.webclient.api.Proxy.ProxyType; import io.helidon.nima.webclient.http1.Http1Client; -import io.helidon.nima.webclient.http1.Http1ClientResponse; +import io.helidon.nima.webserver.WebServer; import io.helidon.nima.webserver.http.HttpRouting; import org.junit.jupiter.api.AfterEach; @@ -43,7 +46,8 @@ class HttpProxyTest { private int proxyPort; private HttpProxy httpProxy; - private final Http1Client client; + private final HttpClient clientHttp1; + private final HttpClient clientHttp2; @SetUpRoute static void routing(HttpRouting.Builder router) { @@ -55,6 +59,7 @@ public void before() { httpProxy = new HttpProxy(0); httpProxy.start(); proxyPort = httpProxy.connectedPort(); + assertThat(httpProxy.counter(), is(0)); } @AfterEach @@ -62,63 +67,104 @@ public void after() { httpProxy.stop(); } - HttpProxyTest(Http1Client client) { - this.client = client; + HttpProxyTest(WebServer server) { + String uri = "http://localhost:" + server.port(); + this.clientHttp1 = Http1Client.builder().baseUri(uri).build(); + this.clientHttp2 = Http2Client.builder().baseUri(uri).build(); } @Test - void testNoProxy() { - noProxyChecks(); + void testNoProxy1() { + noProxyChecks(clientHttp1); } @Test - void testNoProxyTypeDefaultsToNone() { - noProxyChecks(); + void testNoProxy2() { + noProxyChecks(clientHttp2); } @Test - void testNoHosts() { - Proxy proxy = Proxy.builder().host(PROXY_HOST).port(proxyPort).addNoProxy(PROXY_HOST).build(); - try (Http1ClientResponse response = client.get("/get").proxy(proxy).request()) { - assertThat(response.status(), is(Http.Status.OK_200)); - String entity = response.entity().as(String.class); - assertThat(entity, is("Hello")); - } - assertThat(httpProxy.counter(), is(0)); + void testNoHosts1() { + noHosts(clientHttp1); + } + + @Test + void testNoHosts2() { + noHosts(clientHttp2); } @Test - void testNoProxyTypeButHasHost() { + void testNoProxyTypeButHasHost1() { Proxy proxy = Proxy.builder().host(PROXY_HOST).port(proxyPort).build(); - successVerify(proxy); + successVerify(proxy, clientHttp1); } @Test - void testProxyNoneTypeButHasHost() { + void testNoProxyTypeButHasHost2() { + Proxy proxy = Proxy.builder().host(PROXY_HOST).port(proxyPort).build(); + successVerify(proxy, clientHttp2); + } + + @Test + void testProxyNoneTypeButHasHost1() { + Proxy proxy = Proxy.builder().type(ProxyType.NONE).host(PROXY_HOST).port(proxyPort).build(); + successVerify(proxy, clientHttp1); + } + + @Test + void testProxyNoneTypeButHasHost2() { Proxy proxy = Proxy.builder().type(ProxyType.NONE).host(PROXY_HOST).port(proxyPort).build(); - successVerify(proxy); + successVerify(proxy, clientHttp2); + } + + @Test + void testSimpleProxy1() { + Proxy proxy = Proxy.builder().type(ProxyType.HTTP).host(PROXY_HOST).port(proxyPort).build(); + successVerify(proxy, clientHttp1); } @Test - void testSimpleProxy() { + void testSimpleProxy2() { Proxy proxy = Proxy.builder().type(ProxyType.HTTP).host(PROXY_HOST).port(proxyPort).build(); - successVerify(proxy); + successVerify(proxy, clientHttp2); + } + + @Test + void testSystemProxy1() { + ProxySelector original = ProxySelector.getDefault(); + try { + ProxySelector.setDefault(ProxySelector.of(new InetSocketAddress(PROXY_HOST, proxyPort))); + Proxy proxy = Proxy.create(); + successVerify(proxy, clientHttp1); + } finally { + ProxySelector.setDefault(original); + } } @Test - void testSystemProxy() { + void testSystemProxy2() { ProxySelector original = ProxySelector.getDefault(); try { ProxySelector.setDefault(ProxySelector.of(new InetSocketAddress(PROXY_HOST, proxyPort))); Proxy proxy = Proxy.create(); - successVerify(proxy); + successVerify(proxy, clientHttp2); } finally { ProxySelector.setDefault(original); } } - private void successVerify(Proxy proxy) { - try (Http1ClientResponse response = client.get("/get").proxy(proxy).request()) { + private void noHosts(HttpClient client) { + Proxy proxy = Proxy.builder().host(PROXY_HOST).port(proxyPort).addNoProxy(PROXY_HOST).build(); + try (HttpClientResponse response = client.get("/get").proxy(proxy).request()) { + assertThat(response.status(), is(Http.Status.OK_200)); + String entity = response.entity().as(String.class); + assertThat(entity, is("Hello")); + } + assertThat(httpProxy.counter(), is(0)); + } + + private void successVerify(Proxy proxy, HttpClient client) { + try (HttpClientResponse response = client.get("/get").proxy(proxy).request()) { assertThat(response.status(), is(Http.Status.OK_200)); String entity = response.entity().as(String.class); assertThat(entity, is("Hello")); @@ -126,8 +172,8 @@ private void successVerify(Proxy proxy) { assertThat(httpProxy.counter(), is(1)); } - private void noProxyChecks() { - try (Http1ClientResponse response = client.get("/get").request()) { + private void noProxyChecks(HttpClient client) { + try (HttpClientResponse response = client.get("/get").request()) { assertThat(response.status(), is(Http.Status.OK_200)); String entity = response.entity().as(String.class); assertThat(entity, is("Hello")); diff --git a/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpsProxyTest.java b/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpsProxyTest.java index 7f3752772fc..88eafb407a7 100644 --- a/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpsProxyTest.java +++ b/nima/tests/integration/webclient/webclient/src/test/java/io/helidon/nima/tests/integration/webclient/HttpsProxyTest.java @@ -16,6 +16,10 @@ package io.helidon.nima.tests.integration.webclient; +import static io.helidon.common.http.Http.Method.GET; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + import java.io.IOException; import java.net.InetSocketAddress; import java.net.ProxySelector; @@ -24,13 +28,15 @@ import io.helidon.common.http.Http; import io.helidon.common.pki.Keys; import io.helidon.nima.common.tls.Tls; +import io.helidon.nima.http2.webclient.Http2Client; import io.helidon.nima.testing.junit5.webserver.ServerTest; import io.helidon.nima.testing.junit5.webserver.SetUpRoute; import io.helidon.nima.testing.junit5.webserver.SetUpServer; +import io.helidon.nima.webclient.api.HttpClient; +import io.helidon.nima.webclient.api.HttpClientResponse; import io.helidon.nima.webclient.api.Proxy; import io.helidon.nima.webclient.api.Proxy.ProxyType; import io.helidon.nima.webclient.http1.Http1Client; -import io.helidon.nima.webclient.http1.Http1ClientResponse; import io.helidon.nima.webserver.WebServer; import io.helidon.nima.webserver.WebServerConfig.Builder; import io.helidon.nima.webserver.http.HttpRouting; @@ -39,10 +45,6 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import static io.helidon.common.http.Http.Method.GET; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.MatcherAssert.assertThat; - @ServerTest class HttpsProxyTest { @@ -50,7 +52,8 @@ class HttpsProxyTest { private static int PROXY_PORT; private static HttpProxy httpProxy; - private final Http1Client client; + private final HttpClient clientHttp1; + private final HttpClient clientHttp2; HttpsProxyTest(WebServer server) { int port = server.port(); @@ -60,10 +63,8 @@ class HttpsProxyTest { .endpointIdentificationAlgorithm(Tls.ENDPOINT_IDENTIFICATION_NONE) .build(); - client = Http1Client.builder() - .baseUri("https://localhost:" + port) - .tls(tls) - .build(); + this.clientHttp1 = Http1Client.builder().baseUri("https://localhost:" + port).tls(tls).build(); + this.clientHttp2 = Http2Client.builder().baseUri("https://localhost:" + port).tls(tls).build(); } @BeforeAll @@ -102,55 +103,85 @@ static void routing(HttpRouting.Builder router) { } @Test - void testNoProxy() { - noProxyChecks(); + void testNoProxy1() { + noProxyChecks(clientHttp1); + } + + @Test + void testNoProxy2() { + noProxyChecks(clientHttp2); } @Test - void testNoProxyTypeDefaultsToNone() { - noProxyChecks(); + void testNoProxyTypeButHasHost1() { + Proxy proxy = Proxy.builder().host(PROXY_HOST).port(PROXY_PORT).build(); + successVerify(proxy, clientHttp1); } @Test - void testNoProxyTypeButHasHost() { + void testNoProxyTypeButHasHost2() { Proxy proxy = Proxy.builder().host(PROXY_HOST).port(PROXY_PORT).build(); - successVerify(proxy); + successVerify(proxy, clientHttp2); } @Test - void testProxyNoneTypeButHasHost() { + void testProxyNoneTypeButHasHost1() { Proxy proxy = Proxy.builder().type(ProxyType.NONE).host(PROXY_HOST).port(PROXY_PORT).build(); - successVerify(proxy); + successVerify(proxy, clientHttp1); } @Test - void testSimpleProxy() { + void testProxyNoneTypeButHasHost2() { + Proxy proxy = Proxy.builder().type(ProxyType.NONE).host(PROXY_HOST).port(PROXY_PORT).build(); + successVerify(proxy, clientHttp2); + } + + @Test + void testSimpleProxy1() { + Proxy proxy = Proxy.builder().type(ProxyType.HTTP).host(PROXY_HOST).port(PROXY_PORT).build(); + successVerify(proxy, clientHttp1); + } + + @Test + void testSimpleProxy2() { Proxy proxy = Proxy.builder().type(ProxyType.HTTP).host(PROXY_HOST).port(PROXY_PORT).build(); - successVerify(proxy); + successVerify(proxy, clientHttp2); + } + + @Test + void testSystemProxy1() { + ProxySelector original = ProxySelector.getDefault(); + try { + ProxySelector.setDefault(ProxySelector.of(new InetSocketAddress(PROXY_HOST, PROXY_PORT))); + Proxy proxy = Proxy.create(); + successVerify(proxy, clientHttp1); + } finally { + ProxySelector.setDefault(original); + } } @Test - void testSystemProxy() { + void testSystemProxy2() { ProxySelector original = ProxySelector.getDefault(); try { ProxySelector.setDefault(ProxySelector.of(new InetSocketAddress(PROXY_HOST, PROXY_PORT))); Proxy proxy = Proxy.create(); - successVerify(proxy); + successVerify(proxy, clientHttp2); } finally { ProxySelector.setDefault(original); } } - private void successVerify(Proxy proxy) { - try (Http1ClientResponse response = client.get("/get").proxy(proxy).request()) { + private void successVerify(Proxy proxy, HttpClient client) { + try (HttpClientResponse response = client.get("/get").proxy(proxy).request()) { assertThat(response.status(), is(Http.Status.OK_200)); String entity = response.entity().as(String.class); assertThat(entity, is("Hello")); } } - private void noProxyChecks() { - try (Http1ClientResponse response = client.get("/get").request()) { + private void noProxyChecks(HttpClient client) { + try (HttpClientResponse response = client.get("/get").request()) { assertThat(response.status(), is(Http.Status.OK_200)); String entity = response.entity().as(String.class); assertThat(entity, is("Hello")); diff --git a/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1CallChainBase.java b/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1CallChainBase.java index b6770c4a645..088d0e34f04 100644 --- a/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1CallChainBase.java +++ b/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1CallChainBase.java @@ -30,6 +30,8 @@ import io.helidon.common.http.ClientResponseHeaders; import io.helidon.common.http.Headers; import io.helidon.common.http.Http; +import io.helidon.common.http.Http.Header; +import io.helidon.common.http.Http.Method; import io.helidon.common.http.Http1HeadersParser; import io.helidon.common.http.WritableHeaders; import io.helidon.common.socket.HelidonSocket; @@ -116,15 +118,20 @@ abstract WebClientServiceResponse doProceed(ClientConnection connection, BufferData writeBuffer); void prologue(BufferData nonEntityData, WebClientServiceRequest request, ClientUri uri) { - // TODO When proxy is implemented, change default value of Http1ClientConfig.relativeUris to false - // and below conditional statement to: - // proxy == Proxy.noProxy() || proxy.noProxyPredicate().apply(finalUri) || clientConfig.relativeUris - String schemeHostPort = clientConfig.relativeUris() ? "" : uri.scheme() + "://" + uri.host() + ":" + uri.port(); - nonEntityData.writeAscii(request.method().text() - + " " - + schemeHostPort - + uri.pathWithQueryAndFragment() - + " HTTP/1.1\r\n"); + if (request.method() == Method.CONNECT) { + // When CONNECT, the first line contains the remote host:port, in the same way as the HOST header. + nonEntityData.writeAscii(request.method().text() + + " " + + request.headers().get(Header.HOST).value() + + " HTTP/1.1\r\n"); + } else { + String schemeHostPort = clientConfig.relativeUris() ? "" : uri.scheme() + "://" + uri.host() + ":" + uri.port(); + nonEntityData.writeAscii(request.method().text() + + " " + + schemeHostPort + + uri.pathWithQueryAndFragment() + + " HTTP/1.1\r\n"); + } } ClientResponseHeaders readHeaders(DataReader reader) { diff --git a/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1StatusParser.java b/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1StatusParser.java index 2f16de7baea..3b3c636235f 100644 --- a/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1StatusParser.java +++ b/nima/webclient/http1/src/main/java/io/helidon/nima/webclient/http1/Http1StatusParser.java @@ -24,14 +24,14 @@ import io.helidon.common.http.Http; /** - * Parser of HTTP/1.1 response status. + * Parser of HTTP/1.0 or HTTP/1.1 response status. */ public final class Http1StatusParser { private Http1StatusParser() { } /** - * Read the status line from HTTP/1.1 response. + * Read the status line from HTTP/1.0 or HTTP/1.1 response. * * @param reader data reader to obtain bytes from * @param maxLength maximal number of bytes that can be processed before end of line is reached @@ -73,15 +73,15 @@ public static Http.Status readStatus(DataReader reader, int maxLength) { reader.skip(1); // space newLine -= space; newLine--; - if (!protocolVersion.equals("1.1")) { - throw new IllegalStateException("HTTP response did not contain correct status line. Version is not 1.1: \n" + if (!protocolVersion.equals("1.0") && !protocolVersion.equals("1.1")) { + throw new IllegalStateException("HTTP response did not contain correct status line. Version is not 1.0 or 1.1: \n" + BufferData.create(protocolVersion.getBytes(StandardCharsets.US_ASCII)) .debugDataHex()); } - // HTTP/1.1 200 OK + // HTTP/1.0 or HTTP/1.1 200 OK space = reader.findOrNewLine(Bytes.SPACE_BYTE, newLine); if (space == newLine) { - throw new IllegalStateException("HTTP Response did not contain HTTP status line. Line: HTTP/1.1\n" + throw new IllegalStateException("HTTP Response did not contain HTTP status line. Line: HTTP/1.0 or HTTP/1.1\n" + reader.readBuffer(newLine).debugDataHex()); } String code = reader.readAsciiString(space); @@ -94,7 +94,7 @@ public static Http.Status readStatus(DataReader reader, int maxLength) { try { return Http.Status.create(Integer.parseInt(code), phrase); } catch (NumberFormatException e) { - throw new IllegalStateException("HTTP Response did not contain HTTP status line. Line HTTP/1.1 \n" + throw new IllegalStateException("HTTP Response did not contain HTTP status line. Line HTTP/1.0 or HTTP/1.1 \n" + BufferData.create(code.getBytes(StandardCharsets.US_ASCII)) + "\n" + BufferData.create(phrase.getBytes(StandardCharsets.US_ASCII))); }