diff --git a/changes.md b/changes.md index 3f7022419d..dd954dc6c5 100644 --- a/changes.md +++ b/changes.md @@ -173,6 +173,9 @@ Changes log of the tunnel, instead of sending them to the origin server inside the tunnel. A refusal of the proxy to open the tunnel with a challenge is returned as a 407 response. Issue #1569. + - The HTTP client uses the "https.proxyHost" and "https.proxyPort" system properties for + HTTPS requests, instead of "http.proxyHost" and "http.proxyPort", like the JDK. The + "proxyHost" and "proxyPort" parameters still apply to both. Issue #1570. - The `Max-Age` and `Path` attributes of a cookie setting are no longer quoted. Issue #1334. - `Restlet.handle()` no longer fails with a `NullPointerException` when the Restlet has no context and cannot be started. - `CookieAuthenticator` no longer lets the next restlet overwrite the redirection sent after diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java index 8b98638774..5cc3fabbd4 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientCall.java @@ -239,7 +239,12 @@ String getTunnelProxyIdentity() { /** Indicates whether the exchange goes through a CONNECT tunnel of the HTTP proxy. */ private boolean isTunneled() { - return secure && (helper.getProxyHost() != null); + return secure && (helper.getProxy(true) != null); + } + + /** Indicates whether the exchange is sent to an HTTP forward proxy. */ + private boolean isForwardProxied() { + return !secure && (helper.getProxy(false) != null); } /** @@ -325,6 +330,9 @@ void handle(org.restlet.Request request, org.restlet.Response response) { HeaderUtils.addRequestHeaders(request, requestHeaders); if (isTunneled()) { moveTunnelProxyAuthorization(request); + } else if (!isForwardProxied()) { + // Direct connection: the proxy credentials must not reach the origin server. + requestHeaders.removeAll(HeaderConstants.HEADER_PROXY_AUTHORIZATION, true); } Status status = sendRequest(request); diff --git a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java index b72c58fc19..bb64bfde51 100644 --- a/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java +++ b/org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java @@ -168,13 +168,15 @@ * * proxyHost * String - * System property "http.proxyHost" - * The host name of the HTTP proxy. + * System property "http.proxyHost" for HTTP requests, "https.proxyHost" for HTTPS + * requests + * The host name of the HTTP proxy, used for both HTTP and HTTPS requests when set. * * * proxyPort * int - * System property "http.proxyPort" or "3128" + * System property "http.proxyPort" for HTTP requests, "https.proxyPort" for HTTPS requests, + * or "3128" * The port of the HTTP proxy. * * @@ -314,7 +316,7 @@ NettyClientCall create(Request request) { } boolean secure = "https".equalsIgnoreCase(scheme); - boolean forwardProxy = !secure && (getProxyHost() != null); + boolean forwardProxy = !secure && (getProxy(false) != null); String target = ReferenceUtils.format(request.getResourceRef(), forwardProxy, request); return new NettyClientCall(this, request.getMethod().toString(), ref, target, secure); } @@ -469,7 +471,7 @@ private static void resetResponse(Response response, Request next) { Channel acquire(NettyClientCall call) throws IOException { if (call.isConfidential() && getTransportMode() == TransportMode.HTTP3 - && getProxyHost() == null) { + && getProxy(true) == null) { return acquireHttp3(call); } @@ -664,9 +666,11 @@ private Channel connect(final OriginPool pool, final String proxyAuthorization) boolean acquired = true; try { final SslContext ssl = pool.secure ? getSslContext() : null; - final String proxyHost = getProxyHost(); + final InetSocketAddress proxyRef = getProxy(pool.secure); final InetSocketAddress proxy = - (proxyHost == null) ? null : new InetSocketAddress(proxyHost, getProxyPort()); + (proxyRef == null) + ? null + : new InetSocketAddress(proxyRef.getHostString(), proxyRef.getPort()); Bootstrap bootstrap = new Bootstrap() @@ -1123,7 +1127,46 @@ public int getMaxResponseHeaderSize() { } /** - * Returns the host name of the HTTP proxy, if specified. + * Returns the HTTP proxy to send a request to, or null to connect directly to the origin + * server. + * + * @param secure True for an HTTPS request, false for an HTTP request. + * @return The unresolved address of the HTTP proxy, or null. + */ + InetSocketAddress getProxy(boolean secure) { + String host = secure ? getHttpsProxyHost() : getProxyHost(); + return (host == null) + ? null + : InetSocketAddress.createUnresolved( + host, secure ? getHttpsProxyPort() : getProxyPort()); + } + + /** + * Returns the host name of the HTTP proxy for HTTPS requests, if specified: the "proxyHost" + * parameter, or the "https.proxyHost" system property. + * + * @return the host name of the HTTP proxy for HTTPS requests, if specified. + */ + public String getHttpsProxyHost() { + return getHelpedParameters() + .getFirstValue("proxyHost", System.getProperty("https.proxyHost")); + } + + /** + * Returns the port of the HTTP proxy for HTTPS requests: the "proxyPort" parameter, or the + * "https.proxyPort" system property, 3128 otherwise. + * + * @return the port of the HTTP proxy for HTTPS requests. + */ + public int getHttpsProxyPort() { + return Integer.parseInt( + getHelpedParameters() + .getFirstValue("proxyPort", System.getProperty("https.proxyPort", "3128"))); + } + + /** + * Returns the host name of the HTTP proxy for HTTP requests, if specified: the "proxyHost" + * parameter, or the "http.proxyHost" system property. * * @return the host name of the HTTP proxy, if specified. */ @@ -1133,7 +1176,8 @@ public String getProxyHost() { } /** - * Returns the port of the HTTP proxy, if specified, 3128 otherwise. + * Returns the port of the HTTP proxy for HTTP requests: the "proxyPort" parameter, or the + * "http.proxyPort" system property, 3128 otherwise. * * @return the port of the HTTP proxy. */ diff --git a/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java b/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java index 8ba8ddc87e..819e820708 100644 --- a/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java +++ b/org.restlet/src/test/java/org/restlet/engine/connector/ProxyTestCase.java @@ -24,7 +24,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Base64; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -255,6 +257,65 @@ private static ChallengeResponse rawCredentials(String token) { return result; } + @Test + void usesHttpsProxySystemPropertiesForHttpsRequests() throws Exception { + startHttpsServer(HELLO); + withSystemProperties( + Map.of("https.proxyHost", "localhost", "https.proxyPort", proxyPort()), + () -> { + client = newHttpsClient(false); + Response response = + client.handle( + new Request( + Method.GET, + "https://localhost:" + server.getActualPort() + "/s")); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + }); + + assertEquals( + List.of("CONNECT localhost:" + server.getActualPort() + " HTTP/1.1"), + proxy.requestLines); + } + + @Test + void ignoresHttpProxySystemPropertiesForHttpsRequests() throws Exception { + startHttpsServer(HELLO); + withSystemProperties( + Map.of("http.proxyHost", "localhost", "http.proxyPort", proxyPort()), + () -> { + client = newHttpsClient(false); + Response response = + client.handle( + new Request( + Method.GET, + "https://localhost:" + server.getActualPort() + "/s")); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + }); + + assertTrue(proxy.requestLines.isEmpty(), proxy.requestLines.toString()); + } + + @Test + void doesNotSendProxyCredentialsToOriginServerWithoutProxy() throws Exception { + startHttpsServer(PROXY_AUTHORIZATION_ECHO); + withSystemProperties( + Map.of("http.proxyHost", "localhost", "http.proxyPort", proxyPort()), + () -> { + client = newHttpsClient(false); + Request request = + new Request( + Method.GET, + "https://localhost:" + server.getActualPort() + "/s"); + request.setProxyChallengeResponse( + new ChallengeResponse(ChallengeScheme.HTTP_BASIC, "scott", "tiger")); + Response response = client.handle(request); + assertEquals(Status.SUCCESS_OK, response.getStatus()); + assertEquals("none", response.getEntityAsText()); + }); + + assertTrue(proxy.requestLines.isEmpty(), proxy.requestLines.toString()); + } + @Test void reportsUnreachableProxy() throws Exception { server = new Server(new Context(), Protocol.HTTP, 0, HELLO); @@ -297,13 +358,19 @@ private void startHttpsServer(Restlet next) throws Exception { } private Client newHttpsClient(String... parameters) throws Exception { + return newHttpsClient(true, parameters); + } + + private Client newHttpsClient(boolean proxyParameters, String... parameters) throws Exception { Path keystore = keystore(); Context context = new Context(); for (int i = 0; i < parameters.length; i += 2) { context.getParameters().add(parameters[i], parameters[i + 1]); } - context.getParameters().add("proxyHost", "localhost"); - context.getParameters().add("proxyPort", Integer.toString(proxy.port())); + if (proxyParameters) { + context.getParameters().add("proxyHost", "localhost"); + context.getParameters().add("proxyPort", proxyPort()); + } context.getParameters().add("truststorePath", keystore.toString()); context.getParameters().add("truststorePassword", "testtest"); context.getParameters().add("trustStoreType", "PKCS12"); @@ -312,6 +379,36 @@ private Client newHttpsClient(String... parameters) throws Exception { return result; } + private String proxyPort() { + return Integer.toString(proxy.port()); + } + + /** Runs an action with system properties set, then restores their previous values. */ + private static void withSystemProperties(Map properties, ThrowingAction action) + throws Exception { + Map previous = new HashMap<>(); + for (Map.Entry property : properties.entrySet()) { + previous.put( + property.getKey(), System.setProperty(property.getKey(), property.getValue())); + } + try { + action.run(); + } finally { + for (Map.Entry property : previous.entrySet()) { + if (property.getValue() == null) { + System.clearProperty(property.getKey()); + } else { + System.setProperty(property.getKey(), property.getValue()); + } + } + } + } + + @FunctionalInterface + private interface ThrowingAction { + void run() throws Exception; + } + /** * Minimal HTTP proxy: forwards absolute-form requests (one connection to the origin per client * connection, bytes relayed both ways) and opens CONNECT tunnels.