Skip to content

Move BiDi creation to RemoteWebDriver. Allow enabling BiDi in Safari options.#17753

Open
pujagani wants to merge 9 commits into
SeleniumHQ:trunkfrom
pujagani:add-transport-layer
Open

Move BiDi creation to RemoteWebDriver. Allow enabling BiDi in Safari options.#17753
pujagani wants to merge 9 commits into
SeleniumHQ:trunkfrom
pujagani:add-transport-layer

Conversation

@pujagani

@pujagani pujagani commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🔗 Related Issues

💥 What does this PR do?

It moves BiDi creation to the parent class RemoteWebDriver and removes BiDi creation from child classes like FirefoxDriver and ChromiumDriver. Also, add a method to enable BiDi for Safari.

🔧 Implementation Notes

This is the pattern that Java follows and will the aim that Selenium 5, we have BiDi commands as first class citizen on the driver class, this is aligned with that goal. Also, Augmenter logic is no longer required for end-users who want to use BiDi with Grid using the RemoteWebDriver.

RemoteWebDriver now creates and owns the BiDi connection directly in startSession(), so BiDiProvider no longer needs to inject HasBiDi via the Augmenter.

A related test using the BiDiProvider to test Grid config is now removed. Rationale: connectionLimitIsRespected relied on BiDiProvider to manually create multiple raw BiDi connections to a single session. RemoteWebDriver now owns exactly one BiDi connection per session internally, making that multi-connection scenario unreachable through the public API.

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s):
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

Pending, enabling tests for Safari BiDi.

🔄 Types of changes

  • Cleanup (formatting, renaming)
  • New feature (non-breaking change which adds functionality)

@selenium-ci selenium-ci added the C-java Java Bindings label Jul 7, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Centralize BiDi initialization in RemoteWebDriver; add SafariOptions enableBiDi

✨ Enhancement 🕐 20-40 Minutes

Grey Divider

AI Description

• Centralize WebDriver BiDi session creation/teardown in RemoteWebDriver.
• Remove per-browser BiDi initialization from ChromiumDriver and FirefoxDriver.
• Add SafariOptions.enableBiDi() to opt into experimental Safari BiDi support.
Diagram

graph TD
  SOpt["SafariOptions.enableBiDi()"] --> Caps[("Capabilities: webSocketUrl")] --> RWD["RemoteWebDriver.startSession()"] --> Dec{"webSocketUrl true?"} --> BiDi["BiDi session (WS)"]
  Chromium["ChromiumDriver"] --> RWD
  Firefox["FirefoxDriver"] --> RWD
  RWD --> Close["close()/quit()"] --> BiDi
  subgraph Legend
    direction LR
    _cls["Class"] ~~~ _dec{"Decision"} ~~~ _cap[("Capability/Config")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep BiDi initialization in each concrete driver
  • ➕ Driver-specific logic can handle browser quirks more directly
  • ➕ Avoids changing RemoteWebDriver session lifecycle behavior
  • ➖ Duplicates BiDi wiring across drivers and increases maintenance cost
  • ➖ Harder to make BiDi a first-class, consistent feature across drivers
2. Lazy-initialize BiDi on first access (e.g., in getBiDi/maybeGetBiDi)
  • ➕ Avoids websocket/client creation cost when BiDi isn’t used
  • ➕ Reduces session-start complexity and error surface
  • ➖ Failure modes occur later and are harder to diagnose
  • ➖ Requires careful synchronization/thread-safety and lifecycle handling
3. Introduce a BiDi factory/provider pluggable per driver
  • ➕ Centralizes most logic while allowing per-browser overrides
  • ➕ Easier to evolve Safari/Firefox/Chromium differences without forking RemoteWebDriver
  • ➖ More abstraction and API surface area than needed right now
  • ➖ Still requires a clear contract for when/how capabilities are validated

Recommendation: The chosen approach (centralizing BiDi creation/teardown in RemoteWebDriver gated by the webSocketUrl capability) is the best default for consistency and aligns with making BiDi a first-class driver feature. Consider lazy init only if session-start overhead becomes an issue; otherwise, keep creation early so capability/URL validation fails fast.

Files changed (4) +62 / -110

Enhancement (2) +61 / -5
RemoteWebDriver.javaImplement HasBiDi and centralize BiDi creation/teardown in session lifecycle +44/-5

Implement HasBiDi and centralize BiDi creation/teardown in session lifecycle

• RemoteWebDriver now implements HasBiDi, conditionally creates a BiDi session during startSession() when webSocketUrl was requested, and ensures BiDi is closed during close()/quit(). Adds URL validation and consistent BiDiException wrapping for malformed websocket URIs.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java

SafariOptions.javaAdd SafariOptions.enableBiDi() for experimental BiDi capability wiring +17/-0

Add SafariOptions.enableBiDi() for experimental BiDi capability wiring

• Introduces an enableBiDi() helper that warns about experimental support and sets both webSocketUrl and the Safari experimental websocket capability key. Adds the new option constant used to toggle Safari's experimental BiDi behavior.

java/src/org/openqa/selenium/safari/SafariOptions.java

Refactor (2) +1 / -105
ChromiumDriver.javaRemove ChromiumDriver-owned BiDi initialization and accessors +0/-47

Remove ChromiumDriver-owned BiDi initialization and accessors

• Deletes local BiDi URI parsing, websocket client setup, and maybeGetBiDi() override. ChromiumDriver now relies on RemoteWebDriver for BiDi lifecycle and keeps only DevTools/CDP-specific setup.

java/src/org/openqa/selenium/chromium/ChromiumDriver.java

FirefoxDriver.javaRemove FirefoxDriver-owned BiDi fields and getBiDi/maybeGetBiDi overrides +1/-58

Remove FirefoxDriver-owned BiDi fields and getBiDi/maybeGetBiDi overrides

• Eliminates BiDi URI/caching fields, BiDi construction, and Firefox-specific getBiDi() error handling. Capability storage is simplified to wrap super.getCapabilities(), with BiDi delegated to RemoteWebDriver.

java/src/org/openqa/selenium/firefox/FirefoxDriver.java

@qodo-code-review

qodo-code-review Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 18 rules

Grey Divider


Action required

1. HasBiDi misdetects availability ✗ Dismissed 🐞 Bug ≡ Correctness
Description
RemoteWebDriver now always implements HasBiDi, but only initializes a BiDi connection when
webSocketUrl=true was requested, so driver instanceof HasBiDi no longer implies BiDi is actually
available. In-repo BiDi modules (e.g., org.openqa.selenium.bidi.module.Network) rely on
instanceof HasBiDi as a guard and then call getBiDi(), which will now throw BiDiException at
runtime for RemoteWebDriver sessions where BiDi wasn’t established.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R110-114]

@Augmentable
public class RemoteWebDriver
    implements WebDriver,
+        HasBiDi,
        JavascriptExecutor,
Evidence
RemoteWebDriver now matches instanceof HasBiDi for all remote sessions, but it only populates its
internal biDi Optional when the requested capabilities include webSocketUrl=true.
HasBiDi#getBiDi() throws if maybeGetBiDi() is empty, and BiDi modules like Network rely on the
instanceof HasBiDi check before calling getBiDi(), which becomes an unreliable guard and leads
to runtime exceptions.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[110-121]
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[261-295]
java/src/org/openqa/selenium/bidi/HasBiDi.java[24-33]
java/src/org/openqa/selenium/bidi/module/Network.java[73-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RemoteWebDriver` now implements `HasBiDi` for all remote sessions, but BiDi is only created when the session is started with `webSocketUrl=true`. Code that uses `instanceof HasBiDi` as a proxy for BiDi availability (several BiDi helper/modules in this repo) will now incorrectly pass the guard for any `RemoteWebDriver` and then fail at runtime when calling `getBiDi()`.

### Issue Context
- `HasBiDi#getBiDi()` throws when `maybeGetBiDi()` is empty.
- Multiple BiDi modules currently do:
 - `if (!(driver instanceof HasBiDi)) throw ...;`
 - `((HasBiDi) driver).getBiDi();`
 This pattern becomes unreliable once `RemoteWebDriver` always implements `HasBiDi`.

### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[110-115]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[261-295]
- java/src/org/openqa/selenium/bidi/module/Network.java[73-83]
- java/src/org/openqa/selenium/bidi/HasBiDi.java[24-33]

### What to change
- Update BiDi modules/constructors to validate BiDi availability using `maybeGetBiDi().isPresent()` (or a capabilities-based check), not `instanceof HasBiDi`.
 - Example pattern:
   - `if (!(driver instanceof HasBiDi) || ((HasBiDi) driver).maybeGetBiDi().isEmpty()) { throw new IllegalArgumentException("BiDi not enabled; set webSocketUrl: true"); }`
   - then safely use `((HasBiDi) driver).getBiDi()`.
- Apply the same change across other BiDi modules that use the same guard pattern (BrowsingContext, Script, Storage, etc.).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. maybeGetBiDi() missing Javadoc ✗ Dismissed 📘 Rule violation ✧ Quality
Description
RemoteWebDriver.maybeGetBiDi() was added/overridden without a Javadoc block. This violates the
requirement that all changed public API methods include complete Javadoc.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R460-464]

+  @Deprecated(since = "4.46", forRemoval = true)
+  @Override
+  public Optional<BiDi> maybeGetBiDi() {
+    return biDi;
+  }
Evidence
PR Compliance ID 330201 requires a Javadoc block immediately above each changed public method. The
added maybeGetBiDi() method has no /** ... */ Javadoc preceding it.

Rule 330201: Require complete Javadoc on public API methods
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[460-464]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RemoteWebDriver.maybeGetBiDi()` is a public API method and was changed in this PR, but it has no Javadoc comment immediately above the method signature.

## Issue Context
The project compliance rules require Javadoc for all changed public API methods.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[460-464]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. enableBiDi() lacks test coverage ✗ Dismissed 📘 Rule violation ▣ Testability
Description
A new public API method SafariOptions.enableBiDi() was added but there is no corresponding unit
test asserting the new capabilities it sets. This risks regressions and violates the requirement
that new functionality includes automated tests.
Code

java/src/org/openqa/selenium/safari/SafariOptions.java[R95-106]

+  /**
+   * Enables the WebDriver BiDi protocol. BiDi support in Safari is experimental and requires Safari
+   * Technology Preview; see {@link #setUseTechnologyPreview(boolean)}.
+   *
+   * @return this {@link SafariOptions} instance for chaining
+   */
+  public SafariOptions enableBiDi() {
+    LOG.warning("Safari's WebDriver BiDi support is experimental and may not work as expected.");
+    setCapability("webSocketUrl", true);
+    setCapability(Option.EXPERIMENTAL_WEB_SOCKET_URL, true);
+    return this;
+  }
Evidence
PR Compliance ID 389273 requires tests for new functionality. The PR adds
SafariOptions.enableBiDi() but the existing SafariOptionsTest does not include any test that
calls enableBiDi() or asserts the added capabilities.

Rule 389273: Require tests for all new functionality and bug fixes
java/src/org/openqa/selenium/safari/SafariOptions.java[95-106]
java/test/org/openqa/selenium/safari/SafariOptionsTest.java[35-111]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SafariOptions.enableBiDi()` adds new user-visible behavior (setting `webSocketUrl` and `safari:experimentalWebSocketUrl`) but there is no unit test verifying those capabilities are present.

## Issue Context
There is an existing `SafariOptionsTest` suite, but it does not cover `enableBiDi()`.

## Fix Focus Areas
- java/src/org/openqa/selenium/safari/SafariOptions.java[95-106]
- java/test/org/openqa/selenium/safari/SafariOptionsTest.java[35-111]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. BiDi init aborts session ✓ Resolved 🐞 Bug ☼ Reliability
Description
RemoteWebDriver.startSession eagerly calls createBiDi() when the request has webSocketUrl=true,
but createBiDi throws BiDiException if the returned webSocketUrl capability is missing or not a
ws/wss String. This can fail driver construction even though the WebDriver session was created
successfully (e.g., servers that return boolean webSocketUrl or omit it when unsupported).
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R436-443]

+  private Optional<BiDi> createBiDi() {
+    Object rawUrl = this.capabilities.getCapability("webSocketUrl");
+    if (!(rawUrl instanceof String)
+        || (!((String) rawUrl).startsWith("ws://") && !((String) rawUrl).startsWith("wss://"))) {
+      throw new BiDiException(
+          "Check if this browser version supports BiDi and if the"
+              + " 'webSocketUrl: true' capability is set.");
+    }
Evidence
The PR calls BiDi creation during session start and throws if the returned capability is not a
ws/wss String; elsewhere in the repo, BiDi support is treated as optional and implementations are
known to return boolean/non-string values for webSocketUrl. This mismatch can convert a successful
session creation into a constructor-time failure.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[262-307]
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[436-458]
java/src/org/openqa/selenium/grid/node/local/LocalNode.java[1242-1275]
java/src/org/openqa/selenium/bidi/BiDiProvider.java[89-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RemoteWebDriver.startSession()` may throw after successfully creating a WebDriver session because `createBiDi()` throws when the returned `webSocketUrl` capability isn’t a valid `ws://`/`wss://` string.

### Issue Context
Some parts of the codebase already account for implementations returning non-string values for `webSocketUrl` (e.g., boolean), or removing the capability when BiDi isn’t supported. With this PR, requesting `webSocketUrl: true` can now turn those cases into a hard failure during driver construction.

### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[291-296]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[436-458]

### Suggested change
- Make `createBiDi()` **non-throwing** for missing/invalid `webSocketUrl` and return `Optional.empty()` (optionally log at WARNING/FINE).
- If you still want an explicit failure, move it to `getBiDi()` (or override `getBiDi()` in `RemoteWebDriver`) so session creation succeeds but BiDi access fails with a clear message when BiDi is requested but unavailable.
- Consider tracking a `bidiRequested` flag (based on the *requested* capabilities) to produce an accurate exception message when `getBiDi()` is called.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. Case-sensitive ws URL check ✓ Resolved 🐞 Bug ≡ Correctness
Description
RemoteWebDriver.createBiDi() validates webSocketUrl using a case-sensitive
startsWith("ws://"|"wss://"), so a valid WebSocket URI with an uppercase scheme (or leading
whitespace) will be rejected and BiDi will not be initialized (only a warning is logged). This can
cause BiDi to be unexpectedly unavailable even when the remote end returned a usable endpoint
string.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R436-440]

+    Object rawUrl = this.capabilities.getCapability("webSocketUrl");
+    if (!(rawUrl instanceof String)
+        || (!((String) rawUrl).startsWith("ws://") && !((String) rawUrl).startsWith("wss://"))) {
+      LOG.warning("BiDi was requested but the remote end did not return a valid webSocketUrl.");
+      return Optional.empty();
Evidence
The new BiDi initialization path rejects webSocketUrl unless it starts with lowercase ws:// or
wss://, which is a stricter constraint than other in-repo WebSocket handling that treats schemes
case-insensitively.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[435-441]
java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java[363-386]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RemoteWebDriver.createBiDi()` currently checks the WebSocket URL via a case-sensitive string prefix check (`startsWith("ws://")` / `startsWith("wss://")`). Since URI schemes are treated case-insensitively elsewhere in the codebase, this strict check can reject valid endpoints (e.g., `WS://...`) and prevent BiDi from being created.

### Issue Context
The code already constructs a `URI` and handles `URISyntaxException`. The scheme validation should be derived from the parsed `URI` (or at least made case-insensitive + whitespace-tolerant) rather than relying on `String.startsWith`.

### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[436-440]

Suggested implementation sketch:
- If `rawUrl` is a `String`, trim it.
- Parse it with `new URI(trimmed)`.
- Validate `uri.getScheme()` via `equalsIgnoreCase("ws") || equalsIgnoreCase("wss")`.
- If invalid, log + return `Optional.empty()` as today.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. BiDi creation gated by request ✗ Dismissed 📘 Rule violation ≡ Correctness
Description
RemoteWebDriver now only initializes the internal BiDi connection when the *requested*
capabilities include webSocketUrl=true, even if the remote end returns a valid webSocketUrl
string in the session’s returned capabilities. This can be a backward-incompatible user-visible
behavior change for callers that previously relied on BiDi being available whenever the remote
provided the URL.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R293-295]

+      if (Boolean.TRUE.equals(capabilities.getCapability("webSocketUrl"))) {
+        this.biDi = createBiDi();
+      }
Evidence
PR Compliance ID 389266 requires maintaining backward-compatible public API/behavior. The new
startSession logic only calls createBiDi() when the *input* capabilities contains
webSocketUrl=true, and createBiDi() reads the *returned* this.capabilities.webSocketUrl; this
combination can change previously observable behavior for callers who did not set the request
capability.

Rule 389266: Maintain backward-compatible public API and ABI
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[293-295]
java/src/org/openqa/selenium/remote/RemoteWebDriver.java[435-443]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RemoteWebDriver.startSession()` currently creates the BiDi connection only when the *requested* capabilities contain `webSocketUrl=true`. This can break existing users if a server/grid returns a `webSocketUrl` in the *returned* capabilities (string) but the client did not explicitly request it.

## Issue Context
The new logic sets `this.capabilities` from the remote response, and `createBiDi()` already validates `this.capabilities.getCapability("webSocketUrl")` as a `ws://`/`wss://` string, but the creation call is gated on the request-side capability check.

## Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[293-295]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[435-443]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Leaked BiDi HttpClient ✓ Resolved 🐞 Bug ☼ Reliability
Description
RemoteWebDriver.createBiDi() allocates a new HttpClient and then constructs a BiDi Connection, but
if websocket establishment throws (e.g., ConnectionFailedException), the newly-created HttpClient is
never closed. This can leak resources/threads on repeated failures and fails driver construction
after the WebDriver session was already created.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R447-451]

+      HttpClient.Factory clientFactory = HttpClient.Factory.createDefault();
+      ClientConfig wsConfig = this.clientConfig.baseUri(wsUri);
+      HttpClient wsClient = clientFactory.createClient(wsConfig);
+      Connection biDiConnection = new Connection(wsClient, wsUri.toString());
+      return Optional.of(new BiDi(biDiConnection, wsConfig.wsTimeout()));
Evidence
The new code creates wsClient and immediately constructs new Connection(wsClient, ...) without
any cleanup on runtime failures; the Connection constructor opens the websocket via
HttpClient.openSocket, and the JDK client throws ConnectionFailedException on failures, meaning
the wsClient can be leaked if the constructor throws before a Connection exists to close it.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[435-459]
java/src/org/openqa/selenium/bidi/Connection.java[81-87]
java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java[158-170]
java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java[250-272]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RemoteWebDriver.createBiDi()` creates a new `HttpClient` (`wsClient`) and then opens a BiDi websocket via `new Connection(wsClient, ...)`. If websocket opening fails (runtime `ConnectionFailedException` from `HttpClient.openSocket`), `createBiDi()` currently lets the exception propagate without closing `wsClient`, leaking resources.

### Issue Context
- `org.openqa.selenium.bidi.Connection` opens the websocket in its constructor.
- `JdkHttpClient.openSocket` throws `ConnectionFailedException` on connection/setup failures.

### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[435-459]

### Implementation sketch
- Wrap the `Connection`/`BiDi` construction in a `try { ... } catch (RuntimeException e) { wsClient.close(); throw e; }`.
- Optionally, if the desired behavior is “BiDi requested but can’t connect => continue without BiDi”, catch `ConnectionFailedException`, log at WARNING, close `wsClient`, and return `Optional.empty()` instead of throwing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (2)
8. enableBiDi() logs warning 📘 Rule violation ◔ Observability
Description
SafariOptions.enableBiDi() logs a routine configuration choice using LOG.warning, even though it
is not an operational problem condition. This can create unnecessary noise and may trigger
alerts/log filters that treat warnings as actionable incidents.
Code

java/src/org/openqa/selenium/safari/SafariOptions.java[R101-103]

+  public SafariOptions enableBiDi() {
+    LOG.warning("Safari's WebDriver BiDi support is experimental and may not work as expected.");
+    setCapability("webSocketUrl", true);
Evidence
PR Compliance ID 330199 requires routine/expected events to be logged at info and reserves
warning for actionable/problem conditions. The newly added enableBiDi() method logs an expected
configuration action with LOG.warning(...).

Rule 330199: Use appropriate log levels for message intent
java/src/org/openqa/selenium/safari/SafariOptions.java[95-106]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SafariOptions.enableBiDi()` uses `LOG.warning(...)` for an expected, user-initiated configuration action.

## Issue Context
The compliance rule requires using `warning` for conditions that may require intervention, and `info` for normal operational/configuration events.

## Fix Focus Areas
- java/src/org/openqa/selenium/safari/SafariOptions.java[101-104]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Logs BiDi URL value ✓ Resolved 🐞 Bug ⛨ Security
Description
RemoteWebDriver.createBiDi() logs the full returned webSocketUrl when URI parsing fails, which can
include session identifiers and a potentially connectable BiDi endpoint in application logs. This is
sensitive on Grid where webSocketUrl commonly embeds /session/<id>/... in the URL path.
Code

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[R451-454]

+      LOG.warning(
+          "BiDi was requested but the remote end returned an invalid webSocketUrl: "
+              + webSocketUrl);
+      return Optional.empty();
Evidence
The new code logs the complete webSocketUrl string on parse failure. Elsewhere in the repo, Grid
tests and UI fixtures show webSocketUrl values include /session/<id>/..., so logging the full
value exposes session identifiers/endpoints.

java/src/org/openqa/selenium/remote/RemoteWebDriver.java[451-454]
java/test/org/openqa/selenium/grid/router/TunnelWebsocketTest.java[527-535]
javascript/grid-ui/src/tests/components/RunningSessions.test.tsx[101-120]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`RemoteWebDriver#createBiDi()` logs the full `webSocketUrl` string on `URISyntaxException`. Since the URL commonly contains a session id and endpoint path, emitting it at WARNING level can leak sensitive session-specific data into logs.

### Issue Context
This warning is triggered when BiDi was requested and the remote end returns a malformed `webSocketUrl` capability. Grid commonly returns URLs like `ws://<host>:<port>/session/<id>/bidi`.

### Fix Focus Areas
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[451-454]

### Suggested fix
- Remove the `+ webSocketUrl` concatenation from the WARNING log, or log a redacted/sanitized version (e.g., scheme+host+port only; strip path/query), and optionally log the exception via `LOG.log(Level.WARNING, ..., e)` without printing the raw URL.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
Comment thread java/src/org/openqa/selenium/safari/SafariOptions.java
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 5627182

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 10347f6

Comment thread java/src/org/openqa/selenium/safari/SafariOptions.java
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 26bb8e6

Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit ebae01d

Comment thread java/src/org/openqa/selenium/remote/RemoteWebDriver.java
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 0a20547

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 69d6161

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-java Java Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants