From 4915ed15351fe629dfaee5a174bbab3a6243b03b Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sat, 22 Nov 2025 11:43:52 +0200 Subject: [PATCH 1/5] #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. --- .../openqa/selenium/grid/data/Session.java | 5 ++ .../org/openqa/selenium/grid/node/BUILD.bazel | 1 + .../org/openqa/selenium/grid/node/Node.java | 3 + .../selenium/grid/node/local/LocalNode.java | 89 +++++++++++++++---- java/src/org/openqa/selenium/net/Urls.java | 10 ++- .../netty/server/RequestConverter.java | 25 +++++- .../openqa/selenium/remote/DriverCommand.java | 1 + .../selenium/remote/RemoteWebDriver.java | 14 ++- .../codec/AbstractHttpCommandCodec.java | 2 + .../codec/w3c/W3CHttpResponseCodec.java | 21 +++-- .../openqa/selenium/remote/http/Contents.java | 88 +++++++++++++++++- .../selenium/remote/http/HttpMessage.java | 9 ++ .../selenium/remote/http/HttpRequest.java | 6 +- .../selenium/remote/http/HttpResponse.java | 6 +- .../grid/node/local/LocalNodeTest.java | 7 ++ 15 files changed, 249 insertions(+), 38 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/data/Session.java b/java/src/org/openqa/selenium/grid/data/Session.java index 1cf20ecaf3dd1..061cbfb6b212e 100644 --- a/java/src/org/openqa/selenium/grid/data/Session.java +++ b/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()); + } } diff --git a/java/src/org/openqa/selenium/grid/node/BUILD.bazel b/java/src/org/openqa/selenium/grid/node/BUILD.bazel index 86547ce629a55..2e4b12da144f2 100644 --- a/java/src/org/openqa/selenium/grid/node/BUILD.bazel +++ b/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"), ], ) diff --git a/java/src/org/openqa/selenium/grid/node/Node.java b/java/src/org/openqa/selenium/grid/node/Node.java index d2b0d5223802b..2f5258f4cb828 100644 --- a/java/src/org/openqa/selenium/grid/node/Node.java +++ b/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")), diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java index 8be9d40aaea3a..5033d34a94699 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/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; @@ -50,6 +57,7 @@ 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 +115,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 +132,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 +752,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 +767,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 +824,60 @@ 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 data = Map.of( "filename", filename, - "file", getFileInfo(allFiles[0]), + "file", getFileInfo(file), "contents", content); Map> 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( + "Last-Modified", + HTTP_DATE_FORMAT.format(attributes.lastModifiedTime().toInstant().atZone(UTC))) + .setContent(Contents.file(file)); + } + + private File findDownloadedFile(File downloadsDirectory, String filename) + throws WebDriverException { + List matchingFiles = + asList( + requireNonNullElseGet( + downloadsDirectory.listFiles((dir, name) -> name.equals(filename)), + () -> new File[0])); + if (matchingFiles.isEmpty()) { + List 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 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) { diff --git a/java/src/org/openqa/selenium/net/Urls.java b/java/src/org/openqa/selenium/net/Urls.java index 3efde2635ea0b..c7429265fa903 100644 --- a/java/src/org/openqa/selenium/net/Urls.java +++ b/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.jspecify.annotations.NullMarked; import org.openqa.selenium.internal.Require; @@ -43,7 +45,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) { diff --git a/java/src/org/openqa/selenium/netty/server/RequestConverter.java b/java/src/org/openqa/selenium/netty/server/RequestConverter.java index 6cb1c91ab3eb5..676c650d4a3f9 100644 --- a/java/src/org/openqa/selenium/netty/server/RequestConverter.java +++ b/java/src/org/openqa/selenium/netty/server/RequestConverter.java @@ -37,9 +37,12 @@ import io.netty.handler.codec.http.LastHttpContent; import io.netty.handler.codec.http.QueryStringDecoder; import io.netty.util.ReferenceCountUtil; + +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; +import java.nio.charset.Charset; import java.util.Arrays; import java.util.List; import java.util.logging.Logger; @@ -56,7 +59,7 @@ class RequestConverter extends SimpleChannelInboundHandler { private static final List SUPPORTED_METHODS = Arrays.asList(DELETE, GET, POST, OPTIONS); private volatile FileBackedOutputStream buffer; - private volatile int length; + private volatile long length; private volatile HttpRequest request; @Override @@ -119,7 +122,7 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex if (buffer != null) { ByteSource source = buffer.asByteSource(); - int len = length; + long len = length; request.setContent( new Contents.Supplier() { @@ -133,7 +136,7 @@ public InputStream get() { } @Override - public int length() { + public long length() { return len; } @@ -141,6 +144,22 @@ public int length() { public void close() throws IOException { buffer.reset(); } + + @Override + public String toString() { + return String.format("Last content for %s (%s bytes)", request.toString(), len); + } + + @Override + public String contentAsString(Charset charset) { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try { + source.copyTo(out); + } catch (IOException e) { + throw new RuntimeException(e); + } + return out.toString(charset); + } }); } else { request.setContent(Contents.empty()); diff --git a/java/src/org/openqa/selenium/remote/DriverCommand.java b/java/src/org/openqa/selenium/remote/DriverCommand.java index a7df4b51dbcbd..997ab7cf9a594 100644 --- a/java/src/org/openqa/selenium/remote/DriverCommand.java +++ b/java/src/org/openqa/selenium/remote/DriverCommand.java @@ -154,6 +154,7 @@ public interface DriverCommand { String RESET_COOLDOWN = "resetCooldown"; String GET_DOWNLOADABLE_FILES = "getDownloadableFiles"; String DOWNLOAD_FILE = "downloadFile"; + String GET_DOWNLOADED_FILE = "getDownloadedFile"; String DELETE_DOWNLOADABLE_FILES = "deleteDownloadableFiles"; static CommandPayload NEW_SESSION(Capabilities capabilities) { diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index d6de989b2d9e6..d943db7a6624d 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -23,9 +23,12 @@ import static org.openqa.selenium.HasDownloads.DownloadedFile; import static org.openqa.selenium.remote.CapabilityType.PLATFORM_NAME; +import java.io.BufferedInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.Files; import java.nio.file.Path; import java.time.Duration; import java.util.ArrayList; @@ -80,7 +83,6 @@ import org.openqa.selenium.interactions.Sequence; import org.openqa.selenium.internal.Debug; import org.openqa.selenium.internal.Require; -import org.openqa.selenium.io.Zip; import org.openqa.selenium.logging.LocalLogs; import org.openqa.selenium.logging.LoggingHandler; import org.openqa.selenium.logging.Logs; @@ -88,6 +90,7 @@ import org.openqa.selenium.print.PrintOptions; import org.openqa.selenium.remote.http.ClientConfig; import org.openqa.selenium.remote.http.ConnectionFailedException; +import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.service.DriverCommandExecutor; import org.openqa.selenium.remote.tracing.TracedHttpClient; @@ -727,9 +730,12 @@ public List getDownloadedFiles() { public void downloadFile(String fileName, Path targetLocation) throws IOException { requireDownloadsEnabled(capabilities); - Response response = execute(DriverCommand.DOWNLOAD_FILE, Map.of("name", fileName)); - String contents = ((Map) response.getValue()).get("contents"); - Zip.unzip(contents, targetLocation.toFile()); + Response response = execute(DriverCommand.GET_DOWNLOADED_FILE, Map.of("name", fileName)); + + Contents.Supplier content = (Contents.Supplier) response.getValue(); + try (InputStream fileContent = content.get()) { + Files.copy(new BufferedInputStream(fileContent), targetLocation.resolve(fileName)); + } } /** diff --git a/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java b/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java index 412d742fa079c..a92b271a4a839 100644 --- a/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java +++ b/java/src/org/openqa/selenium/remote/codec/AbstractHttpCommandCodec.java @@ -46,6 +46,7 @@ import static org.openqa.selenium.remote.DriverCommand.GET_CREDENTIALS; import static org.openqa.selenium.remote.DriverCommand.GET_CURRENT_URL; import static org.openqa.selenium.remote.DriverCommand.GET_DOWNLOADABLE_FILES; +import static org.openqa.selenium.remote.DriverCommand.GET_DOWNLOADED_FILE; import static org.openqa.selenium.remote.DriverCommand.GET_ELEMENT_RECT; import static org.openqa.selenium.remote.DriverCommand.GET_ELEMENT_TAG_NAME; import static org.openqa.selenium.remote.DriverCommand.GET_ELEMENT_TEXT; @@ -199,6 +200,7 @@ public AbstractHttpCommandCodec() { defineCommand(GET_DOWNLOADABLE_FILES, get(sessionId + "/se/files")); defineCommand(DOWNLOAD_FILE, post(sessionId + "/se/files")); + defineCommand(GET_DOWNLOADED_FILE, get(sessionId + "/se/files/:name")); defineCommand(DELETE_DOWNLOADABLE_FILES, delete(sessionId + "/se/files")); } diff --git a/java/src/org/openqa/selenium/remote/codec/w3c/W3CHttpResponseCodec.java b/java/src/org/openqa/selenium/remote/codec/w3c/W3CHttpResponseCodec.java index 33cbf8a5f0e71..acf41b4e9d344 100644 --- a/java/src/org/openqa/selenium/remote/codec/w3c/W3CHttpResponseCodec.java +++ b/java/src/org/openqa/selenium/remote/codec/w3c/W3CHttpResponseCodec.java @@ -23,8 +23,8 @@ import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR; import static org.openqa.selenium.json.Json.MAP_TYPE; import static org.openqa.selenium.json.Json.OBJECT_TYPE; -import static org.openqa.selenium.remote.http.Contents.string; +import com.google.common.net.MediaType; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -73,11 +73,12 @@ public class W3CHttpResponseCodec extends AbstractHttpResponseCodec { @Override public Response decode(HttpResponse encodedResponse) { - String content = string(encodedResponse).trim(); - LOG.log( - Level.FINER, - "Decoding response. Response code was: {0} and content: {1}", - new Object[] {encodedResponse.getStatus(), content}); + if (LOG.isLoggable(Level.FINER)) { + LOG.log( + Level.FINER, + "Decoding response. Response code was: {0} and content: {1}", + new Object[] {encodedResponse.getStatus(), encodedResponse.getContent()}); + } String contentType = Objects.requireNonNullElse(encodedResponse.getHeader(HttpHeader.ContentType.getName()), ""); @@ -88,6 +89,7 @@ public Response decode(HttpResponse encodedResponse) { // text"} if (!encodedResponse.isSuccessful()) { LOG.fine("Processing an error"); + String content = encodedResponse.contentAsString().trim(); if (HTTP_BAD_METHOD == encodedResponse.getStatus()) { response.setState("unknown command"); response.setStatus(ErrorCodes.UNKNOWN_COMMAND); @@ -143,6 +145,13 @@ public Response decode(HttpResponse encodedResponse) { response.setState("success"); response.setStatus(ErrorCodes.SUCCESS); + + if (contentType.startsWith(MediaType.OCTET_STREAM.toString())) { + response.setValue(encodedResponse.getContent()); + return response; + } + + String content = encodedResponse.contentAsString().trim(); if (!content.isEmpty()) { if (contentType.startsWith("application/json")) { Map parsed = json.toType(content, MAP_TYPE); diff --git a/java/src/org/openqa/selenium/remote/http/Contents.java b/java/src/org/openqa/selenium/remote/http/Contents.java index 2944d8178d825..a334d9cf95b8c 100644 --- a/java/src/org/openqa/selenium/remote/http/Contents.java +++ b/java/src/org/openqa/selenium/remote/http/Contents.java @@ -18,6 +18,7 @@ package org.openqa.selenium.remote.http; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.Files.readAttributes; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -31,6 +32,7 @@ import java.lang.reflect.Type; import java.nio.charset.Charset; import java.nio.file.Files; +import java.nio.file.attribute.BasicFileAttributes; import java.util.Base64; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; @@ -52,7 +54,7 @@ public interface Supplier extends java.util.function.Supplier, Auto * @return the number of bytes that can be read from the InputStream returned by calling the get * method. */ - int length(); + long length(); /** * Release the related resources, if any. @@ -61,6 +63,8 @@ public interface Supplier extends java.util.function.Supplier, Auto */ @Override void close() throws IOException; + + String contentAsString(Charset charset); } private Contents() { @@ -84,6 +88,57 @@ public static Supplier string(CharSequence value, Charset charset) { return bytes(value.toString().getBytes(charset)); } + public static Supplier file(final File file) { + Require.nonNull("File to download", file); + + return new Supplier() { + private volatile InputStream inputStream; + + @Override + public synchronized InputStream get() { + if (inputStream != null) { + throw new IllegalStateException("File input stream has been opened before"); + } + try { + inputStream = Files.newInputStream(file.toPath()); + } catch (IOException e) { + throw new IllegalStateException("File not readable: " + file.getAbsolutePath(), e); + } + + return inputStream; + } + + @Override + public long length() { + try { + BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); + return attributes.size(); + } catch (IOException e) { + throw new IllegalStateException("File not readable: " + file.getAbsolutePath(), e); + } + } + + public void close() { + if (inputStream != null) { + try { + inputStream.close(); + } catch (IOException ignore) { + } + } + } + + @Override + public String toString() { + return String.format("Contents.file(%s)", file); + } + + @Override + public String contentAsString(Charset charset) { + throw new UnsupportedOperationException("File content may be too large"); + } + }; + } + public static Supplier bytes(byte[] bytes) { Require.nonNull("Bytes to return", bytes, "may be empty"); @@ -98,7 +153,7 @@ public InputStream get() { } @Override - public int length() { + public long length() { if (closed) throw new IllegalStateException("Contents.Supplier has been closed before"); return bytes.length; @@ -107,6 +162,18 @@ public int length() { public void close() { closed = true; } + + @Override + public String toString() { + return bytes.length < 256 ? + new String(bytes, UTF_8) : + String.format("%s bytes: \"%s\"...", bytes.length, new String(bytes, 0, 256, UTF_8)); + } + + @Override + public String contentAsString(Charset charset) { + return new String(bytes, charset); + } }; } @@ -126,15 +193,23 @@ public static String utf8String(Supplier supplier) { return string(supplier, UTF_8); } + /** + * @deprecated Use method {@link Supplier#contentAsString(Charset)} instead. + */ + @Deprecated public static String string(Supplier supplier, Charset charset) { Require.nonNull("Supplier of input", supplier); Require.nonNull("Character set", charset); - return new String(bytes(supplier), charset); + return supplier.contentAsString(charset); } + /** + * @deprecated Use method {@link HttpMessage#contentAsString()} instead + */ + @Deprecated public static String string(HttpMessage message) { - return string(message.getContent(), message.getContentEncoding()); + return message.contentAsString(); } public static Reader utf8Reader(Supplier supplier) { @@ -175,6 +250,11 @@ public static T fromJson(HttpMessage message, Type typeOfT) { } } + /** + * @deprecated Not needed anymore. It's a bad idea to read the entire file to memory. It may cause + * OutOfMemory errors in case of large files. + */ + @Deprecated public static String string(File input) throws IOException { try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); InputStream isr = Files.newInputStream(input.toPath())) { diff --git a/java/src/org/openqa/selenium/remote/http/HttpMessage.java b/java/src/org/openqa/selenium/remote/http/HttpMessage.java index 13d7de07205df..26d74b76cc3ec 100644 --- a/java/src/org/openqa/selenium/remote/http/HttpMessage.java +++ b/java/src/org/openqa/selenium/remote/http/HttpMessage.java @@ -183,6 +183,15 @@ public Contents.Supplier getContent() { return content; } + @Override + public String toString() { + return getContent().toString(); + } + + public String contentAsString() { + return getContent().contentAsString(getContentEncoding()); + } + @SuppressWarnings("unchecked") private M self() { return (M) this; diff --git a/java/src/org/openqa/selenium/remote/http/HttpRequest.java b/java/src/org/openqa/selenium/remote/http/HttpRequest.java index 2f0e50511dffc..cf212f673998f 100644 --- a/java/src/org/openqa/selenium/remote/http/HttpRequest.java +++ b/java/src/org/openqa/selenium/remote/http/HttpRequest.java @@ -74,7 +74,11 @@ public Iterable getQueryParameters(String name) { return queryParameters.get(name); } + @Override public String toString() { - return "(" + getMethod() + ") " + getUri(); + String content = super.toString(); + return content.isEmpty() + ? String.format("(%s) %s", getMethod(), getUri()) + : String.format("(%s) %s %s", getMethod(), getUri(), content); } } diff --git a/java/src/org/openqa/selenium/remote/http/HttpResponse.java b/java/src/org/openqa/selenium/remote/http/HttpResponse.java index be8e90db28946..81166829db73e 100644 --- a/java/src/org/openqa/selenium/remote/http/HttpResponse.java +++ b/java/src/org/openqa/selenium/remote/http/HttpResponse.java @@ -18,7 +18,6 @@ package org.openqa.selenium.remote.http; import static java.net.HttpURLConnection.HTTP_OK; -import static org.openqa.selenium.remote.http.Contents.string; public class HttpResponse extends HttpMessage { @@ -60,6 +59,9 @@ public String getTargetHost() { @Override public String toString() { - return String.format("%s: %s", getStatus(), string(this)); + String content = super.toString(); + return content.isEmpty() + ? String.format("%s", getStatus()) + : String.format("%s: %s", getStatus(), content); } } diff --git a/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java b/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java index ccd2b44f3dae8..f3d696309c124 100644 --- a/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java +++ b/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java @@ -434,4 +434,11 @@ void bidiIsDisabledAndResponseCapsShowThat() throws URISyntaxException { assertThat(bidiEnabled).isNotNull(); assertThat(Boolean.parseBoolean(bidiEnabled.toString())).isFalse(); } + + @Test + void extractsFileNameFromRequestUri() { + assertThat(node.extractFileName("/session/1234/se/files/logo.png")).isEqualTo("logo.png"); + assertThat(node.extractFileName("/session/1234/se/files/файл+with+tähtedega.png")) + .isEqualTo("файл+with+tähtedega.png"); + } } From dd3e920a1f519ba469048e5f75397218ea4d4277 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sat, 22 Nov 2025 12:26:44 +0200 Subject: [PATCH 2/5] #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. --- ...FileBackedOutputStreamContentSupplier.java | 74 ++++++++++++++++ .../netty/server/RequestConverter.java | 56 ++---------- .../remote/http/BytesContentSupplier.java | 65 ++++++++++++++ .../openqa/selenium/remote/http/Contents.java | 88 +------------------ .../remote/http/FileContentSupplier.java | 80 +++++++++++++++++ 5 files changed, 227 insertions(+), 136 deletions(-) create mode 100644 java/src/org/openqa/selenium/netty/server/FileBackedOutputStreamContentSupplier.java create mode 100644 java/src/org/openqa/selenium/remote/http/BytesContentSupplier.java create mode 100644 java/src/org/openqa/selenium/remote/http/FileContentSupplier.java diff --git a/java/src/org/openqa/selenium/netty/server/FileBackedOutputStreamContentSupplier.java b/java/src/org/openqa/selenium/netty/server/FileBackedOutputStreamContentSupplier.java new file mode 100644 index 0000000000000..721faf95b78f7 --- /dev/null +++ b/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); + } +} diff --git a/java/src/org/openqa/selenium/netty/server/RequestConverter.java b/java/src/org/openqa/selenium/netty/server/RequestConverter.java index 676c650d4a3f9..9c990eea72f10 100644 --- a/java/src/org/openqa/selenium/netty/server/RequestConverter.java +++ b/java/src/org/openqa/selenium/netty/server/RequestConverter.java @@ -23,7 +23,6 @@ import static io.netty.handler.codec.http.HttpMethod.OPTIONS; import static io.netty.handler.codec.http.HttpMethod.POST; -import com.google.common.io.ByteSource; import com.google.common.io.FileBackedOutputStream; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; @@ -37,14 +36,9 @@ import io.netty.handler.codec.http.LastHttpContent; import io.netty.handler.codec.http.QueryStringDecoder; import io.netty.util.ReferenceCountUtil; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.UncheckedIOException; -import java.nio.charset.Charset; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Logger; import org.openqa.selenium.internal.Debug; import org.openqa.selenium.remote.http.Contents; @@ -59,7 +53,7 @@ class RequestConverter extends SimpleChannelInboundHandler { private static final List SUPPORTED_METHODS = Arrays.asList(DELETE, GET, POST, OPTIONS); private volatile FileBackedOutputStream buffer; - private volatile long length; + private final AtomicLong length = new AtomicLong(); private volatile HttpRequest request; @Override @@ -96,7 +90,7 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex AttributeKey.HTTP_FLAVOR.getKey(), nettyRequest.protocolVersion().majorVersion()); buffer = null; - length = -1; + length.set(-1); } if (request != null && msg instanceof HttpContent) { @@ -106,12 +100,12 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex if (nBytes > 0) { if (buffer == null) { buffer = new FileBackedOutputStream(3 * 1024 * 1024, true); - length = 0; + length.set(0); } try { buf.readBytes(buffer, nBytes); - length += nBytes; + length.addAndGet(nBytes); } finally { buf.release(); } @@ -121,46 +115,8 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex LOG.log(Debug.getDebugLogLevel(), "End of http request: {0}", msg); if (buffer != null) { - ByteSource source = buffer.asByteSource(); - long len = length; - request.setContent( - new Contents.Supplier() { - @Override - public InputStream get() { - try { - return source.openBufferedStream(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - @Override - public long length() { - return len; - } - - @Override - public void close() throws IOException { - buffer.reset(); - } - - @Override - public String toString() { - return String.format("Last content for %s (%s bytes)", request.toString(), len); - } - - @Override - public String contentAsString(Charset charset) { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - try { - source.copyTo(out); - } catch (IOException e) { - throw new RuntimeException(e); - } - return out.toString(charset); - } - }); + new FileBackedOutputStreamContentSupplier(request.toString(), buffer, length.get())); } else { request.setContent(Contents.empty()); } diff --git a/java/src/org/openqa/selenium/remote/http/BytesContentSupplier.java b/java/src/org/openqa/selenium/remote/http/BytesContentSupplier.java new file mode 100644 index 0000000000000..4acd674b46de5 --- /dev/null +++ b/java/src/org/openqa/selenium/remote/http/BytesContentSupplier.java @@ -0,0 +1,65 @@ +// 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.remote.http; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.io.ByteArrayInputStream; +import java.io.InputStream; +import java.nio.charset.Charset; +import org.openqa.selenium.internal.Require; + +public class BytesContentSupplier implements Contents.Supplier { + private final byte[] bytes; + + public BytesContentSupplier(byte[] bytes) { + this.bytes = Require.nonNull("Bytes to return", bytes, "may be empty"); + } + + private boolean closed; + + @Override + public InputStream get() { + if (closed) throw new IllegalStateException("Contents.Supplier has been closed before"); + + return new ByteArrayInputStream(bytes); + } + + @Override + public long length() { + if (closed) throw new IllegalStateException("Contents.Supplier has been closed before"); + + return bytes.length; + } + + public void close() { + closed = true; + } + + @Override + public String toString() { + return bytes.length < 256 + ? new String(bytes, UTF_8) + : String.format("%s bytes: \"%s\"...", bytes.length, new String(bytes, 0, 256, UTF_8)); + } + + @Override + public String contentAsString(Charset charset) { + return new String(bytes, charset); + } +} diff --git a/java/src/org/openqa/selenium/remote/http/Contents.java b/java/src/org/openqa/selenium/remote/http/Contents.java index a334d9cf95b8c..b3cf0d189b6d6 100644 --- a/java/src/org/openqa/selenium/remote/http/Contents.java +++ b/java/src/org/openqa/selenium/remote/http/Contents.java @@ -18,9 +18,7 @@ package org.openqa.selenium.remote.http; import static java.nio.charset.StandardCharsets.UTF_8; -import static java.nio.file.Files.readAttributes; -import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; @@ -32,7 +30,6 @@ import java.lang.reflect.Type; import java.nio.charset.Charset; import java.nio.file.Files; -import java.nio.file.attribute.BasicFileAttributes; import java.util.Base64; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; @@ -89,92 +86,11 @@ public static Supplier string(CharSequence value, Charset charset) { } public static Supplier file(final File file) { - Require.nonNull("File to download", file); - - return new Supplier() { - private volatile InputStream inputStream; - - @Override - public synchronized InputStream get() { - if (inputStream != null) { - throw new IllegalStateException("File input stream has been opened before"); - } - try { - inputStream = Files.newInputStream(file.toPath()); - } catch (IOException e) { - throw new IllegalStateException("File not readable: " + file.getAbsolutePath(), e); - } - - return inputStream; - } - - @Override - public long length() { - try { - BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); - return attributes.size(); - } catch (IOException e) { - throw new IllegalStateException("File not readable: " + file.getAbsolutePath(), e); - } - } - - public void close() { - if (inputStream != null) { - try { - inputStream.close(); - } catch (IOException ignore) { - } - } - } - - @Override - public String toString() { - return String.format("Contents.file(%s)", file); - } - - @Override - public String contentAsString(Charset charset) { - throw new UnsupportedOperationException("File content may be too large"); - } - }; + return new FileContentSupplier(file); } public static Supplier bytes(byte[] bytes) { - Require.nonNull("Bytes to return", bytes, "may be empty"); - - return new Supplier() { - private boolean closed; - - @Override - public InputStream get() { - if (closed) throw new IllegalStateException("Contents.Supplier has been closed before"); - - return new ByteArrayInputStream(bytes); - } - - @Override - public long length() { - if (closed) throw new IllegalStateException("Contents.Supplier has been closed before"); - - return bytes.length; - } - - public void close() { - closed = true; - } - - @Override - public String toString() { - return bytes.length < 256 ? - new String(bytes, UTF_8) : - String.format("%s bytes: \"%s\"...", bytes.length, new String(bytes, 0, 256, UTF_8)); - } - - @Override - public String contentAsString(Charset charset) { - return new String(bytes, charset); - } - }; + return new BytesContentSupplier(bytes); } public static byte[] bytes(Supplier supplier) { diff --git a/java/src/org/openqa/selenium/remote/http/FileContentSupplier.java b/java/src/org/openqa/selenium/remote/http/FileContentSupplier.java new file mode 100644 index 0000000000000..ae6461333b01d --- /dev/null +++ b/java/src/org/openqa/selenium/remote/http/FileContentSupplier.java @@ -0,0 +1,80 @@ +// 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.remote.http; + +import static java.nio.file.Files.readAttributes; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.attribute.BasicFileAttributes; +import org.openqa.selenium.internal.Require; + +class FileContentSupplier implements Contents.Supplier { + private final File file; + private volatile InputStream inputStream; + + FileContentSupplier(File file) { + this.file = Require.nonNull("File", file); + } + + @Override + public synchronized InputStream get() { + if (inputStream != null) { + throw new IllegalStateException("File input stream has been opened before"); + } + try { + inputStream = Files.newInputStream(file.toPath()); + } catch (IOException e) { + throw new IllegalStateException("File not readable: " + file.getAbsolutePath(), e); + } + + return inputStream; + } + + @Override + public long length() { + try { + BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); + return attributes.size(); + } catch (IOException e) { + throw new IllegalStateException("File not readable: " + file.getAbsolutePath(), e); + } + } + + public void close() { + if (inputStream != null) { + try { + inputStream.close(); + } catch (IOException ignore) { + } + } + } + + @Override + public String toString() { + return String.format("Contents.file(%s)", file); + } + + @Override + public String contentAsString(Charset charset) { + throw new UnsupportedOperationException("File content may be too large"); + } +} From 8371cc18cca39c5db1cf178cfeede9682db252ca Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sat, 22 Nov 2025 15:39:08 +0200 Subject: [PATCH 3/5] #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. --- .../selenium/grid/node/local/LocalNode.java | 10 ++- .../openqa/selenium/remote/http/Contents.java | 4 ++ .../http/InputStreamContentSupplier.java | 71 +++++++++++++++++++ .../remote/http/jdk/JdkHttpClient.java | 6 +- .../remote/http/jdk/JdkHttpMessages.java | 9 ++- .../grid/router/ReverseProxyEndToEndTest.java | 2 - 6 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 java/src/org/openqa/selenium/remote/http/InputStreamContentSupplier.java diff --git a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java index 5033d34a94699..9746525eb8b92 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -54,6 +54,7 @@ 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; @@ -844,12 +845,15 @@ private HttpResponse getDownloadedFile(File downloadsDirectory, String fileName) BasicFileAttributes attributes = readAttributes(file.toPath(), BasicFileAttributes.class); return new HttpResponse() .setHeader("Content-Type", MediaType.OCTET_STREAM.toString()) - .setHeader( - "Last-Modified", - HTTP_DATE_FORMAT.format(attributes.lastModifiedTime().toInstant().atZone(UTC))) + .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 matchingFiles = diff --git a/java/src/org/openqa/selenium/remote/http/Contents.java b/java/src/org/openqa/selenium/remote/http/Contents.java index b3cf0d189b6d6..1326eaad901c5 100644 --- a/java/src/org/openqa/selenium/remote/http/Contents.java +++ b/java/src/org/openqa/selenium/remote/http/Contents.java @@ -89,6 +89,10 @@ public static Supplier file(final File file) { return new FileContentSupplier(file); } + public static Supplier fromStream(InputStream stream, long length) { + return new InputStreamContentSupplier(stream, length); + } + public static Supplier bytes(byte[] bytes) { return new BytesContentSupplier(bytes); } diff --git a/java/src/org/openqa/selenium/remote/http/InputStreamContentSupplier.java b/java/src/org/openqa/selenium/remote/http/InputStreamContentSupplier.java new file mode 100644 index 0000000000000..1d09d07defaa3 --- /dev/null +++ b/java/src/org/openqa/selenium/remote/http/InputStreamContentSupplier.java @@ -0,0 +1,71 @@ +// 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.remote.http; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.Charset; +import org.openqa.selenium.internal.Require; + +class InputStreamContentSupplier implements Contents.Supplier { + + private static final int MAX_TEXT_RESPONSE_SIZE = 256 * 1024 * 1024; + private final InputStream stream; + private final long length; + + InputStreamContentSupplier(InputStream stream, long length) { + this.stream = Require.nonNull("InputStream", stream); + this.length = length; + } + + @Override + public InputStream get() { + return stream; + } + + @Override + public long length() { + return length; + } + + public void close() { + try { + stream.close(); + } catch (IOException ignore) { + } + } + + @Override + public String toString() { + return String.format("Contents.fromStream(%s bytes)", length); + } + + @Override + public String contentAsString(Charset charset) { + if (length > MAX_TEXT_RESPONSE_SIZE) { + throw new UnsupportedOperationException("Cannot print out too large stream content"); + } + try { + return new String(stream.readAllBytes(), UTF_8); + } catch (IOException e) { + throw new RuntimeException(e); + } + } +} diff --git a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java index 931f90c79462c..3f456d92d5f7f 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -20,6 +20,7 @@ import com.google.auto.service.AutoService; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.UncheckedIOException; import java.net.Authenticator; import java.net.PasswordAuthentication; @@ -440,8 +441,7 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { LOG.log(Level.FINE, "Executing request: {0}", req); long start = System.currentTimeMillis(); - - BodyHandler byteHandler = BodyHandlers.ofByteArray(); + BodyHandler byteHandler = BodyHandlers.ofInputStream(); try { HttpMethod method = req.getMethod(); URI rawUri = messages.getRawUri(req); @@ -456,7 +456,7 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { } java.net.http.HttpRequest request = messages.createRequest(req, method, rawUri); - java.net.http.HttpResponse response = client.send(request, byteHandler); + java.net.http.HttpResponse response = client.send(request, byteHandler); switch (response.statusCode()) { case 303: diff --git a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java index 3491d9c1bbf00..9b7bc416c4a43 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java @@ -19,6 +19,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; +import java.io.InputStream; import java.net.URI; import java.net.URLEncoder; import java.net.http.HttpRequest.BodyPublisher; @@ -152,7 +153,7 @@ public URI getRawUri(HttpRequest req) { return URI.create(rawUrl); } - public HttpResponse createResponse(java.net.http.HttpResponse response) { + public HttpResponse createResponse(java.net.http.HttpResponse response) { HttpResponse res = new HttpResponse(); res.setStatus(response.statusCode()); response @@ -163,10 +164,8 @@ public HttpResponse createResponse(java.net.http.HttpResponse response) values.stream() .filter(Objects::nonNull) .forEach(value -> res.addHeader(name, value))); - byte[] responseBody = response.body(); - if (responseBody != null) { - res.setContent(Contents.bytes(responseBody)); - } + long length = response.headers().firstValueAsLong("Content-Length").orElse(-1); + res.setContent(Contents.fromStream(response.body(), length)); return res; } diff --git a/java/test/org/openqa/selenium/grid/router/ReverseProxyEndToEndTest.java b/java/test/org/openqa/selenium/grid/router/ReverseProxyEndToEndTest.java index 6d750338b2ac9..c78dc0f22128f 100644 --- a/java/test/org/openqa/selenium/grid/router/ReverseProxyEndToEndTest.java +++ b/java/test/org/openqa/selenium/grid/router/ReverseProxyEndToEndTest.java @@ -53,7 +53,6 @@ import org.openqa.selenium.json.JsonOutput; import org.openqa.selenium.remote.RemoteWebDriver; import org.openqa.selenium.remote.SessionId; -import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.http.HttpHandler; import org.openqa.selenium.remote.http.HttpRequest; @@ -125,7 +124,6 @@ private static void waitUntilReady(Server server, Duration duration) { .until( c -> { HttpResponse response = c.execute(new HttpRequest(GET, "/status")); - System.out.println(Contents.string(response)); Map status = Values.get(response, MAP_TYPE); return Boolean.TRUE.equals( status != null && Boolean.parseBoolean(status.get("ready").toString())); From 79ff056ad528ef57c215eb18bf73b9ce9aa9e3d4 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sun, 23 Nov 2025 00:15:42 +0200 Subject: [PATCH 4/5] #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. --- java/test/org/openqa/selenium/grid/node/NodeTest.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/java/test/org/openqa/selenium/grid/node/NodeTest.java b/java/test/org/openqa/selenium/grid/node/NodeTest.java index b41a62f2f117e..2180d812ee408 100644 --- a/java/test/org/openqa/selenium/grid/node/NodeTest.java +++ b/java/test/org/openqa/selenium/grid/node/NodeTest.java @@ -149,7 +149,7 @@ public HttpResponse execute(HttpRequest req) throws UncheckedIOException { .add(caps, new TestSessionFactory((id, c) -> new Handler(c))) .maximumConcurrentSessions(2); if (isDownloadsTestCase) { - builder = builder.enableManagedDownloads(true).sessionTimeout(Duration.ofSeconds(1)); + builder = builder.enableManagedDownloads(true).sessionTimeout(ofSeconds(1)); } local = builder.build(); local2 = builder.build(); @@ -568,7 +568,7 @@ void canUploadAFile() throws IOException { assertThat(new String(Files.readAllBytes(uploadDir.listFiles()[0].toPath()))).isEqualTo(hello); node.stop(session.getId()); - assertThat(baseDir).doesNotExist(); + waitUntilDirGetsDeleted(baseDir); } @Test @@ -644,7 +644,7 @@ void canDownloadMultipleFile() throws IOException { TemporaryFilesystem downloadsTfs = local.getDownloadsFilesystem(session.getId()); File someDir = getTemporaryFilesystemBaseDir(downloadsTfs); node.stop(session.getId()); - assertThat(someDir).doesNotExist(); + waitUntilDirGetsDeleted(someDir); } } @@ -975,6 +975,10 @@ private List listFileDownloads(SessionId sessionId) { return (List) map.get("names"); } + private void waitUntilDirGetsDeleted(File dir) { + new FluentWait<>(dir).withTimeout(ofSeconds(2)).until(file -> !file.exists()); + } + private static class MyClock extends Clock { private final AtomicReference now; From 2c190cc9aabe9d84a2b7b599e163af258193dc75 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sun, 23 Nov 2025 14:25:35 +0200 Subject: [PATCH 5/5] #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. --- .../openqa/selenium/grid/data/NodeStatus.java | 6 ++ .../grid/node/local/LocalNodeTest.java | 78 ++++++++++++------- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/data/NodeStatus.java b/java/src/org/openqa/selenium/grid/data/NodeStatus.java index 4b52a617203bd..010ed40b81281 100644 --- a/java/src/org/openqa/selenium/grid/data/NodeStatus.java +++ b/java/src/org/openqa/selenium/grid/data/NodeStatus.java @@ -248,4 +248,10 @@ public Map toJson() { return unmodifiableMap(toReturn); } + + @Override + public String toString() { + return String.format( + "NodeStatus(availability: %s, slots:%s, uri:%s)", availability, slots.size(), externalUri); + } } diff --git a/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java b/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java index f3d696309c124..640a3318c8aaf 100644 --- a/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java +++ b/java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java @@ -17,6 +17,7 @@ package org.openqa.selenium.grid.node.local; +import static java.lang.System.currentTimeMillis; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -27,6 +28,7 @@ import com.google.common.collect.ImmutableSet; import java.net.URI; import java.net.URISyntaxException; +import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.List; @@ -117,14 +119,21 @@ void isOwnerOfAnActiveSession() { @Test void canStopASession() { - node.stop(session.getId()); + SessionId sessionId = session.getId(); + assertThat(node.getSession(sessionId)).isNotNull(); + + node.stop(sessionId); + + waitUntilNodeStopped(sessionId); assertThatExceptionOfType(NoSuchSessionException.class) - .isThrownBy(() -> node.getSession(session.getId())); + .isThrownBy(() -> node.getSession(sessionId)); } @Test void isNotOwnerOfAStoppedSession() { node.stop(session.getId()); + + waitUntilNodeStopped(session.getId()); assertThat(node.isSessionOwner(session.getId())).isFalse(); } @@ -132,7 +141,9 @@ void isNotOwnerOfAStoppedSession() { void cannotAcceptNewSessionsWhileDraining() { node.drain(); assertThat(node.isDraining()).isTrue(); + node.stop(session.getId()); // stop the default session + waitUntilNodeStopped(session.getId()); Capabilities stereotype = new ImmutableCapabilities("cheese", "brie"); Either sessionResponse = @@ -155,41 +166,25 @@ void cannotCreateNewSessionsOnMaxSessionCount() { @Test void canReturnStatusInfo() { - NodeStatus status = node.getStatus(); - assertThat( - status.getSlots().stream() - .map(Slot::getSession) - .filter(Objects::nonNull) - .filter(s -> s.getId().equals(session.getId()))) - .isNotEmpty(); + SessionId sessionId = session.getId(); + assertThat(findSession(sessionId)).isNotEmpty(); - node.stop(session.getId()); - status = node.getStatus(); - assertThat( - status.getSlots().stream() - .map(Slot::getSession) - .filter(Objects::nonNull) - .filter(s -> s.getId().equals(session.getId()))) - .isEmpty(); + node.stop(sessionId); + waitUntilNodeStopped(sessionId); + + assertThat(findSession(sessionId)).isEmpty(); } @Test void nodeStatusInfoIsImmutable() { + SessionId sessionId = session.getId(); NodeStatus status = node.getStatus(); - assertThat( - status.getSlots().stream() - .map(Slot::getSession) - .filter(Objects::nonNull) - .filter(s -> s.getId().equals(session.getId()))) - .isNotEmpty(); + assertThat(findSession(status, sessionId)).isNotEmpty(); - node.stop(session.getId()); - assertThat( - status.getSlots().stream() - .map(Slot::getSession) - .filter(Objects::nonNull) - .filter(s -> s.getId().equals(session.getId()))) - .isNotEmpty(); + node.stop(sessionId); + waitUntilNodeStopped(sessionId); + + assertThat(findSession(status, sessionId)).isNotEmpty(); } @Test @@ -441,4 +436,27 @@ void extractsFileNameFromRequestUri() { assertThat(node.extractFileName("/session/1234/se/files/файл+with+tähtedega.png")) .isEqualTo("файл+with+tähtedega.png"); } + + private void waitUntilNodeStopped(SessionId sessionId) { + long timeout = Duration.ofSeconds(5).toMillis(); + + for (long start = currentTimeMillis(); currentTimeMillis() - start < timeout; ) { + if (findSession(sessionId).isEmpty()) { + break; + } + } + } + + private Optional findSession(SessionId sessionId) { + NodeStatus status = node.getStatus(); + return findSession(status, sessionId); + } + + private Optional findSession(NodeStatus status, SessionId sessionId) { + return status.getSlots().stream() + .map(Slot::getSession) + .filter(Objects::nonNull) + .filter(s -> s.getId().equals(sessionId)) + .findAny(); + } }