From e5c0d17e07c9b18d01f1657cda81d54bfdb30700 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Tue, 30 Jan 2024 19:02:24 +0530 Subject: [PATCH 1/6] [java] Remove "se:bidi" --- .../openqa/selenium/bidi/BiDiProvider.java | 10 ++------ .../grid/node/ProxyNodeWebsockets.java | 21 +++++++++++++--- .../config/DriverServiceSessionFactory.java | 24 ------------------- .../selenium/grid/node/local/LocalNode.java | 20 +++++++++++----- 4 files changed, 34 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..455e2f1e8aa32 100644 --- a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java +++ b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java @@ -49,7 +49,7 @@ public class ProxyNodeWebsockets implements BiFunction, Optional>> { private static final UrlTemplate CDP_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/cdp"); - private static final UrlTemplate BIDI_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/bidi"); + private static final UrlTemplate BIDI_TEMPLATE = new UrlTemplate("/session/{sessionId}"); private static final UrlTemplate FWD_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/fwd"); private static final UrlTemplate VNC_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/vnc"); private static final Logger LOG = Logger.getLogger(ProxyNodeWebsockets.class.getName()); @@ -171,8 +171,23 @@ private Optional> findBiDiEndpoint( Consumer sessionConsumer, SessionId sessionId) { try { - URI uri = new URI(String.valueOf(caps.getCapability("webSocketUrl"))); - return Optional.of(uri) + // Modified websocket uri that is used by the client to point to the Grid + // This is of the format "ws:///session/{session-id}" + URI externalWebsocketUri = new URI(String.valueOf(caps.getCapability("webSocketUrl"))); + + // Constructed websocket uri that points to BiDi running on the browser + // This is of the format "ws://localhost:{biDiPort}/session/{session-id}" + URI websocketUri = + new URI( + externalWebsocketUri.getScheme(), + externalWebsocketUri.getUserInfo(), + "localhost", + (Integer) caps.getCapability("bidi:port"), + externalWebsocketUri.getRawPath(), + externalWebsocketUri.getQuery(), + externalWebsocketUri.getFragment()); + + return Optional.of(websocketUri) .map(bidi -> createWsEndPoint(bidi, downstream, sessionConsumer, sessionId)); } catch (URISyntaxException e) { LOG.warning("Unable to create URI from: " + caps.getCapability("webSocketUrl")); 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..cd7a01f082998 100644 --- a/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java +++ b/java/src/org/openqa/selenium/grid/node/config/DriverServiceSessionFactory.java @@ -186,7 +186,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 +280,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..9f008aaa07bcb 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -806,19 +806,27 @@ 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; + boolean bidiSupported = isSupportingBiDi || toUse.getCapability("webSocketUrl") != null; if (bidiSupported && bidiEnabled && webSocketUrl) { - String bidiPath = String.format("/session/%s/se/bidi", other.getId()); - toUse = new PersistentCapabilities(toUse).setCapability("se:bidi", rewrite(bidiPath)); + 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); + } + toUse = + new PersistentCapabilities(toUse) + .setCapability("bidi:port", uri.getPort()) + .setCapability("webSocketUrl", rewrite(uri.getPath())); } 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); } }); From 614428f5a44b5c99b0b9652997b20332b0e5c290 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Wed, 31 Jan 2024 11:58:12 +0530 Subject: [PATCH 2/6] Add comments --- .../org/openqa/selenium/grid/node/ProxyNodeWebsockets.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java index 455e2f1e8aa32..1e5adb316b870 100644 --- a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java +++ b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java @@ -177,11 +177,13 @@ private Optional> findBiDiEndpoint( // Constructed websocket uri that points to BiDi running on the browser // This is of the format "ws://localhost:{biDiPort}/session/{session-id}" + // The url can be recreated but only information that we need from original "webSocketUrl" is + // the port URI websocketUri = new URI( externalWebsocketUri.getScheme(), externalWebsocketUri.getUserInfo(), - "localhost", + "localhost", // Hardcoded the same way it is done in DriverService creation (Integer) caps.getCapability("bidi:port"), externalWebsocketUri.getRawPath(), externalWebsocketUri.getQuery(), From 7ad5e8d8722987c7fefb0e20c4e11d9c347020c6 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Wed, 31 Jan 2024 12:03:54 +0530 Subject: [PATCH 3/6] Format code --- .../selenium/grid/node/config/DriverServiceSessionFactory.java | 1 - 1 file changed, 1 deletion(-) 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 cd7a01f082998..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; From 0169f126b225df74f2cd176d1cebc8ae4f92f113 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Tue, 6 Feb 2024 15:29:31 +0530 Subject: [PATCH 4/6] [java] Store "local" websocket url in a different namespace and use it --- .../grid/node/ProxyNodeWebsockets.java | 21 ++----------------- .../selenium/grid/node/local/LocalNode.java | 2 +- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java index 1e5adb316b870..7dbbba340671c 100644 --- a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java +++ b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java @@ -171,25 +171,8 @@ private Optional> findBiDiEndpoint( Consumer sessionConsumer, SessionId sessionId) { try { - // Modified websocket uri that is used by the client to point to the Grid - // This is of the format "ws:///session/{session-id}" - URI externalWebsocketUri = new URI(String.valueOf(caps.getCapability("webSocketUrl"))); - - // Constructed websocket uri that points to BiDi running on the browser - // This is of the format "ws://localhost:{biDiPort}/session/{session-id}" - // The url can be recreated but only information that we need from original "webSocketUrl" is - // the port - URI websocketUri = - new URI( - externalWebsocketUri.getScheme(), - externalWebsocketUri.getUserInfo(), - "localhost", // Hardcoded the same way it is done in DriverService creation - (Integer) caps.getCapability("bidi:port"), - externalWebsocketUri.getRawPath(), - externalWebsocketUri.getQuery(), - externalWebsocketUri.getFragment()); - - return Optional.of(websocketUri) + URI uri = new URI(String.valueOf(caps.getCapability("se:gridWebSocketUrl"))); + return Optional.of(uri) .map(bidi -> createWsEndPoint(bidi, downstream, sessionConsumer, sessionId)); } catch (URISyntaxException e) { LOG.warning("Unable to create URI from: " + caps.getCapability("webSocketUrl")); 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 9f008aaa07bcb..92a71f7833334 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -817,7 +817,7 @@ private Session createExternalSession( } toUse = new PersistentCapabilities(toUse) - .setCapability("bidi:port", uri.getPort()) + .setCapability("se:gridWebSocketUrl", uri) .setCapability("webSocketUrl", rewrite(uri.getPath())); } else { // Remove any "webSocketUrl" from the response, BiDi is not supported nor enabled From b3b45da54f0f40ef139ee4b7f2dd6dee2b9688bf Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Wed, 7 Feb 2024 13:43:41 +0530 Subject: [PATCH 5/6] [java] Fix websocket url checks --- .../openqa/selenium/grid/node/local/LocalNode.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 92a71f7833334..05a2f09d28650 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -805,9 +805,14 @@ private Session createExternalSession( } // Check if the user wants to use BiDi - boolean webSocketUrl = toUse.asMap().containsKey("webSocketUrl"); - boolean bidiSupported = isSupportingBiDi || toUse.getCapability("webSocketUrl") != 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 { From f650316ccb271fbf93232c77efd7e51e6710f204 Mon Sep 17 00:00:00 2001 From: Puja Jagani Date: Wed, 7 Feb 2024 15:54:05 +0530 Subject: [PATCH 6/6] [java] Keep /se/bidi intact --- .../src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java | 2 +- java/src/org/openqa/selenium/grid/node/local/LocalNode.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java index 7dbbba340671c..1101dca55056a 100644 --- a/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java +++ b/java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java @@ -49,7 +49,7 @@ public class ProxyNodeWebsockets implements BiFunction, Optional>> { private static final UrlTemplate CDP_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/cdp"); - private static final UrlTemplate BIDI_TEMPLATE = new UrlTemplate("/session/{sessionId}"); + private static final UrlTemplate BIDI_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/bidi"); private static final UrlTemplate FWD_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/fwd"); private static final UrlTemplate VNC_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/vnc"); private static final Logger LOG = Logger.getLogger(ProxyNodeWebsockets.class.getName()); 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 05a2f09d28650..79f687448cf91 100644 --- a/java/src/org/openqa/selenium/grid/node/local/LocalNode.java +++ b/java/src/org/openqa/selenium/grid/node/local/LocalNode.java @@ -820,10 +820,11 @@ private Session createExternalSession( } 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:gridWebSocketUrl", uri) - .setCapability("webSocketUrl", rewrite(uri.getPath())); + .setCapability("webSocketUrl", rewrite(bidiPath)); } else { // Remove any "webSocketUrl" from the response, BiDi is not supported nor enabled MutableCapabilities bidiFiltered = new MutableCapabilities();