Skip to content

Commit

Permalink
4.x: Helidon Webclient (4.0.10) is not routing the requests through p…
Browse files Browse the repository at this point in the history
…roxy configured using Proxy Builder. helidon-io#9022 (helidon-io#9023)

4.x: Helidon Webclient (4.0.10) is not routing the requests through proxy configured using Proxy Builder. helidon-io#9022

Signed-off-by: Jorge Bescos Gascon <jorge.bescos.gascon@oracle.com>
  • Loading branch information
jbescos authored Jul 31, 2024
1 parent b17941d commit 45f46c8
Show file tree
Hide file tree
Showing 8 changed files with 204 additions and 38 deletions.
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
<version.lib.junit-platform>1.9.3</version.lib.junit-platform>
<version.lib.kafka-junit5>3.2.3</version.lib.kafka-junit5>
<version.lib.mockito>2.23.4</version.lib.mockito>
<version.lib.mockserver>5.15.0</version.lib.mockserver>
<version.lib.restito>0.9.1</version.lib.restito>
<version.lib.rxjava2-jdk9-interop>0.1.0</version.lib.rxjava2-jdk9-interop>
<version.lib.rxjava>2.2.10</version.lib.rxjava>
Expand Down Expand Up @@ -1035,6 +1036,11 @@
<artifactId>mockito-core</artifactId>
<version>${version.lib.mockito}</version>
</dependency>
<dependency>
<groupId>org.mock-server</groupId>
<artifactId>mockserver-netty-no-dependencies</artifactId>
<version>${version.lib.mockserver}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jgit</groupId>
<artifactId>org.eclipse.jgit.junit</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ public abstract class ClientRequestBase<T extends ClientRequest<T>, 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<String, AtomicLong> COUNTERS = new ConcurrentHashMap<>();
private static final Set<String> SUPPORTED_SCHEMES = Set.of("https", "http");

Expand Down Expand Up @@ -253,7 +257,7 @@ public T proxy(Proxy proxy) {

@Override
public R request() {
headers.setIfAbsent(USER_AGENT_HEADER);
additionalHeaders();
return validateAndSubmit(BufferData.EMPTY_BYTES);
}

Expand All @@ -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.
*
Expand Down
74 changes: 50 additions & 24 deletions webclient/api/src/main/java/io/helidon/webclient/api/Proxy.java
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -90,6 +88,7 @@ public class Proxy {
private final Optional<char[]> password;
private final ProxySelector systemProxySelector;
private final Optional<Header> proxyAuthHeader;
private final boolean forceHttpConnect;

private Proxy(Proxy.Builder builder) {
this.host = builder.host();
Expand All @@ -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;
Expand Down Expand Up @@ -451,7 +451,8 @@ private static Optional<Boolean> 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,
Expand All @@ -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();
}

Expand Down Expand Up @@ -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));
}
};
Expand All @@ -587,6 +589,7 @@ public static class Builder implements io.helidon.common.Builder<Builder, Proxy>
private int port = 80;
private String username;
private char[] password;
private boolean forceHttpConnect = false;

private Builder() {
}
Expand Down Expand Up @@ -632,6 +635,11 @@ public Proxy build() {
* <td>Proxy password</td>
* </tr>
* <tr>
* <td>httpConnect</td>
* <td>{@code false}</td>
* <td>Specify whether the HTTP client will always execute HTTP CONNECT.</td>
* </tr>
* <tr>
* <td>no-proxy</td>
* <td>{@code no default}</td>
* <td>Contains list of the hosts which should be excluded from using proxy</td>
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -742,6 +764,10 @@ ProxyType type() {
return type;
}

boolean forceHttpConnect() {
return forceHttpConnect;
}

String host() {
return host;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 5 additions & 0 deletions webclient/tests/http1/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,10 @@
<artifactId>vertx-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mock-server</groupId>
<artifactId>mockserver-netty-no-dependencies</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
@@ -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";
}
}
}
Loading

0 comments on commit 45f46c8

Please sign in to comment.