[java] Handle redirects inside the JdkHttpClient (#11816)

joerg1985 · diemol · web-flow · commit ca503605fe56 · 2023-04-04T12:53:47.000+02:00
Handle redirects inside the JdkHttpClient

Co-authored-by: Diego Molina &lt;diemol@users.noreply.github.com&gt;
diff --git a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java
@@ -39,6 +39,7 @@
 import java.io.UncheckedIOException;
 import java.net.Authenticator;
 import java.net.PasswordAuthentication;
+import java.net.ProtocolException;
 import java.net.Proxy;
 import java.net.ProxySelector;
 import java.net.SocketAddress;
@@ -62,8 +63,6 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static java.net.http.HttpClient.Redirect.ALWAYS;
-
 public class JdkHttpClient implements HttpClient {
   public static final Logger LOG = Logger.getLogger(JdkHttpClient.class.getName());
   private final JdkHttpMessages messages;
@@ -82,7 +81,7 @@ public class JdkHttpClient implements HttpClient {
 
     java.net.http.HttpClient.Builder builder = java.net.http.HttpClient.newBuilder()
       .connectTimeout(config.connectionTimeout())
-      .followRedirects(ALWAYS)
+      .followRedirects(java.net.http.HttpClient.Redirect.NEVER)
       .executor(executorService);
 
     Credentials credentials = config.credentials();
@@ -289,7 +288,42 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
 
     BodyHandler<byte[]> byteHandler = BodyHandlers.ofByteArray();
     try {
-      return messages.createResponse(client.send(messages.createRequest(req), byteHandler));
+      URI rawUri = messages.getRawUri(req);
+
+      // We need a custom handling of redirects to:
+      // - increase the maximum number of retries to 100
+      // - avoid a downgrade of POST requests, see the javadoc of j.n.h.HttpClient.Redirect
+      // - not run into https://bugs.openjdk.org/browse/JDK-8304701
+      for (int i = 0; i < 100; i++) {
+        java.net.http.HttpRequest request = messages.createRequest(req, rawUri);
+        java.net.http.HttpResponse<byte[]> response = client.send(request, byteHandler);
+
+        switch (response.statusCode()) {
+          case 301:
+          case 302:
+          case 303:
+          case 307:
+          case 308:
+            URI location = rawUri.resolve(response.headers()
+              .firstValue("location")
+              .orElseThrow(() -> new ProtocolException(
+                "HTTP " + response.statusCode() + " without 'location' header set"
+              )));
+
+            if ("https".equalsIgnoreCase(rawUri.getScheme()) && !"https".equalsIgnoreCase(location.getScheme()) ) {
+              throw new SecurityException("Downgrade from secure to insecure connection.");
+            } else if ("wss".equalsIgnoreCase(rawUri.getScheme()) && !"wss".equalsIgnoreCase(location.getScheme()) ) {
+              throw new SecurityException("Downgrade from secure to insecure connection.");
+            }
+
+            rawUri = location;
+            continue;
+          default:
+            return messages.createResponse(response);
+        }
+      }
+
+      throw new ProtocolException("Too many redirects: 101");
     } catch (HttpTimeoutException e) {
       throw new TimeoutException(e);
     } catch (IOException e) {
diff --git a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java
@@ -45,8 +45,8 @@ public JdkHttpMessages(ClientConfig config) {
     this.config = Objects.requireNonNull(config, "Client config");
   }
 
-  public java.net.http.HttpRequest createRequest(HttpRequest req) {
-    String rawUrl = getRawUrl(config.baseUri(), req.getUri());
+  public java.net.http.HttpRequest createRequest(HttpRequest req, URI rawUri) {
+    String rawUrl = rawUri.toString();
 
     // Add query string if necessary
     String queryString = StreamSupport.stream(req.getQueryParameterNames().spliterator(), false)
@@ -129,20 +129,18 @@ private BodyPublisher notChunkingBodyPublisher(HttpRequest req) {
     return BodyPublishers.fromPublisher(chunking, Long.parseLong(length));
   }
 
-  private String getRawUrl(URI baseUrl, String uri) {
+  public URI getRawUri(HttpRequest req) {
+    URI baseUrl = config.baseUri();
+    String uri = req.getUri();
     String rawUrl;
+
     if (uri.startsWith("ws://") || uri.startsWith("wss://") ||
-      uri.startsWith("http://") || uri.startsWith("https://")) {
+        uri.startsWith("http://") || uri.startsWith("https://")) {
       rawUrl = uri;
     } else {
       rawUrl = baseUrl.toString().replaceAll("/$", "") + uri;
     }
 
-    return rawUrl;
-  }
-
-  public URI getRawUri(HttpRequest req) {
-    String rawUrl = getRawUrl(config.baseUri(), req.getUri());
     return URI.create(rawUrl);
   }
 
