16612 download large files (#16627)

* #16612 support downloading large files from Grid

I added a new Grid endpoint "/se/files/:name" which allows downloading the file directly, without encoding it to Base64 and adding to Json. This transformation kills the performance and causes OutOfMemory errors for large files (e.g. 256+ MB).

NB! Be sure that `toString()` method of objects (HttpRequest, HttpResponse, Contents.Supplier) never returns too long string - it spam debug logs and can cause OOM during debugging.

* #16612 extract anonymous implementations of `Contents.Supplier` to separate classes

It makes debugging easier. You can easily see what instances they are and where they come from.

* #16612 optimize method `RemoteWebDriver.downloadFile()`

Instead of reading the whole file to a byte array, just save given InputStream directly to the file.

Now it can download large files (I tried 4GB) while consuming very low memory.

* #16612 just in case, return `Contents.fromStream` only when downloading files.

For json responses, still return `Contents.bytes` which allows re-reading its content multiple times. Just in case.

* #16612 fix flaky test: wait until the downloads folder gets deleted

After stopping a Grid node, the folder is deleted asynchronously (by cache removal listener). So we need to wait for it in test.

* #16612 fix flaky test: wait until the grid node is fully stopped

At least on my machine, stopping the node takes some time, and any checks right after `node.stop(sessionId)` often can fail.

* #16612 fix flaky test LocalNewSessionQueueTest

Gr... This is extremely hard to debug test.

After hours of debugging, I came to a conclusion that we just need to increase the timeout. On my machine, `latch` gets decreased after ~1.2 seconds. So 1 second was not enough.

* #16612 fix flaky test JdkHttpClientTest

I don't know why, but sometimes we receive `HttpTimeoutException` instead of `InterruptedException`. Seems reasonable to consider execution as interrupted in both cases. (?)

* #16612 remove unneeded code

None of `is.readNBytes` implementations returns -1. It always returns 0 or positive number.

* #16612 upload logs in case of test failure

* ignore few more IDEA files

* #16612 slightly improve logging in `W3CHttpResponseCodec.decode`

Don't log the entire response body - just content type and length.

Author: asolntsev

.github/workflows/bazel.yml

@@ -185,6 +185,14 @@ jobs:
         if: ${{ always() && inputs.artifact-name != 'ignore-artifacts' }}
         run: |
           git diff > changes.patch
+      - name: "Upload test logs"
+        if: failure()
+        uses: actions/upload-artifact@v5
+        with:
+          name: test-logs-${{ inputs.os }}-${{ inputs.name }}-${{ inputs.browser }}
+          retention-days: 7
+          path: |
+            **/*.log
       - name: "Upload changes"
         if: ${{ always() && inputs.artifact-name != 'ignore-artifacts' }}
         uses: actions/upload-artifact@v4

.gitignore

@@ -26,10 +26,14 @@ javascript/selenium-webdriver/lib/atoms/is-displayed.js
 javascript/selenium-webdriver/lib/atoms/mutation-listener.js
 javascript/safari-driver/node_modules/
 javascript/webdriver/devtools/types/
+.idea/bazelVersionCache.xml
+.idea/GitLink.xml
 .idea/vcs.xml
 .idea/workspace.xml
 .idea/dictionaries/
+.idea/runConfigurations.xml
 .idea/scopes/
+.idea/shelf
 .idea/sonarlint/
 .idea/qaplug_profiles.xml
 out

java/src/org/openqa/selenium/grid/data/NodeStatus.java

@@ -248,4 +248,10 @@ public Map<String, Object> toJson() {
 
     return unmodifiableMap(toReturn);
   }
+
+  @Override
+  public String toString() {
+    return String.format(
+        "NodeStatus(availability: %s, slots:%s, uri:%s)", availability, slots.size(), externalUri);
+  }
 }

java/src/org/openqa/selenium/grid/data/Session.java

@@ -144,4 +144,9 @@ public boolean equals(Object that) {
   public int hashCode() {
     return Objects.hash(id, uri);
   }
+
+  @Override
+  public String toString() {
+    return String.format("Session(id:%s, url:%s, started at: %s)", id, uri, getStartTime());
+  }
 }

java/src/org/openqa/selenium/grid/node/BUILD.bazel

@@ -22,5 +22,6 @@ java_library(
         "//java/src/org/openqa/selenium/remote",
         "//java/src/org/openqa/selenium/status",
         artifact("com.google.guava:guava"),
+        artifact("org.jspecify:jspecify"),
     ],
 )

java/src/org/openqa/selenium/grid/node/Node.java

@@ -171,6 +171,9 @@ protected Node(
             get("/session/{sessionId}/se/files")
                 .to(params -> new DownloadFile(this, sessionIdFrom(params)))
                 .with(spanDecorator("node.download_file")),
+            get("/session/{sessionId}/se/files/{fileName}")
+                .to(params -> new DownloadFile(this, sessionIdFrom(params)))
+                .with(spanDecorator("node.download_file")),
             post("/session/{sessionId}/se/files")
                 .to(params -> new DownloadFile(this, sessionIdFrom(params)))
                 .with(spanDecorator("node.download_file")),

java/src/org/openqa/selenium/grid/node/local/LocalNode.java

@@ -19,12 +19,18 @@
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static java.nio.file.Files.readAttributes;
+import static java.time.ZoneOffset.UTC;
+import static java.time.format.DateTimeFormatter.RFC_1123_DATE_TIME;
+import static java.util.Arrays.asList;
+import static java.util.Locale.US;
+import static java.util.Objects.requireNonNullElseGet;
 import static org.openqa.selenium.HasDownloads.DownloadedFile;
 import static org.openqa.selenium.concurrent.ExecutorServices.shutdownGracefully;
 import static org.openqa.selenium.grid.data.Availability.DOWN;
 import static org.openqa.selenium.grid.data.Availability.DRAINING;
 import static org.openqa.selenium.grid.data.Availability.UP;
 import static org.openqa.selenium.grid.node.CapabilityResponseEncoder.getEncoder;
+import static org.openqa.selenium.net.Urls.urlDecode;
 import static org.openqa.selenium.remote.CapabilityType.ENABLE_DOWNLOADS;
 import static org.openqa.selenium.remote.HttpSessionId.getSessionId;
 import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES;
@@ -39,6 +45,7 @@
 import com.github.benmanes.caffeine.cache.Ticker;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
+import com.google.common.net.MediaType;
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
@@ -47,9 +54,11 @@
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.FileTime;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
+import java.time.format.DateTimeFormatter;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
@@ -107,6 +116,7 @@
 import org.openqa.selenium.json.Json;
 import org.openqa.selenium.remote.Browser;
 import org.openqa.selenium.remote.SessionId;
+import org.openqa.selenium.remote.http.Contents;
 import org.openqa.selenium.remote.http.HttpMethod;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
@@ -123,6 +133,7 @@ public class LocalNode extends Node implements Closeable {
 
   private static final Json JSON = new Json();
   private static final Logger LOG = Logger.getLogger(LocalNode.class.getName());
+  private static final DateTimeFormatter HTTP_DATE_FORMAT = RFC_1123_DATE_TIME.withLocale(US);
 
   private final EventBus bus;
   private final URI externalUri;
@@ -742,9 +753,12 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
         Optional.ofNullable(tempFS.getBaseDir().listFiles()).orElse(new File[] {})[0];
 
     try {
-      if (req.getMethod().equals(HttpMethod.GET)) {
+      if (req.getMethod().equals(HttpMethod.GET) && req.getUri().endsWith("/se/files")) {
         return listDownloadedFiles(downloadsDirectory);
       }
+      if (req.getMethod().equals(HttpMethod.GET)) {
+        return getDownloadedFile(downloadsDirectory, extractFileName(req));
+      }
       if (req.getMethod().equals(HttpMethod.DELETE)) {
         return deleteDownloadedFile(downloadsDirectory);
       }
@@ -754,6 +768,19 @@ public HttpResponse downloadFile(HttpRequest req, SessionId id) {
     }
   }
 
+  private String extractFileName(HttpRequest req) {
+    return extractFileName(req.getUri());
+  }
+
+  String extractFileName(String uri) {
+    String prefix = "/se/files/";
+    int index = uri.lastIndexOf(prefix);
+    if (index < 0) {
+      throw new IllegalArgumentException("Unexpected URL for downloading a file: " + uri);
+    }
+    return urlDecode(uri.substring(index + prefix.length())).replace(' ', '+');
+  }
+
   /** User wants to list files that can be downloaded */
   private HttpResponse listDownloadedFiles(File downloadsDirectory) {
     File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {});
@@ -798,29 +825,63 @@ private HttpResponse getDownloadedFile(HttpRequest req, File downloadsDirectory)
                     new WebDriverException(
                         "Please specify file to download in payload as {\"name\":"
                             + " \"fileToDownload\"}"));
-    File[] allFiles =
-        Optional.ofNullable(downloadsDirectory.listFiles((dir, name) -> name.equals(filename)))
-            .orElse(new File[] {});
-    if (allFiles.length == 0) {
-      throw new WebDriverException(
-          String.format(
-              "Cannot find file [%s] in directory %s.",
-              filename, downloadsDirectory.getAbsolutePath()));
-    }
-    if (allFiles.length != 1) {
-      throw new WebDriverException(
-          String.format("Expected there to be only 1 file. There were: %s.", allFiles.length));
-    }
-    String content = Zip.zip(allFiles[0]);
+    File file = findDownloadedFile(downloadsDirectory, filename);
+    String content = Zip.zip(file);
     Map<String, Object> data =
         Map.of(
             "filename", filename,
-            "file", getFileInfo(allFiles[0]),
+            "file", getFileInfo(file),
             "contents", content);
     Map<String, Map<String, Object>> result = Map.of("value", data);
     return new HttpResponse().setContent(asJson(result));
   }
 
+  private HttpResponse getDownloadedFile(File downloadsDirectory, String fileName)
+      throws IOException {
+    if (fileName.isEmpty()) {
+      throw new WebDriverException("Please specify file to download in URL");
+    }
+    File file = findDownloadedFile(downloadsDirectory, fileName);
+    BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class);
+    return new HttpResponse()
+        .setHeader("Content-Type", MediaType.OCTET_STREAM.toString())
+        .setHeader("Content-Length", String.valueOf(attributes.size()))
+        .setHeader("Last-Modified", lastModifiedHeader(attributes.lastModifiedTime()))
+        .setContent(Contents.file(file));
+  }
+
+  private String lastModifiedHeader(FileTime fileTime) {
+    return HTTP_DATE_FORMAT.format(fileTime.toInstant().atZone(UTC));
+  }
+
+  private File findDownloadedFile(File downloadsDirectory, String filename)
+      throws WebDriverException {
+    List<File> matchingFiles =
+        asList(
+            requireNonNullElseGet(
+                downloadsDirectory.listFiles((dir, name) -> name.equals(filename)),
+                () -> new File[0]));
+    if (matchingFiles.isEmpty()) {
+      List<File> files = downloadedFiles(downloadsDirectory);
+      throw new WebDriverException(
+          String.format(
+              "Cannot find file [%s] in directory %s. Found %s files: %s.",
+              filename, downloadsDirectory.getAbsolutePath(), files.size(), files));
+    }
+    if (matchingFiles.size() != 1) {
+      throw new WebDriverException(
+          String.format(
+              "Expected there to be only 1 file. Found %s files: %s.",
+              matchingFiles.size(), matchingFiles));
+    }
+    return matchingFiles.get(0);
+  }
+
+  private static List<File> downloadedFiles(File downloadsDirectory) {
+    File[] files = requireNonNullElseGet(downloadsDirectory.listFiles(), () -> new File[0]);
+    return asList(files);
+  }
+
   private HttpResponse deleteDownloadedFile(File downloadsDirectory) {
     File[] files = Optional.ofNullable(downloadsDirectory.listFiles()).orElse(new File[] {});
     for (File file : files) {

java/src/org/openqa/selenium/net/Urls.java

@@ -17,14 +17,16 @@
 
 package org.openqa.selenium.net;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.net.URLDecoder;
 import java.net.URLEncoder;
-import java.nio.charset.StandardCharsets;
 import org.openqa.selenium.internal.Require;
 
 public class Urls {
@@ -41,7 +43,11 @@ private Urls() {
    * @see URLEncoder#encode(java.lang.String, java.lang.String)
    */
   public static String urlEncode(String value) {
-    return URLEncoder.encode(value, StandardCharsets.UTF_8);
+    return URLEncoder.encode(value, UTF_8);
+  }
+
+  public static String urlDecode(String encodedValue) {
+    return URLDecoder.decode(encodedValue, UTF_8);
   }
 
   public static URL fromUri(URI uri) {

java/src/org/openqa/selenium/netty/server/FileBackedOutputStreamContentSupplier.java

@@ -0,0 +1,74 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you 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 org.openqa.selenium.netty.server;
+
+import com.google.common.io.FileBackedOutputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.nio.charset.Charset;
+import org.openqa.selenium.remote.http.Contents;
+
+class FileBackedOutputStreamContentSupplier implements Contents.Supplier {
+  private final String description;
+  private final FileBackedOutputStream buffer;
+  private final long length;
+
+  FileBackedOutputStreamContentSupplier(
+      String description, FileBackedOutputStream buffer, long length) {
+    this.description = description;
+    this.buffer = buffer;
+    this.length = length;
+  }
+
+  @Override
+  public InputStream get() {
+    try {
+      return buffer.asByteSource().openBufferedStream();
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
+    }
+  }
+
+  @Override
+  public long length() {
+    return length;
+  }
+
+  @Override
+  public void close() throws IOException {
+    buffer.reset();
+  }
+
+  @Override
+  public String toString() {
+    return String.format("Content for %s (%s bytes)", description, length);
+  }
+
+  @Override
+  public String contentAsString(Charset charset) {
+    ByteArrayOutputStream out = new ByteArrayOutputStream();
+    try {
+      buffer.asByteSource().copyTo(out);
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+    return out.toString(charset);
+  }
+}