From fccfed1e9232026f0a3a0bf9ecf49e077939e0e4 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Tue, 26 Mar 2024 12:18:39 +0000 Subject: [PATCH 1/4] Explicitly export Select and WebDriverWait from selenium.webdriver.support.ui (#13491) Without this, `mypy` in strict mode gives: ``` Module "selenium.webdriver.support.ui" does not explicitly export attribute "Select" [attr-defined] ``` when following the documentation at https://selenium-python.readthedocs.io/navigating.html#filling-in-forms. Co-authored-by: Diego Molina --- py/selenium/webdriver/support/ui.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/py/selenium/webdriver/support/ui.py b/py/selenium/webdriver/support/ui.py index ee3b2b65ba9bf..25b9664bb44ae 100644 --- a/py/selenium/webdriver/support/ui.py +++ b/py/selenium/webdriver/support/ui.py @@ -15,5 +15,7 @@ # specific language governing permissions and limitations # under the License. -from .select import Select # noqa -from .wait import WebDriverWait # noqa +from .select import Select +from .wait import WebDriverWait + +__all__ = ["Select", "WebDriverWait"] From 4cfe983a3539ccb215b2b4fa869d7b8a7e456ad9 Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Tue, 26 Mar 2024 13:04:41 +0000 Subject: [PATCH 2/4] Add return type to webelement.submit (#13490) Without this, `mypy` in strict mode gives me the error: ``` Call to untyped function "submit" in typed context ``` Co-authored-by: Diego Molina --- py/selenium/webdriver/remote/webelement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/selenium/webdriver/remote/webelement.py b/py/selenium/webdriver/remote/webelement.py index fe2626a8f2817..d5752ca678180 100644 --- a/py/selenium/webdriver/remote/webelement.py +++ b/py/selenium/webdriver/remote/webelement.py @@ -93,7 +93,7 @@ def click(self) -> None: """Clicks the element.""" self._execute(Command.CLICK_ELEMENT) - def submit(self): + def submit(self) -> None: """Submits a form.""" script = ( "/* submitForm */var form = arguments[0];\n" From 55e7a536e94ec4e86ff979f3e354602d644af5e2 Mon Sep 17 00:00:00 2001 From: joerg1985 <16140691+joerg1985@users.noreply.github.com> Date: Tue, 26 Mar 2024 15:13:46 +0100 Subject: [PATCH 3/4] [java] removed usage of FileBackedOutputStream in the client (#13308) * [java] removed usage of FileBackedOutputStream in the client * [java] removed usage of FileBackedOutputStream in the client --------- Co-authored-by: Diego Molina --- .../selenium/devtools/idealized/Network.java | 6 +- .../selenium/grid/data/SessionRequest.java | 3 +- .../netty/server/RequestConverter.java | 43 ++++--- .../selenium/remote/NewSessionPayload.java | 60 +++------ .../selenium/remote/ProtocolHandshake.java | 31 +---- .../openqa/selenium/remote/http/BUILD.bazel | 1 - .../openqa/selenium/remote/http/Contents.java | 121 +++++++++--------- .../remote/http/DumpHttpExchangeFilter.java | 10 -- .../selenium/remote/http/HttpMessage.java | 15 ++- .../remote/http/jdk/JdkHttpMessages.java | 22 +--- .../openqa/selenium/grid/node/NodeTest.java | 11 +- .../DriverServiceSessionFactoryTest.java | 3 +- .../remote/NewSessionPayloadTest.java | 9 +- 13 files changed, 137 insertions(+), 198 deletions(-) diff --git a/java/src/org/openqa/selenium/devtools/idealized/Network.java b/java/src/org/openqa/selenium/devtools/idealized/Network.java index 3dcdabb534a65..0725b19b63e5f 100644 --- a/java/src/org/openqa/selenium/devtools/idealized/Network.java +++ b/java/src/org/openqa/selenium/devtools/idealized/Network.java @@ -21,8 +21,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.logging.Level.WARNING; -import java.io.ByteArrayInputStream; -import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; import java.util.Base64; @@ -293,13 +291,13 @@ protected HttpResponse createHttpResponse( String body, Boolean bodyIsBase64Encoded, List> headers) { - Supplier content; + Contents.Supplier content; if (body == null) { content = Contents.empty(); } else if (bodyIsBase64Encoded != null && bodyIsBase64Encoded) { byte[] decoded = Base64.getDecoder().decode(body); - content = () -> new ByteArrayInputStream(decoded); + content = Contents.bytes(decoded); } else { content = Contents.string(body, UTF_8); } diff --git a/java/src/org/openqa/selenium/grid/data/SessionRequest.java b/java/src/org/openqa/selenium/grid/data/SessionRequest.java index 88fc330b59b84..aa299de2a8129 100644 --- a/java/src/org/openqa/selenium/grid/data/SessionRequest.java +++ b/java/src/org/openqa/selenium/grid/data/SessionRequest.java @@ -41,7 +41,6 @@ import org.openqa.selenium.json.TypeToken; import org.openqa.selenium.remote.Dialect; import org.openqa.selenium.remote.NewSessionPayload; -import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpRequest; public class SessionRequest { @@ -61,7 +60,7 @@ public SessionRequest(RequestId requestId, HttpRequest request, Instant enqueued this.enqueued = Require.nonNull("Enqueued time", enqueued); Require.nonNull("Request", request); - try (NewSessionPayload payload = NewSessionPayload.create(Contents.reader(request))) { + try (NewSessionPayload payload = NewSessionPayload.create(request.getContent())) { desiredCapabilities = payload.stream() .filter(capabilities -> !capabilities.asMap().isEmpty()) diff --git a/java/src/org/openqa/selenium/netty/server/RequestConverter.java b/java/src/org/openqa/selenium/netty/server/RequestConverter.java index aa02ea6ab9f53..d245530963922 100644 --- a/java/src/org/openqa/selenium/netty/server/RequestConverter.java +++ b/java/src/org/openqa/selenium/netty/server/RequestConverter.java @@ -44,6 +44,7 @@ import java.util.List; import java.util.logging.Logger; import org.openqa.selenium.internal.Debug; +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; @@ -55,6 +56,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 HttpRequest request; @Override @@ -91,6 +93,7 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex AttributeKey.HTTP_FLAVOR.getKey(), nettyRequest.protocolVersion().majorVersion()); buffer = null; + length = -1; } if (msg instanceof HttpContent) { @@ -100,10 +103,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; } try { buf.readBytes(buffer, nBytes); + length += nBytes; } finally { buf.release(); } @@ -114,29 +119,31 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpObject msg) throws Ex if (buffer != null) { ByteSource source = buffer.asByteSource(); + int len = length; request.setContent( - () -> { - try { - return source.openBufferedStream(); - } catch (IOException e) { - throw new UncheckedIOException(e); + new Contents.Supplier() { + @Override + public InputStream get() { + try { + return source.openBufferedStream(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Override + public int length() { + return len; + } + + @Override + public void close() throws IOException { + buffer.reset(); } }); } else { - request.setContent( - () -> - new InputStream() { - @Override - public int read() throws IOException { - return -1; - } - - @Override - public int read(byte[] b, int off, int len) throws IOException { - return -1; - } - }); + request.setContent(Contents.empty()); } ctx.fireChannelRead(request); diff --git a/java/src/org/openqa/selenium/remote/NewSessionPayload.java b/java/src/org/openqa/selenium/remote/NewSessionPayload.java index 724f1f3c25e66..9f49eb00ca449 100644 --- a/java/src/org/openqa/selenium/remote/NewSessionPayload.java +++ b/java/src/org/openqa/selenium/remote/NewSessionPayload.java @@ -21,15 +21,10 @@ import static org.openqa.selenium.json.Json.LIST_OF_MAPS_TYPE; import static org.openqa.selenium.json.Json.MAP_TYPE; -import com.google.common.io.FileBackedOutputStream; import java.io.Closeable; import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStreamWriter; import java.io.Reader; -import java.io.StringReader; import java.io.UncheckedIOException; -import java.io.Writer; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -50,6 +45,7 @@ import org.openqa.selenium.json.Json; import org.openqa.selenium.json.JsonInput; import org.openqa.selenium.json.JsonOutput; +import org.openqa.selenium.remote.http.Contents; public class NewSessionPayload implements Closeable { @@ -57,25 +53,11 @@ public class NewSessionPayload implements Closeable { private static final Predicate ACCEPTED_W3C_PATTERNS = new AcceptedW3CCapabilityKeys(); private final Json json = new Json(); - private final FileBackedOutputStream backingStore; + private final Contents.Supplier supplier; private final Set dialects; - private NewSessionPayload(Reader source) { - // Dedicate up to 10% of all RAM or 20% of available RAM (whichever is smaller) to storing this - // payload. - int threshold = - (int) - Math.min( - Integer.MAX_VALUE, - Math.min( - Runtime.getRuntime().freeMemory() / 5, Runtime.getRuntime().maxMemory() / 10)); - - backingStore = new FileBackedOutputStream(threshold); - try (Writer writer = new OutputStreamWriter(backingStore, UTF_8)) { - source.transferTo(writer); - } catch (IOException e) { - throw new UncheckedIOException(e); - } + private NewSessionPayload(Contents.Supplier supplier) { + this.supplier = supplier; Set dialects = new LinkedHashSet<>(); try { @@ -89,12 +71,6 @@ private NewSessionPayload(Reader source) { } catch (IOException e) { throw new UncheckedIOException(e); } - - try { - source.close(); - } catch (IOException e) { - // Ignore - } } public static NewSessionPayload create(Capabilities caps) { @@ -120,12 +96,15 @@ public static NewSessionPayload create(Map source) { Require.precondition( source.containsKey("capabilities"), "New session payload must contain capabilities"); - String json = new Json().toJson(Require.nonNull("Payload", source)); - return new NewSessionPayload(new StringReader(json)); + return new NewSessionPayload(Contents.asJson(Require.nonNull("Payload", source))); + } + + public static NewSessionPayload create(Contents.Supplier supplier) { + return new NewSessionPayload(supplier); } - public static NewSessionPayload create(Reader source) { - return new NewSessionPayload(source); + public Contents.Supplier getSupplier() { + return supplier; } private void validate() throws IOException { @@ -213,8 +192,7 @@ public void writeTo(Appendable appendable) throws IOException { } private void writeMetaData(JsonOutput out) throws IOException { - try (Reader reader = - new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8); + try (Reader reader = Contents.reader(supplier, UTF_8); JsonInput input = json.newInput(reader)) { input.beginObject(); while (input.hasNext()) { @@ -253,8 +231,7 @@ public Set getDownstreamDialects() { public Map getMetadata() { Set ignoredMetadataKeys = Set.of("capabilities"); - try (Reader reader = - new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8); + try (Reader reader = Contents.reader(supplier, UTF_8); JsonInput input = json.newInput(reader)) { Map toReturn = new LinkedHashMap<>(); @@ -284,7 +261,7 @@ public Map getMetadata() { @Override public void close() { try { - backingStore.reset(); + supplier.close(); } catch (IOException e) { throw new UncheckedIOException(e); } @@ -324,8 +301,7 @@ private Stream> getW3C() throws IOException { } private boolean isW3C() throws IOException { - try (Reader reader = - new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8); + try (Reader reader = Contents.reader(supplier, UTF_8); JsonInput input = json.newInput(reader)) { input.beginObject(); while (input.hasNext()) { @@ -341,8 +317,7 @@ private boolean isW3C() throws IOException { } private Map getAlwaysMatch() throws IOException { - try (Reader reader = - new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8); + try (Reader reader = Contents.reader(supplier, UTF_8); JsonInput input = json.newInput(reader)) { input.beginObject(); while (input.hasNext()) { @@ -367,8 +342,7 @@ private Map getAlwaysMatch() throws IOException { } private Collection> getFirstMatches() throws IOException { - try (Reader reader = - new InputStreamReader(backingStore.asByteSource().openBufferedStream(), UTF_8); + try (Reader reader = Contents.reader(supplier, UTF_8); JsonInput input = json.newInput(reader)) { input.beginObject(); while (input.hasNext()) { diff --git a/java/src/org/openqa/selenium/remote/ProtocolHandshake.java b/java/src/org/openqa/selenium/remote/ProtocolHandshake.java index 2aebdaecb4b76..f5bca85d9de82 100644 --- a/java/src/org/openqa/selenium/remote/ProtocolHandshake.java +++ b/java/src/org/openqa/selenium/remote/ProtocolHandshake.java @@ -17,23 +17,16 @@ package org.openqa.selenium.remote; -import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singleton; import static org.openqa.selenium.json.Json.JSON_UTF_8; import static org.openqa.selenium.remote.CapabilityType.PROXY; import static org.openqa.selenium.remote.http.Contents.string; -import com.google.common.io.CountingOutputStream; -import com.google.common.io.FileBackedOutputStream; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStreamWriter; -import java.io.Writer; import java.util.Collection; import java.util.Map; import java.util.Objects; import java.util.function.Function; -import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Stream; @@ -45,6 +38,7 @@ import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; import org.openqa.selenium.json.JsonException; +import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpHandler; import org.openqa.selenium.remote.http.HttpHeader; import org.openqa.selenium.remote.http.HttpMethod; @@ -78,36 +72,17 @@ public Result createSession(HttpHandler client, Command command) throws IOExcept public Either createSession( HttpHandler client, NewSessionPayload payload) throws IOException { - int threshold = (int) Math.min(Runtime.getRuntime().freeMemory() / 10, Integer.MAX_VALUE); - FileBackedOutputStream os = new FileBackedOutputStream(threshold, true); - - try (CountingOutputStream counter = new CountingOutputStream(os); - Writer writer = new OutputStreamWriter(counter, UTF_8)) { - payload.writeTo(writer); - Supplier contentSupplier = - () -> { - try { - return os.asByteSource().openBufferedStream(); - } catch (IOException e) { - throw new RuntimeException(e); - } - }; - return createSession(client, contentSupplier, counter.getCount()); - } + return createSession(client, payload.getSupplier()); } private Either createSession( - HttpHandler client, Supplier contentSupplier, long size) { + HttpHandler client, Contents.Supplier contentSupplier) { // Create the http request and send it HttpRequest request = new HttpRequest(HttpMethod.POST, "/session"); HttpResponse response; long start = System.currentTimeMillis(); - // Setting the CONTENT_LENGTH will allow a http client implementation not to read the data in - // memory. Usually the payload is small and buffering it to memory is okay, except for a new - // session e.g. with profiles. - request.setHeader(HttpHeader.ContentLength.getName(), String.valueOf(size)); request.setHeader(HttpHeader.ContentType.getName(), JSON_UTF_8); request.setContent(contentSupplier); diff --git a/java/src/org/openqa/selenium/remote/http/BUILD.bazel b/java/src/org/openqa/selenium/remote/http/BUILD.bazel index d0b46d51f9438..8719b69bda047 100644 --- a/java/src/org/openqa/selenium/remote/http/BUILD.bazel +++ b/java/src/org/openqa/selenium/remote/http/BUILD.bazel @@ -17,7 +17,6 @@ java_export( "//java:auto-service", "//java/src/org/openqa/selenium:core", "//java/src/org/openqa/selenium/json", - artifact("com.google.guava:guava"), artifact("dev.failsafe:failsafe"), ], ) diff --git a/java/src/org/openqa/selenium/remote/http/Contents.java b/java/src/org/openqa/selenium/remote/http/Contents.java index f0e48e2f868d3..2944d8178d825 100644 --- a/java/src/org/openqa/selenium/remote/http/Contents.java +++ b/java/src/org/openqa/selenium/remote/http/Contents.java @@ -19,20 +19,19 @@ import static java.nio.charset.StandardCharsets.UTF_8; -import com.google.common.io.FileBackedOutputStream; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.OutputStreamWriter; import java.io.Reader; import java.io.UncheckedIOException; import java.lang.reflect.Type; import java.nio.charset.Charset; import java.nio.file.Files; import java.util.Base64; -import java.util.function.Supplier; import org.openqa.selenium.internal.Require; import org.openqa.selenium.json.Json; import org.openqa.selenium.json.JsonInput; @@ -42,34 +41,76 @@ public class Contents { private static final Json JSON = new Json(); + /** + * A supplier that can be called multiple times, each invocation must return a new InputStream + * ready to read. The number of bytes that can be read from the InputStream must be identical to + * the number returned by the length method. + */ + public interface Supplier extends java.util.function.Supplier, AutoCloseable { + + /** + * @return the number of bytes that can be read from the InputStream returned by calling the get + * method. + */ + int length(); + + /** + * Release the related resources, if any. + * + * @throws IOException on I/O failure + */ + @Override + void close() throws IOException; + } + private Contents() { // Utility class } - public static Supplier empty() { + public static Supplier empty() { return bytes(new byte[0]); } - public static Supplier utf8String(CharSequence value) { + public static Supplier utf8String(CharSequence value) { Require.nonNull("Value to return", value); return string(value, UTF_8); } - public static Supplier string(CharSequence value, Charset charset) { + public static Supplier string(CharSequence value, Charset charset) { Require.nonNull("Value to return", value); Require.nonNull("Character set", charset); return bytes(value.toString().getBytes(charset)); } - public static Supplier bytes(byte[] bytes) { + public static Supplier bytes(byte[] bytes) { Require.nonNull("Bytes to return", bytes, "may be empty"); - return () -> new ByteArrayInputStream(bytes); + 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 int length() { + if (closed) throw new IllegalStateException("Contents.Supplier has been closed before"); + + return bytes.length; + } + + public void close() { + closed = true; + } + }; } - public static byte[] bytes(Supplier supplier) { + public static byte[] bytes(Supplier supplier) { Require.nonNull("Supplier of input", supplier); try (InputStream is = supplier.get(); @@ -81,11 +122,11 @@ public static byte[] bytes(Supplier supplier) { } } - public static String utf8String(Supplier supplier) { + public static String utf8String(Supplier supplier) { return string(supplier, UTF_8); } - public static String string(Supplier supplier, Charset charset) { + public static String string(Supplier supplier, Charset charset) { Require.nonNull("Supplier of input", supplier); Require.nonNull("Character set", charset); @@ -96,13 +137,13 @@ public static String string(HttpMessage message) { return string(message.getContent(), message.getContentEncoding()); } - public static Reader utf8Reader(Supplier supplier) { + public static Reader utf8Reader(Supplier supplier) { Require.nonNull("Supplier", supplier); return reader(supplier, UTF_8); } - public static Reader reader(Supplier supplier, Charset charset) { + public static Reader reader(Supplier supplier, Charset charset) { Require.nonNull("Supplier of input", supplier); Require.nonNull("Character set", charset); @@ -114,15 +155,15 @@ public static Reader reader(HttpMessage message) { } /** - * @return an {@link InputStream} containing the object converted to a UTF-8 JSON string. + * @return an {@link Supplier} containing the object converted to a UTF-8 JSON string. */ - public static Supplier asJson(Object obj) { - StringBuilder builder = new StringBuilder(); - try (JsonOutput out = JSON.newOutput(builder)) { + public static Supplier asJson(Object obj) { + ByteArrayOutputStream output = new ByteArrayOutputStream(); + try (JsonOutput out = JSON.newOutput(new OutputStreamWriter(output, UTF_8))) { out.writeClassName(false); out.write(obj); } - return utf8String(builder); + return bytes(output.toByteArray()); } public static T fromJson(HttpMessage message, Type typeOfT) { @@ -134,13 +175,6 @@ public static T fromJson(HttpMessage message, Type typeOfT) { } } - public static Supplier memoize(Supplier delegate) { - if (delegate instanceof MemoizedSupplier) { - return delegate; - } - return new MemoizedSupplier(delegate); - } - public static String string(File input) throws IOException { try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); InputStream isr = Files.newInputStream(input.toPath())) { @@ -152,43 +186,4 @@ public static String string(File input) throws IOException { return Base64.getEncoder().encodeToString(bos.toByteArray()); } } - - private static final class MemoizedSupplier implements Supplier { - - private volatile boolean initialized; - private volatile FileBackedOutputStream fos; - private final Supplier delegate; - - private MemoizedSupplier(Supplier delegate) { - this.delegate = delegate; - } - - @Override - public InputStream get() { - if (!initialized) { - synchronized (this) { - if (!initialized) { - try (InputStream is = delegate.get()) { - this.fos = new FileBackedOutputStream(3 * 1024 * 1024, true); - is.transferTo(fos); - initialized = true; - } catch (IOException e) { - throw new UncheckedIOException(e); - } finally { - try { - this.fos.close(); - } catch (IOException ignore) { - } - } - } - } - } - - try { - return Require.state("Source", fos.asByteSource()).nonNull().openBufferedStream(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - } } diff --git a/java/src/org/openqa/selenium/remote/http/DumpHttpExchangeFilter.java b/java/src/org/openqa/selenium/remote/http/DumpHttpExchangeFilter.java index 3338e7e4830a1..c4b2c5f8c890e 100644 --- a/java/src/org/openqa/selenium/remote/http/DumpHttpExchangeFilter.java +++ b/java/src/org/openqa/selenium/remote/http/DumpHttpExchangeFilter.java @@ -17,8 +17,6 @@ package org.openqa.selenium.remote.http; -import java.io.InputStream; -import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; import org.openqa.selenium.internal.Require; @@ -59,11 +57,6 @@ private void expandHeadersAndContent(StringBuilder builder, HttpMessage messa /** visible for testing only */ String requestLogMessage(HttpRequest req) { - // There's no requirement that requests or responses can be read more than once. Protect - // ourselves. - Supplier memoized = Contents.memoize(req.getContent()); - req.setContent(memoized); - StringBuilder reqInfo = new StringBuilder(); reqInfo.append("HTTP Request: ").append(req).append("\n"); expandHeadersAndContent(reqInfo, req); @@ -72,9 +65,6 @@ String requestLogMessage(HttpRequest req) { /** visible for testing only */ String responseLogMessage(HttpResponse res) { - Supplier resContents = Contents.memoize(res.getContent()); - res.setContent(resContents); - StringBuilder resInfo = new StringBuilder("HTTP Response: "); resInfo.append("Status code: ").append(res.getStatus()).append("\n"); expandHeadersAndContent(resInfo, res); diff --git a/java/src/org/openqa/selenium/remote/http/HttpMessage.java b/java/src/org/openqa/selenium/remote/http/HttpMessage.java index a56f599f04591..d8ec4129d4da7 100644 --- a/java/src/org/openqa/selenium/remote/http/HttpMessage.java +++ b/java/src/org/openqa/selenium/remote/http/HttpMessage.java @@ -19,7 +19,9 @@ import static java.nio.charset.StandardCharsets.UTF_8; +import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; import java.nio.charset.Charset; import java.util.ArrayList; import java.util.Arrays; @@ -38,7 +40,7 @@ abstract class HttpMessage> { private final Map> headers = new HashMap<>(); private final Map attributes = new HashMap<>(); - private Supplier content = Contents.empty(); + private Contents.Supplier content = Contents.empty(); /** * Retrieves a user-defined attribute of this message. Attributes are stored as simple key-value @@ -167,12 +169,21 @@ public Charset getContentEncoding() { return charset; } + @Deprecated public M setContent(Supplier supplier) { + try { + return setContent(Contents.bytes(supplier.get().readAllBytes())); + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + } + + public M setContent(Contents.Supplier supplier) { this.content = Require.nonNull("Supplier", supplier); return self(); } - public Supplier getContent() { + public Contents.Supplier getContent() { return content; } 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 ad44af7703f0c..ccf41787ee14c 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpMessages.java @@ -19,7 +19,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; -import java.io.ByteArrayInputStream; import java.net.URI; import java.net.URLEncoder; import java.net.http.HttpRequest.BodyPublisher; @@ -112,28 +111,17 @@ public java.net.http.HttpRequest createRequest(HttpRequest req, HttpMethod metho /** * Some drivers do not support chunked transport, we ensure the http client is not using chunked - * transport. This is done by using a BodyPublisher with a known size, in best case without - * wasting memory by buffering the request. + * transport. This is done by using a BodyPublisher with a known size. * * @return a BodyPublisher with a known size */ private BodyPublisher notChunkingBodyPublisher(HttpRequest req) { - String length = req.getHeader("content-length"); - - if (length == null) { - // read the data into a byte array to know the length - byte[] bytes = Contents.bytes(req.getContent()); - if (bytes.length == 0) { - // Looks like we were given a request with no payload. - return BodyPublishers.noBody(); - } - return BodyPublishers.ofByteArray(bytes); - } + Contents.Supplier content = req.getContent(); // we know the length of the request and use it - BodyPublisher chunking = BodyPublishers.ofInputStream(req.getContent()); + BodyPublisher chunking = BodyPublishers.ofInputStream(content); - return BodyPublishers.fromPublisher(chunking, Long.parseLong(length)); + return BodyPublishers.fromPublisher(chunking, content.length()); } public URI getRawUri(HttpRequest req) { @@ -171,7 +159,7 @@ public HttpResponse createResponse(java.net.http.HttpResponse response) .forEach(value -> res.addHeader(name, value))); byte[] responseBody = response.body(); if (responseBody != null) { - res.setContent(() -> new ByteArrayInputStream(responseBody)); + res.setContent(Contents.bytes(responseBody)); } return res; diff --git a/java/test/org/openqa/selenium/grid/node/NodeTest.java b/java/test/org/openqa/selenium/grid/node/NodeTest.java index 3626a3df73ba8..9f6d4c50b404a 100644 --- a/java/test/org/openqa/selenium/grid/node/NodeTest.java +++ b/java/test/org/openqa/selenium/grid/node/NodeTest.java @@ -31,7 +31,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; @@ -525,7 +524,7 @@ void canUploadAFile() throws IOException { String hello = "Hello, world!"; String zip = Zip.zip(createTmpFile(hello)); String payload = new Json().toJson(Collections.singletonMap("file", zip)); - req.setContent(() -> new ByteArrayInputStream(payload.getBytes())); + req.setContent(Contents.bytes(payload.getBytes())); node.execute(req); File baseDir = getTemporaryFilesystemBaseDir(local.getUploadsFilesystem(session.getId())); @@ -553,7 +552,7 @@ void canDownloadAFile() throws IOException { String zip = simulateFileDownload(session.getId(), hello); String payload = new Json().toJson(Collections.singletonMap("name", zip)); - req.setContent(() -> new ByteArrayInputStream(payload.getBytes())); + req.setContent(Contents.bytes(payload.getBytes())); HttpResponse rsp = node.execute(req); Map raw = new Json().toType(string(rsp), Json.MAP_TYPE); try { @@ -592,7 +591,7 @@ void canDownloadMultipleFile() throws IOException { simulateFileDownload(session.getId(), "Goodbye, world!"); String payload = new Json().toJson(Collections.singletonMap("name", zip)); - req.setContent(() -> new ByteArrayInputStream(payload.getBytes())); + req.setContent(Contents.bytes(payload.getBytes())); HttpResponse rsp = node.execute(req); Map raw = new Json().toType(string(rsp), Json.MAP_TYPE); try { @@ -758,7 +757,7 @@ void canDownloadFileThrowsErrorMsgWhenWrongPayloadIsGiven() { HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId())); String payload = new Json().toJson(Collections.singletonMap("my-file", "README.md")); - req.setContent(() -> new ByteArrayInputStream(payload.getBytes())); + req.setContent(Contents.bytes(payload.getBytes())); String msg = "Please specify file to download in payload as {\"name\": \"fileToDownload\"}"; assertThatThrownBy(() -> node.execute(req)).hasMessageContaining(msg); @@ -780,7 +779,7 @@ void canDownloadFileThrowsErrorMsgWhenFileNotFound() { HttpRequest req = new HttpRequest(POST, String.format("/session/%s/se/files", session.getId())); String payload = new Json().toJson(Collections.singletonMap("name", "README.md")); - req.setContent(() -> new ByteArrayInputStream(payload.getBytes())); + req.setContent(Contents.bytes(payload.getBytes())); String msg = "Cannot find file [README.md] in directory"; assertThatThrownBy(() -> node.execute(req)).hasMessageContaining(msg); diff --git a/java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java b/java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java index 3bfafdf61776f..a23294af6d1a7 100644 --- a/java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java +++ b/java/test/org/openqa/selenium/grid/node/config/DriverServiceSessionFactoryTest.java @@ -47,6 +47,7 @@ import org.openqa.selenium.internal.Either; import org.openqa.selenium.remote.Dialect; import org.openqa.selenium.remote.http.ClientConfig; +import org.openqa.selenium.remote.http.Contents; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.http.HttpRequest; import org.openqa.selenium.remote.http.HttpResponse; @@ -135,7 +136,7 @@ void shouldNotInstantiateSessionIfRemoteEndReturnsInvalidResponse() throws IOExc .thenReturn( new HttpResponse() .setStatus(200) - .setContent(() -> new ByteArrayInputStream("Hello, world!".getBytes()))); + .setContent(Contents.bytes("Hello, world!".getBytes()))); when(clientFactory.createClient(any(URL.class))).thenReturn(httpClient); DriverServiceSessionFactory factory = factoryFor("chrome", builder); diff --git a/java/test/org/openqa/selenium/remote/NewSessionPayloadTest.java b/java/test/org/openqa/selenium/remote/NewSessionPayloadTest.java index 4cf0d61c7ca85..866dda8916f11 100644 --- a/java/test/org/openqa/selenium/remote/NewSessionPayloadTest.java +++ b/java/test/org/openqa/selenium/remote/NewSessionPayloadTest.java @@ -31,7 +31,7 @@ import com.google.common.collect.ImmutableMap; import java.io.IOException; -import java.io.StringReader; +import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -41,6 +41,7 @@ import org.openqa.selenium.ImmutableCapabilities; import org.openqa.selenium.Platform; import org.openqa.selenium.json.Json; +import org.openqa.selenium.remote.http.Contents; @Tag("UnitTests") class NewSessionPayloadTest { @@ -56,7 +57,8 @@ void shouldIndicateDownstreamW3cDialect() { } String json = new Json().toJson(caps); - try (NewSessionPayload payload = NewSessionPayload.create(new StringReader(json))) { + try (NewSessionPayload payload = + NewSessionPayload.create(Contents.string(json, StandardCharsets.UTF_8))) { assertEquals(singleton(Dialect.W3C), payload.getDownstreamDialects()); } } @@ -262,7 +264,8 @@ private List create(Map source) { } String json = new Json().toJson(source); - try (NewSessionPayload payload = NewSessionPayload.create(new StringReader(json))) { + try (NewSessionPayload payload = + NewSessionPayload.create(Contents.string(json, StandardCharsets.UTF_8))) { fromDisk = payload.stream().collect(toList()); } From 907b2197dada02fe80de04843799f50d3d129f54 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Tue, 26 Mar 2024 20:32:01 +0530 Subject: [PATCH 4/4] [java] Remove "se:bidi" (#13528) * [java] Remove "se:bidi" * Add comments * Format code * [java] Store "local" websocket url in a different namespace and use it * [java] Fix websocket url checks * [java] Keep /se/bidi intact --------- Co-authored-by: Diego Molina --- .../openqa/selenium/bidi/BiDiProvider.java | 10 ++----- .../grid/node/ProxyNodeWebsockets.java | 2 +- .../config/DriverServiceSessionFactory.java | 25 ----------------- .../selenium/grid/node/local/LocalNode.java | 28 ++++++++++++++----- 4 files changed, 24 insertions(+), 41 deletions(-) diff --git a/java/src/org/openqa/selenium/bidi/BiDiProvider.java b/java/src/org/openqa/selenium/bidi/BiDiProvider.java index c23ffa554bfcc..ae1ac58d7f901 100644 --- a/java/src/org/openqa/selenium/bidi/BiDiProvider.java +++ b/java/src/org/openqa/selenium/bidi/BiDiProvider.java @@ -56,14 +56,8 @@ public HasBiDi getImplementation(Capabilities caps, ExecuteMethod executeMethod) } private Optional getBiDiUrl(Capabilities caps) { - Object bidiCapability; - if (caps.asMap().containsKey("se:bidi")) { - // Session is created remotely - bidiCapability = caps.getCapability("se:bidi"); - } else { - bidiCapability = caps.getCapability("webSocketUrl"); - } - Optional webSocketUrl = Optional.ofNullable((String) bidiCapability); + Object biDiCapability = caps.getCapability("webSocketUrl"); + Optional webSocketUrl = Optional.ofNullable((String) biDiCapability); return webSocketUrl.map( uri -> { diff --git a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java index 82cc397772c22..1101dca55056a 100644 --- a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java +++ b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java @@ -171,7 +171,7 @@ private Optional> findBiDiEndpoint( Consumer sessionConsumer, SessionId sessionId) { try { - URI uri = new URI(String.valueOf(caps.getCapability("webSocketUrl"))); + URI uri = new URI(String.valueOf(caps.getCapability("se:gridWebSocketUrl"))); return Optional.of(uri) .map(bidi -> createWsEndPoint(bidi, downstream, sessionConsumer, sessionId)); } catch (URISyntaxException e) { diff --git a/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java b/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java index 58044d4752770..8648247f315c9 100644 --- a/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java +++ b/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java @@ -22,7 +22,6 @@ import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION; import java.net.URI; -import java.net.URISyntaxException; import java.net.URL; import java.time.Duration; import java.time.Instant; @@ -186,7 +185,6 @@ public Either apply(CreateSessionRequest sess } caps = readDevToolsEndpointAndVersion(caps); - caps = readBiDiEndpoint(caps); caps = readVncEndpoint(capabilities, caps); span.addEvent("Driver service created session", attributeMap); @@ -281,29 +279,6 @@ public DevToolsInfo(URI cdpEndpoint, String version) { return caps; } - private Capabilities readBiDiEndpoint(Capabilities caps) { - - Optional webSocketUrl = - Optional.ofNullable((String) caps.getCapability("webSocketUrl")); - - Optional websocketUri = - webSocketUrl.map( - uri -> { - try { - return new URI(uri); - } catch (URISyntaxException e) { - LOG.warning(e.getMessage()); - } - return null; - }); - - if (websocketUri.isPresent()) { - return new PersistentCapabilities(caps).setCapability("se:bidi", websocketUri.get()); - } - - return caps; - } - private Capabilities readVncEndpoint(Capabilities requestedCaps, Capabilities returnedCaps) { String seVncEnabledCap = "se:vncEnabled"; String seNoVncPortCap = "se:noVncPort"; 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 0e5d283158536..79f687448cf91 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -805,20 +805,34 @@ private Session createExternalSession( } // Check if the user wants to use BiDi - boolean webSocketUrl = toUse.asMap().containsKey("webSocketUrl"); - // Add se:bidi if necessary to send the bidi url back - boolean bidiSupported = isSupportingBiDi || toUse.getCapability("se:bidi") != null; - if (bidiSupported && bidiEnabled && webSocketUrl) { + // This will be null if the user has not set the capability. + Object webSocketUrl = toUse.getCapability("webSocketUrl"); + + // In case of Firefox versions that do not support webSocketUrl, it returns the capability as it + // is i.e. boolean value. So need to check if it is a string. + // Check if the Node supports BiDi and if the client wants to use BiDi. + boolean bidiSupported = isSupportingBiDi && (webSocketUrl instanceof String); + if (bidiSupported && bidiEnabled) { + String biDiUrl = (String) other.getCapabilities().getCapability("webSocketUrl"); + URI uri = null; + try { + uri = new URI(biDiUrl); + } catch (URISyntaxException e) { + throw new IllegalArgumentException("Unable to create URI from " + uri); + } String bidiPath = String.format("/session/%s/se/bidi", other.getId()); - toUse = new PersistentCapabilities(toUse).setCapability("se:bidi", rewrite(bidiPath)); + toUse = + new PersistentCapabilities(toUse) + .setCapability("se:gridWebSocketUrl", uri) + .setCapability("webSocketUrl", rewrite(bidiPath)); } else { - // Remove any se:bidi* from the response, BiDi is not supported nor enabled + // Remove any "webSocketUrl" from the response, BiDi is not supported nor enabled MutableCapabilities bidiFiltered = new MutableCapabilities(); toUse .asMap() .forEach( (key, value) -> { - if (!key.startsWith("se:bidi")) { + if (!key.startsWith("webSocketUrl")) { bidiFiltered.setCapability(key, value); } });