diff --git a/pom.xml b/pom.xml index dcb99a6d96e..bd5f88935e0 100644 --- a/pom.xml +++ b/pom.xml @@ -79,6 +79,7 @@ 1.9.3 3.2.3 2.23.4 + 5.15.0 0.9.1 0.1.0 2.2.10 @@ -1035,6 +1036,11 @@ mockito-core ${version.lib.mockito} + + org.mock-server + mockserver-netty-no-dependencies + ${version.lib.mockserver} + org.eclipse.jgit org.eclipse.jgit.junit diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java b/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java index e07cb562396..bdd3f053203 100644 --- a/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java +++ b/webclient/api/src/main/java/io/helidon/webclient/api/ClientRequestBase.java @@ -59,6 +59,10 @@ public abstract class ClientRequestBase, R extends Ht */ public static final Header USER_AGENT_HEADER = HeaderValues.create(HeaderNames.USER_AGENT, "Helidon " + Version.VERSION); + /** + * Proxy connection header. + */ + public static final Header PROXY_CONNECTION = HeaderValues.create("Proxy-Connection", "keep-alive"); private static final Map COUNTERS = new ConcurrentHashMap<>(); private static final Set SUPPORTED_SCHEMES = Set.of("https", "http"); @@ -253,7 +257,7 @@ public T proxy(Proxy proxy) { @Override public R request() { - headers.setIfAbsent(USER_AGENT_HEADER); + additionalHeaders(); return validateAndSubmit(BufferData.EMPTY_BYTES); } @@ -262,17 +266,24 @@ public final R submit(Object entity) { if (!(entity instanceof byte[] bytes && bytes.length == 0)) { rejectHeadWithEntity(); } - headers.setIfAbsent(USER_AGENT_HEADER); + additionalHeaders(); return validateAndSubmit(entity); } @Override public final R outputStream(OutputStreamHandler outputStreamConsumer) { rejectHeadWithEntity(); - headers.setIfAbsent(USER_AGENT_HEADER); + additionalHeaders(); return doOutputStream(outputStreamConsumer); } + /** + * Append additional headers before sending the request. + */ + protected void additionalHeaders() { + headers.setIfAbsent(USER_AGENT_HEADER); + } + /** * HTTP method to be invoked. * diff --git a/webclient/api/src/main/java/io/helidon/webclient/api/Proxy.java b/webclient/api/src/main/java/io/helidon/webclient/api/Proxy.java index 269d53136fc..22749b614e3 100644 --- a/webclient/api/src/main/java/io/helidon/webclient/api/Proxy.java +++ b/webclient/api/src/main/java/io/helidon/webclient/api/Proxy.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. + * Copyright (c) 2023, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -58,8 +58,6 @@ public class Proxy { private static final System.Logger LOGGER = System.getLogger(Proxy.class.getName()); private static final Tls NO_TLS = Tls.builder().enabled(false).build(); - private static final Header PROXY_CONNECTION = - HeaderValues.create("Proxy-Connection", "keep-alive"); /** * No proxy instance. @@ -90,6 +88,7 @@ public class Proxy { private final Optional password; private final ProxySelector systemProxySelector; private final Optional
proxyAuthHeader; + private final boolean forceHttpConnect; private Proxy(Proxy.Builder builder) { this.host = builder.host(); @@ -102,6 +101,7 @@ private Proxy(Proxy.Builder builder) { this.port = builder.port(); this.username = builder.username(); this.password = builder.password(); + this.forceHttpConnect = builder.forceHttpConnect(); if (type == ProxyType.SYSTEM) { this.noProxy = inetSocketAddress -> true; @@ -451,7 +451,8 @@ private static Optional isIpV6HostRegExp(String host) { private static Socket connectToProxy(WebClient webClient, InetSocketAddress proxyAddress, InetSocketAddress targetAddress, - Proxy proxy) { + Proxy proxy, + boolean tls) { WebClientConfig clientConfig = webClient.prototype(); TcpClientConnection connection = TcpClientConnection.create(webClient, @@ -469,26 +470,26 @@ private static Socket connectToProxy(WebClient webClient, it -> { }) .connect(); - - HttpClientRequest request = webClient.method(Method.CONNECT) - .followRedirects(false) // do not follow redirects for proxy connect itself - .connection(connection) - .uri("http://" + proxyAddress.getHostName() + ":" + proxyAddress.getPort()) - .protocolId("http/1.1") // MUST be 1.1, if not available, proxy connection will fail - .header(HeaderNames.HOST, targetAddress.getHostName() + ":" + targetAddress.getPort()) - .accept(MediaTypes.WILDCARD); - if (clientConfig.keepAlive()) { - request.header(HeaderValues.CONNECTION_KEEP_ALIVE) - .header(PROXY_CONNECTION); - } - proxy.proxyAuthHeader.ifPresent(request::header); - // we cannot close the response, as that would close the connection - HttpClientResponse response = request.request(); - if (response.status().family() != Status.Family.SUCCESSFUL) { - response.close(); - throw new IllegalStateException("Proxy sent wrong HTTP response code: " + response.status()); + if (proxy.forceHttpConnect || tls || proxy.username.isPresent()) { + HttpClientRequest request = webClient.method(Method.CONNECT) + .followRedirects(false) // do not follow redirects for proxy connect itself + .connection(connection) + .uri("http://" + proxyAddress.getHostName() + ":" + proxyAddress.getPort()) + .protocolId("http/1.1") // MUST be 1.1, if not available, proxy connection will fail + .header(HeaderNames.HOST, targetAddress.getHostName() + ":" + targetAddress.getPort()) + .accept(MediaTypes.WILDCARD); + if (clientConfig.keepAlive()) { + request.header(HeaderValues.CONNECTION_KEEP_ALIVE) + .header(ClientRequestBase.PROXY_CONNECTION); + } + proxy.proxyAuthHeader.ifPresent(request::header); + // we cannot close the response, as that would close the connection + HttpClientResponse response = request.request(); + if (response.status().family() != Status.Family.SUCCESSFUL) { + response.close(); + throw new IllegalStateException("Proxy sent wrong HTTP response code: " + response.status()); + } } - return connection.socket(); } @@ -562,7 +563,8 @@ Socket connect(WebClient webClient, .map(proxyAddress -> connectToProxy(webClient, proxyAddress, targetAddress, - proxy)) + proxy, + tls)) .orElseGet(() -> NONE.connect(webClient, proxy, targetAddress, socketOptions, tls)); } }; @@ -587,6 +589,7 @@ public static class Builder implements io.helidon.common.Builder private int port = 80; private String username; private char[] password; + private boolean forceHttpConnect = false; private Builder() { } @@ -632,6 +635,11 @@ public Proxy build() { * Proxy password * * + * httpConnect + * {@code false} + * Specify whether the HTTP client will always execute HTTP CONNECT. + * + * * no-proxy * {@code no default} * Contains list of the hosts which should be excluded from using proxy @@ -667,6 +675,20 @@ public Builder type(ProxyType type) { return this; } + /** + * Forces HTTP CONNECT with the proxy server. + * Otherwise it will not execute HTTP CONNECT when the request is + * plain HTTP with no authentication. + * + * @param forceHttpConnect HTTP CONNECT + * @return updated builder instance + */ + @ConfiguredOption + public Builder forceHttpConnect(boolean forceHttpConnect) { + this.forceHttpConnect = forceHttpConnect; + return this; + } + /** * Sets a new host value. * @@ -742,6 +764,10 @@ ProxyType type() { return type; } + boolean forceHttpConnect() { + return forceHttpConnect; + } + String host() { return host; } diff --git a/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1ClientRequestImpl.java b/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1ClientRequestImpl.java index 85da8835b6e..22260adcaed 100644 --- a/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1ClientRequestImpl.java +++ b/webclient/http1/src/main/java/io/helidon/webclient/http1/Http1ClientRequestImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2022, 2023 Oracle and/or its affiliates. + * Copyright (c) 2022, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,6 +31,7 @@ import io.helidon.http.media.MediaContext; import io.helidon.webclient.api.ClientRequestBase; import io.helidon.webclient.api.ClientUri; +import io.helidon.webclient.api.Proxy.ProxyType; import io.helidon.webclient.api.WebClientServiceRequest; import io.helidon.webclient.api.WebClientServiceResponse; @@ -179,6 +180,14 @@ protected MediaContext mediaContext() { return super.mediaContext(); } + @Override + protected void additionalHeaders() { + super.additionalHeaders(); + if (proxy().type() != ProxyType.NONE) { + header(PROXY_CONNECTION); + } + } + Http1ClientImpl http1Client() { return http1Client; } diff --git a/webclient/tests/http1/pom.xml b/webclient/tests/http1/pom.xml index 318419589d8..cd2d23777f0 100644 --- a/webclient/tests/http1/pom.xml +++ b/webclient/tests/http1/pom.xml @@ -67,5 +67,10 @@ vertx-core test + + org.mock-server + mockserver-netty-no-dependencies + test + diff --git a/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpMockProxyTest.java b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpMockProxyTest.java new file mode 100644 index 00000000000..7b1b4fa841b --- /dev/null +++ b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpMockProxyTest.java @@ -0,0 +1,87 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.helidon.webclient.tests; + +import static io.helidon.http.Method.GET; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockserver.integration.ClientAndServer.startClientAndServer; +import static org.mockserver.model.HttpForward.forward; +import static org.mockserver.model.HttpRequest.request; + +import io.helidon.http.Status; +import io.helidon.webclient.api.HttpClientResponse; +import io.helidon.webclient.api.Proxy; +import io.helidon.webclient.api.WebClient; +import io.helidon.webserver.WebServer; +import io.helidon.webserver.http.HttpRouting; +import io.helidon.webserver.testing.junit5.ServerTest; +import io.helidon.webserver.testing.junit5.SetUpRoute; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockserver.integration.ClientAndServer; +import org.mockserver.model.HttpForward.Scheme; + +@ServerTest +class HttpMockProxyTest { + + private static final int PROXY_PORT = 1080; + private final WebClient webClient; + private final int serverPort; + private ClientAndServer mockServer; + + HttpMockProxyTest(WebServer server) { + this.serverPort = server.port(); + this.webClient = WebClient.builder() + .baseUri("http://localhost:" + serverPort) + .proxy(Proxy.builder().host("localhost").port(PROXY_PORT).build()) + .build(); + } + + @SetUpRoute + static void routing(HttpRouting.Builder router) { + router.route(GET, "/get", Routes::get); + } + + @BeforeEach + public void before() { + mockServer = startClientAndServer(PROXY_PORT); + mockServer.when(request()).forward(forward().withScheme(Scheme.HTTP).withHost("localhost").withPort(serverPort)); + } + + @AfterEach + public void after() { + mockServer.stop(); + } + + @Test + public void issue9022() { + try (HttpClientResponse response = webClient.get("/get").request()) { + assertThat(response.status(), is(Status.OK_200)); + String entity = response.entity().as(String.class); + assertThat(entity, is("Hello")); + } + } + + private static class Routes { + private static String get() { + return "Hello"; + } + } +} diff --git a/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpProxyTest.java b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpProxyTest.java index 17dec4e243d..973e9f813c3 100644 --- a/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpProxyTest.java +++ b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpProxyTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. + * Copyright (c) 2023, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ import java.net.ProxySelector; import io.helidon.http.Status; +import io.helidon.webclient.api.ClientRequest; +import io.helidon.webclient.api.ClientRequestBase; import io.helidon.webclient.api.HttpClient; import io.helidon.webclient.api.HttpClientResponse; import io.helidon.webclient.api.Proxy; @@ -39,6 +41,8 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; @ServerTest class HttpProxyTest { @@ -128,37 +132,37 @@ void testNoHosts2() { @Test void testNoProxyTypeButHasHost1() { - Proxy proxy = Proxy.builder().host(PROXY_HOST).port(proxyPort).build(); + Proxy proxy = Proxy.builder().forceHttpConnect(true).host(PROXY_HOST).port(proxyPort).build(); successVerify(proxy, clientHttp1); } @Test void testNoProxyTypeButHasHost2() { - Proxy proxy = Proxy.builder().host(PROXY_HOST).port(proxyPort).build(); + Proxy proxy = Proxy.builder().forceHttpConnect(true).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(); + Proxy proxy = Proxy.builder().forceHttpConnect(true).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(); + Proxy proxy = Proxy.builder().forceHttpConnect(true).type(ProxyType.NONE).host(PROXY_HOST).port(proxyPort).build(); successVerify(proxy, clientHttp2); } @Test void testSimpleProxy1() { - Proxy proxy = Proxy.builder().type(ProxyType.HTTP).host(PROXY_HOST).port(proxyPort).build(); + Proxy proxy = Proxy.builder().forceHttpConnect(true).type(ProxyType.HTTP).host(PROXY_HOST).port(proxyPort).build(); successVerify(proxy, clientHttp1); } @Test void testSimpleProxy2() { - Proxy proxy = Proxy.builder().type(ProxyType.HTTP).host(PROXY_HOST).port(proxyPort).build(); + Proxy proxy = Proxy.builder().forceHttpConnect(true).type(ProxyType.HTTP).host(PROXY_HOST).port(proxyPort).build(); successVerify(proxy, clientHttp2); } @@ -197,11 +201,18 @@ private void noHosts(HttpClient client) { } private void successVerify(Proxy proxy, HttpClient client) { - try (HttpClientResponse response = client.get("/get").proxy(proxy).request()) { + ClientRequest request = client.get("/get").proxy(proxy); + try (HttpClientResponse response = request.request()) { assertThat(response.status(), is(Status.OK_200)); String entity = response.entity().as(String.class); assertThat(entity, is("Hello")); } + boolean proxyConnection = request.headers().contains(ClientRequestBase.PROXY_CONNECTION); + if (client == clientHttp1) { + assertTrue(proxyConnection, "HTTP1 requires Proxy-Connection header"); + } else { + assertFalse(proxyConnection, "HTTP2 does not allow Proxy-Connection header"); + } assertThat(httpProxy.counter(), is(1)); } diff --git a/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpsProxyTest.java b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpsProxyTest.java index ad29a424f49..11e8e10c172 100644 --- a/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpsProxyTest.java +++ b/webclient/tests/http1/src/test/java/io/helidon/webclient/tests/HttpsProxyTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 Oracle and/or its affiliates. + * Copyright (c) 2023, 2024 Oracle and/or its affiliates. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,8 @@ import io.helidon.common.pki.Keys; import io.helidon.common.tls.Tls; import io.helidon.http.Status; +import io.helidon.webclient.api.ClientRequest; +import io.helidon.webclient.api.ClientRequestBase; import io.helidon.webclient.api.HttpClient; import io.helidon.webclient.api.HttpClientResponse; import io.helidon.webclient.api.Proxy; @@ -44,6 +46,8 @@ import static io.helidon.http.Method.GET; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; @ServerTest class HttpsProxyTest { @@ -182,11 +186,18 @@ void testSystemProxy2() { } private void successVerify(Proxy proxy, HttpClient client) { - try (HttpClientResponse response = client.get("/get").proxy(proxy).request()) { + ClientRequest request = client.get("/get").proxy(proxy); + try (HttpClientResponse response = request.request()) { assertThat(response.status(), is(Status.OK_200)); String entity = response.entity().as(String.class); assertThat(entity, is("Hello")); } + boolean proxyConnection = request.headers().contains(ClientRequestBase.PROXY_CONNECTION); + if (client == clientHttp1) { + assertTrue(proxyConnection, "HTTP1 requires Proxy-Connection header"); + } else { + assertFalse(proxyConnection, "HTTP2 does not allow Proxy-Connection header"); + } } private void noProxyChecks(HttpClient client) {