Skip to content

Commit

Permalink
6962 include proxy setting for relative uris 4.x (helidon-io#7425)
Browse files Browse the repository at this point in the history 
* Include Proxy setting as part of deciding relativeURIs logic

Description:
This is a feature parity change with prior versions related to relativeUris config logic. When proxy is set or host is in no-proxy list, the request uses absolute URI because of Section 5.1.2 Request-URI spec in https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html which states: "The absoluteURI form is REQUIRED when the request is being made to a proxy.". If absoluteURI does not work as the request URI, then relativeUris config can be set to true to override this behavior.

Documentation impact:
When relativeUris is documented as a webclient config, we can add the notes mentioned in the Description above.

Additional notes:
This change can be compared to this code section in 3.x that performs the same logic: https://github.com/helidon-io/helidon/blob/helidon-3.x/webclient/webclient/src/main/java/io/helidon/webclient/WebClientRequestBuilderImpl.java#L756-L763
  • Loading branch information
@klustria @dalexandrov
klustria authored and dalexandrov committed Aug 26, 2023
1 parent e50c7ba commit a792580
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 30 deletions.
3 changes: 1 addition & 2 deletions webclient/api/src/main/java/io/helidon/webclient/api/HttpClientConfigBlueprint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ default ClientRequestHeaders defaultRequestHeaders() {
*
* @return relative URIs flag
*/
// TODO Set the default value to false when proxy is implemented and see Http1CallChainBase.prologue for other changes
@ConfiguredOption("true")
@ConfiguredOption("false")
boolean relativeUris();

/**
Expand Down
17 changes: 16 additions & 1 deletion webclient/api/src/main/java/io/helidon/webclient/api/Proxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,25 @@ public ProxyType type() {
* @param uri the uri
* @return true if it is in no hosts, otherwise false
*/
private boolean isNoHosts(InetSocketAddress uri) {
public boolean isNoHosts(InetSocketAddress uri) {
return noProxy.apply(uri);
}

/**
* Verifies whether the specified Uri is using system proxy.
*
* @param uri the uri
* @return true if the uri resource will be proxied
*/
public boolean isUsingSystemProxy(String uri) {
if (systemProxySelector != null) {
List<java.net.Proxy> proxies = systemProxySelector
.select(URI.create(uri));
return !proxies.isEmpty() && !proxies.get(0).equals(java.net.Proxy.NO_PROXY);
}
return false;
}

/**
* Creates an Optional with the InetSocketAddress of the server proxy for the specified uri.
*
Expand Down
15 changes: 13 additions & 2 deletions webclient/http1/src/main/java/io/helidon/webclient/http1/Http1CallChainBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.InputStream;
import java.io.UncheckedIOException;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.concurrent.CompletableFuture;
Expand Down Expand Up @@ -157,10 +158,20 @@ void prologue(BufferData nonEntityData, WebClientServiceRequest request, ClientU
+ request.headers().get(Http.HeaderNames.HOST).value()
+ " HTTP/1.1\r\n");
} else {
String schemeHostPort = clientConfig.relativeUris() ? "" : uri.scheme() + "://" + uri.host() + ":" + uri.port();
// When proxy is set, ensure that the request uses absolute URI because of Section 5.1.2 Request-URI in
// https://www.w3.org/Protocols/rfc2616/rfc2616-sec5.html which states: "The absoluteURI form is REQUIRED when the
// request is being made to a proxy."
String absoluteUri = uri.scheme() + "://" + uri.host() + ":" + uri.port();
InetSocketAddress uriAddress = new InetSocketAddress(uri.host(), uri.port());
String requestUri = proxy == Proxy.noProxy()
|| (proxy.type() == Proxy.ProxyType.HTTP && proxy.isNoHosts(uriAddress))
|| (proxy.type() == Proxy.ProxyType.SYSTEM && !proxy.isUsingSystemProxy(absoluteUri))
|| clientConfig.relativeUris()
? "" // don't set host details, so it becomes relative URI
: absoluteUri;
nonEntityData.writeAscii(request.method().text()
+ " "
+ schemeHostPort
+ requestUri
+ uri.pathWithQueryAndFragment()
+ " HTTP/1.1\r\n");
}
Expand Down
153 changes: 128 additions & 25 deletions webclient/http1/src/test/java/io/helidon/webclient/http1/Http1ClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import io.helidon.logging.common.LogConfig;
import io.helidon.webclient.api.ClientConnection;
import io.helidon.webclient.api.HttpClientResponse;
import io.helidon.webclient.api.Proxy;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -78,6 +79,17 @@ class Http1ClientTest {
.sendExpectContinue(false)
.build();
private static final int dummyPort = 1234;
private static final String TARGET_HOST = "www.oracle.com";
private static final String TARGET_URI_PATH = "/test";
public static final String PROXY_HOST = "http://www-proxy-hqdc.us.oracle.com";
public static final String PROXY_PORT = "80";

private enum RelativeUrisValue {
TRUE, FALSE, DEFAULT
}
private enum ProxyConfiguration {
UNSET, NO_PROXY, HTTP, HTTP_SET_NO_PROXY_HOST, SYSTEM_UNSET, SYSTEM_SET_PROXY, SYSTEM_SET_PROXY_AND_NON_PROXY_HOST
}

@Test
void testMaxHeaderSizeFail() {
Expand Down Expand Up @@ -218,24 +230,59 @@ void testSkipUrlEncoding() {

@ParameterizedTest
@MethodSource("relativeUris")
void testRelativeUris(boolean relativeUris, boolean outputStream, String requestUri, String expectedUriStart) {
Http1Client client = Http1Client.builder().relativeUris(relativeUris).build();
void testRelativeUris(ProxyConfiguration proxyConfig, RelativeUrisValue relativeUris, boolean outputStream, String requestUri, String expectedUriStart) {
Proxy proxy = null;
switch (proxyConfig) {
case UNSET -> {} // proxy is already initialized to null which is the goal of this condition, so no-op
case NO_PROXY -> proxy = Proxy.noProxy();
case HTTP -> proxy = createHttpProxyBuilder().build();
case HTTP_SET_NO_PROXY_HOST -> proxy = createHttpProxyBuilder().addNoProxy(TARGET_HOST).build();
case SYSTEM_UNSET -> proxy = Proxy.create();
case SYSTEM_SET_PROXY -> {
proxy = Proxy.create();
System.setProperty("http.proxyHost", PROXY_HOST);
System.setProperty("http.proxyPort", PROXY_PORT);
}
case SYSTEM_SET_PROXY_AND_NON_PROXY_HOST -> {
proxy = Proxy.create();
System.setProperty("http.proxyHost", PROXY_HOST);
System.setProperty("http.proxyPort", PROXY_PORT);
System.setProperty("http.nonProxyHosts", "localhost|127.0.0.1|10.*.*.*|*.example.com|etc|" + TARGET_HOST);
}
}

Http1Client client;
switch (relativeUris) {
case TRUE -> client = Http1Client.builder().relativeUris(true).build();
case FALSE -> client = Http1Client.builder().relativeUris(false).build();
default -> client = Http1Client.create(); // Don't set relativeUris and accept whatever is the default
}
FakeHttp1ClientConnection connection = new FakeHttp1ClientConnection();
Http1ClientRequest request = client.put(requestUri);
Http1ClientRequest request = proxy != null ? client.put(requestUri).proxy(proxy) : client.put(requestUri);
request.connection(connection);
HttpClientResponse response;
if (outputStream) {
response = getHttp1ClientResponseFromOutputStream(request, new String[] {"Sending Something"});
} else {
response = request.submit("Sending Something");
}
HttpClientResponse response = outputStream
? getHttp1ClientResponseFromOutputStream(request, new String[] {"Sending Something"})
: request.submit("Sending Something");

assertThat(response.status(), is(Http.Status.OK_200));
StringTokenizer st = new StringTokenizer(connection.getPrologue(), " ");
// skip method part
st.nextToken();
// Validate URI part
assertThat(st.nextToken(), startsWith(expectedUriStart));

// Clear proxy system properties that were set
switch (proxyConfig) {
case SYSTEM_SET_PROXY -> {
System.clearProperty("http.proxyHost");
System.clearProperty("http.proxyPort");
}
case SYSTEM_SET_PROXY_AND_NON_PROXY_HOST -> {
System.clearProperty("http.proxyHost");
System.clearProperty("http.proxyPort");
System.clearProperty("http.nonProxyHosts");
}
}
}

@ParameterizedTest
Expand Down Expand Up @@ -392,26 +439,82 @@ private static Http1ClientResponse getHttp1ClientResponseFromOutputStream(Http1C
return response;
}

private static Proxy.Builder createHttpProxyBuilder() {
return Proxy.builder()
.type(Proxy.ProxyType.HTTP)
.host(PROXY_HOST)
.port(Integer.parseInt(PROXY_PORT));
}

private static Stream<Arguments> relativeUris() {
// Request type
boolean isOutputStream = true;
boolean isEntity = !isOutputStream;

return Stream.of(
// OutputStream (chunk request)
arguments(false, true, "http://www.dummy.com/test", "http://www.dummy.com:80/"),
arguments(false, true, "http://www.dummy.com:1111/test", "http://www.dummy.com:1111/"),
arguments(false, true, "https://www.dummy.com/test", "https://www.dummy.com:443/"),
arguments(false, true, "https://www.dummy.com:1111/test", "https://www.dummy.com:1111/"),
arguments(true, true, "http://www.dummy.com/test", "/test"),
arguments(true, true, "http://www.dummy.com:1111/test", "/test"),
arguments(true, true, "https://www.dummy.com/test", "/test"),
arguments(true, true, "https://www.dummy.com:1111/test", "/test"),
// Expects absolute URI
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.FALSE, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, "http://" + TARGET_HOST + ":80/"),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.FALSE, isOutputStream,
"https://" + TARGET_HOST + TARGET_URI_PATH, "https://" + TARGET_HOST + ":443/"),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.TRUE, isOutputStream,
"http://" + TARGET_HOST + ":1111/test", TARGET_URI_PATH),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.FALSE, isOutputStream,
"http://" + TARGET_HOST + ":1111/test", "http://" + TARGET_HOST + ":1111/"),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.DEFAULT, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, "http://" + TARGET_HOST + ":80/"),
arguments(ProxyConfiguration.SYSTEM_SET_PROXY, RelativeUrisValue.FALSE, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, "http://" + TARGET_HOST + ":80/"),
// Expects relative URI
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.TRUE, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.TRUE, isOutputStream,
"https://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.HTTP_SET_NO_PROXY_HOST, RelativeUrisValue.DEFAULT, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.NO_PROXY, RelativeUrisValue.DEFAULT, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.NO_PROXY, RelativeUrisValue.FALSE, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.NO_PROXY, RelativeUrisValue.DEFAULT, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.SYSTEM_UNSET, RelativeUrisValue.FALSE, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.SYSTEM_SET_PROXY_AND_NON_PROXY_HOST, RelativeUrisValue.FALSE, isOutputStream,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
// non-OutputStream (single entity request)
arguments(false, false, "http://www.dummy.com/test", "http://www.dummy.com:80/"),
arguments(false, false, "http://www.dummy.com:1111/test", "http://www.dummy.com:1111/"),
arguments(false, false, "https://www.dummy.com/test", "https://www.dummy.com:443/"),
arguments(false, false, "https://www.dummy.com:1111/test", "https://www.dummy.com:1111/"),
arguments(true, false, "http://www.dummy.com/test", "/test"),
arguments(true, false, "http://www.dummy.com:1111/test", "/test"),
arguments(true, false, "https://www.dummy.com/test", "/test"),
arguments(true, false, "https://www.dummy.com:1111/test", "/test"));
// Expects absolute URI
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.FALSE, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, "http://" + TARGET_HOST + ":80/"),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.FALSE, isEntity,
"https://" + TARGET_HOST + TARGET_URI_PATH, "https://" + TARGET_HOST + ":443/"),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.TRUE, isEntity,
"http://" + TARGET_HOST + ":1111/test", TARGET_URI_PATH),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.FALSE, isEntity,
"http://" + TARGET_HOST + ":1111/test", "http://" + TARGET_HOST + ":1111/"),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.DEFAULT, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, "http://" + TARGET_HOST + ":80/"),
arguments(ProxyConfiguration.SYSTEM_SET_PROXY, RelativeUrisValue.FALSE, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, "http://" + TARGET_HOST + ":80/"),
// Expects relative URI
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.TRUE, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.HTTP, RelativeUrisValue.TRUE, isEntity,
"https://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.HTTP_SET_NO_PROXY_HOST, RelativeUrisValue.DEFAULT, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.NO_PROXY, RelativeUrisValue.DEFAULT, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.NO_PROXY, RelativeUrisValue.FALSE, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.NO_PROXY, RelativeUrisValue.DEFAULT, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.SYSTEM_UNSET, RelativeUrisValue.FALSE, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH),
arguments(ProxyConfiguration.SYSTEM_SET_PROXY_AND_NON_PROXY_HOST, RelativeUrisValue.FALSE, isEntity,
"http://" + TARGET_HOST + TARGET_URI_PATH, TARGET_URI_PATH)
);
}

private static Stream<Arguments> headerValues() {
Expand Down

0 comments on commit a792580

Please sign in to comment.