fix: use the https.proxyHost and https.proxyPort properties for HTTPS

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

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);

org.restlet/src/main/java/org/restlet/engine/netty/NettyClientHelper.java

@@ -168,13 +168,15 @@
  * <tr>
  * <td>proxyHost</td>
  * <td>String</td>
- * <td>System property "http.proxyHost"</td>
- * <td>The host name of the HTTP proxy.</td>
+ * <td>System property "http.proxyHost" for HTTP requests, "https.proxyHost" for HTTPS
+ * requests</td>
+ * <td>The host name of the HTTP proxy, used for both HTTP and HTTPS requests when set.</td>
  * </tr>
  * <tr>
  * <td>proxyPort</td>
  * <td>int</td>
- * <td>System property "http.proxyPort" or "3128"</td>
+ * <td>System property "http.proxyPort" for HTTP requests, "https.proxyPort" for HTTPS requests,
+ * or "3128"</td>
  * <td>The port of the HTTP proxy.</td>
  * </tr>
  * <tr>
@@ -314,7 +316,7 @@
         }
 
         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 @@
     Channel acquire(NettyClientCall call) throws IOException {
         if (call.isConfidential()
                 && getTransportMode() == TransportMode.HTTP3
-                && getProxyHost() == null) {
+                && getProxy(true) == null) {
             return acquireHttp3(call);
         }
 
@@ -664,9 +666,11 @@
         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 @@
     }
 
     /**
-     * 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 @@
     }
 
     /**
-     * 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.
      */

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<String, String> properties, ThrowingAction action)
+            throws Exception {
+        Map<String, String> previous = new HashMap<>();
+        for (Map.Entry<String, String> property : properties.entrySet()) {
+            previous.put(
+                    property.getKey(), System.setProperty(property.getKey(), property.getValue()));
+        }
+        try {
+            action.run();
+        } finally {
+            for (Map.Entry<String, String> 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.