From 894234a212e12557fc1a3c0eff0bd7305579d337 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sun, 18 Jan 2026 00:28:42 +0200 Subject: [PATCH 1/4] [bugfix] close browser after RemoteWebDriverBiDiTest This test left 3 browsers open :( The reason is that 1. it stopped grid first, 2. and hoped that JupiterTestBase will close `localDriver`. 3. But when JupiterTestBase tried to close `localDriver`, it failed because its `remoteUrl` was not available anymore (the grid had been already stopped). --- .../grid/router/RemoteWebDriverBiDiTest.java | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java b/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java index 2c7703684067f..fe507998dfbab 100644 --- a/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java +++ b/java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java @@ -17,14 +17,18 @@ package org.openqa.selenium.grid.router; +import static java.util.Objects.requireNonNull; +import static java.util.logging.Level.FINE; +import static java.util.logging.Level.INFO; import static org.assertj.core.api.Assertions.assertThat; import java.io.StringReader; -import java.util.Objects; +import java.net.URL; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.logging.Logger; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -49,12 +53,15 @@ import org.openqa.selenium.testing.drivers.Browser; class RemoteWebDriverBiDiTest extends JupiterTestBase { + private static final Logger LOG = Logger.getLogger(RemoteWebDriverBiDiTest.class.getName()); private Deployment deployment; + private URL remoteUrl; @BeforeEach void setup() { - Browser browser = Objects.requireNonNull(Browser.detect()); + Browser browser = requireNonNull(Browser.detect()); + LOG.log(FINE, () -> String.format("Starting grid server for %s...", browser)); deployment = DeploymentTypes.STANDALONE.start( @@ -65,23 +72,29 @@ void setup() { + "selenium-manager = false\n" + "driver-implementation = " + String.format("\"%s\"", browser.displayName())))); - } + remoteUrl = deployment.getServer().getUrl(); + LOG.log(INFO, () -> String.format("Started grid server for %s: %s", browser, remoteUrl)); - private void createDriver() { - Browser browser = Objects.requireNonNull(Browser.detect()); - localDriver = new RemoteWebDriver(deployment.getServer().getUrl(), browser.getCapabilities()); + localDriver = new RemoteWebDriver(remoteUrl, browser.getCapabilities()); localDriver = new Augmenter().augment(localDriver); } @AfterEach void tearDownDeployment() { - Safely.safelyCall(() -> deployment.tearDown()); + if (localDriver != null) { + localDriver.quit(); + } + + if (deployment != null) { + LOG.log(FINE, () -> String.format("Stopping grid server %s ...", remoteUrl)); + Safely.safelyCall(() -> deployment.tearDown()); + LOG.log(INFO, () -> String.format("Stopped grid server %s", remoteUrl)); + } } @Test @NoDriverBeforeTest void ensureBiDiSessionCreation() { - createDriver(); try (BiDi biDi = ((HasBiDi) localDriver).getBiDi()) { BiDiSessionStatus status = biDi.getBidiSessionStatus(); assertThat(status).isNotNull(); @@ -92,7 +105,6 @@ void ensureBiDiSessionCreation() { @Test @NoDriverBeforeTest void canListenToLogs() throws ExecutionException, InterruptedException, TimeoutException { - createDriver(); try (LogInspector logInspector = new LogInspector(localDriver)) { CompletableFuture future = new CompletableFuture<>(); logInspector.onConsoleEntry(future::complete); @@ -117,7 +129,6 @@ void canListenToLogs() throws ExecutionException, InterruptedException, TimeoutE @Test @NoDriverBeforeTest void canNavigateToUrl() { - createDriver(); BrowsingContext browsingContext = new BrowsingContext(localDriver, WindowType.TAB); String url = appServer.whereIs("/bidi/logEntryAdded.html"); From e880354cbd484638551cb3b618e6695fcfffafa5 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sun, 18 Jan 2026 00:36:13 +0200 Subject: [PATCH 2/4] add info to thrown exceptions, especially UnreachableBrowserException When throwing UnreachableBrowserException, it's important to mention URL of Selenium Grid (which is actually unavailable). Then user knows what's the real reason of "unreachable browser". --- .../openqa/selenium/bidi/BiDiProvider.java | 2 +- .../org/openqa/selenium/bidi/Connection.java | 18 ++--- .../netty/server/WebSocketMessageHandler.java | 9 ++- .../netty/server/WebSocketUpgradeHandler.java | 7 +- .../selenium/remote/RemoteWebDriver.java | 6 ++ .../selenium/remote/http/CloseMessage.java | 5 ++ .../remote/http/jdk/ConnectionException.java | 36 ++++++++++ .../remote/http/jdk/JdkHttpClient.java | 69 +++++++++++-------- .../remote/http/jdk/ProtocolException.java | 27 ++++++++ java/test/logging.properties | 6 ++ 10 files changed, 143 insertions(+), 42 deletions(-) create mode 100644 java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java create mode 100644 java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java create mode 100644 java/test/logging.properties diff --git a/java/src/org/openqa/selenium/bidi/BiDiProvider.java b/java/src/org/openqa/selenium/bidi/BiDiProvider.java index a43046a1e13e7..3dc25de87d781 100644 --- a/java/src/org/openqa/selenium/bidi/BiDiProvider.java +++ b/java/src/org/openqa/selenium/bidi/BiDiProvider.java @@ -93,7 +93,7 @@ private Optional getBiDiUrl(Capabilities caps) { try { return new URI(uri); } catch (URISyntaxException e) { - return null; + throw new BiDiException("Invalid BiDi URL: " + uri, e); } }); } diff --git a/java/src/org/openqa/selenium/bidi/Connection.java b/java/src/org/openqa/selenium/bidi/Connection.java index 9b667eee627dc..cc684d6af03e4 100644 --- a/java/src/org/openqa/selenium/bidi/Connection.java +++ b/java/src/org/openqa/selenium/bidi/Connection.java @@ -276,14 +276,18 @@ public void onText(CharSequence data) { try { handle(data); } catch (Exception e) { - throw new BiDiException("Unable to process: " + data, e); + throw new BiDiException("Unable to process BiDi response: " + data, e); } }); } @Override public void onClose(int code, String reason) { - LOG.fine("BiDi connection websocket closed"); + LOG.log( + Level.FINE, + () -> + String.format( + "BiDi connection websocket closed (code: %s, reason: \"%s\")", code, reason)); underlyingSocketClosed.set(true); } } @@ -303,7 +307,7 @@ private void handle(CharSequence data) { } else if (raw.get("method") instanceof String && raw.get("params") instanceof Map) { handleEventResponse(raw); } else { - LOG.warning(() -> "Unhandled type:" + data); + LOG.warning(() -> "Unhandled type BiDi response type: " + data); } } @@ -340,11 +344,9 @@ private void handleEventResponse(Map rawDataMap) { LOG.log( getDebugLogLevel(), () -> - "Method" - + rawDataMap.get("method") - + "called with" - + eventCallbacks.size() - + "callbacks available"); + String.format( + "Method %s called with %s callbacks available", + rawDataMap.get("method"), eventCallbacks.size())); Lock lock = callbacksLock.readLock(); // A waiting writer will block a reader to enter the lock, even if there are currently other // readers holding the lock. TryLock will bypass the waiting writers and acquire the read lock. diff --git a/java/src/org/openqa/selenium/netty/server/WebSocketMessageHandler.java b/java/src/org/openqa/selenium/netty/server/WebSocketMessageHandler.java index 709b8ce1c005e..7dd969dd7c25b 100644 --- a/java/src/org/openqa/selenium/netty/server/WebSocketMessageHandler.java +++ b/java/src/org/openqa/selenium/netty/server/WebSocketMessageHandler.java @@ -21,11 +21,14 @@ import io.netty.channel.SimpleChannelInboundHandler; import io.netty.util.AttributeKey; import java.util.function.Consumer; +import java.util.logging.Level; +import java.util.logging.Logger; import org.openqa.selenium.internal.Require; +import org.openqa.selenium.remote.http.CloseMessage; import org.openqa.selenium.remote.http.Message; class WebSocketMessageHandler extends SimpleChannelInboundHandler { - + private static final Logger LOG = Logger.getLogger(WebSocketMessageHandler.class.getName()); private final AttributeKey> key; public WebSocketMessageHandler(AttributeKey> key) { @@ -39,6 +42,10 @@ protected void channelRead0(ChannelHandlerContext ctx, Message msg) { } Consumer handler = ctx.channel().attr(key).get(); + if (handler == null && msg instanceof CloseMessage && ((CloseMessage) msg).code() == -1) { + LOG.log(Level.FINE, "Failed to handle websocket message \"{0}\" - handler is null", msg); + return; + } ctx.executor() .execute( diff --git a/java/src/org/openqa/selenium/netty/server/WebSocketUpgradeHandler.java b/java/src/org/openqa/selenium/netty/server/WebSocketUpgradeHandler.java index cd4c841de0404..d4917da5a3859 100644 --- a/java/src/org/openqa/selenium/netty/server/WebSocketUpgradeHandler.java +++ b/java/src/org/openqa/selenium/netty/server/WebSocketUpgradeHandler.java @@ -233,10 +233,11 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception { Consumer consumer = ctx.channel().attr(key).getAndSet(null); if (consumer != null) { + CloseMessage channelGotInactive = new CloseMessage(1001, "channel got inactive"); try { - consumer.accept(new CloseMessage(1001, "channel got inactive")); - } catch (Exception ex) { - LOG.log(Level.FINE, "failed to send the close message, code: 1001", ex); + consumer.accept(channelGotInactive); + } catch (RuntimeException ex) { + LOG.log(Level.FINE, ex, () -> "failed to send " + channelGotInactive); } } } diff --git a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java index a2c2f3a12a9e5..c7e9a3559322b 100644 --- a/java/src/org/openqa/selenium/remote/RemoteWebDriver.java +++ b/java/src/org/openqa/selenium/remote/RemoteWebDriver.java @@ -93,6 +93,7 @@ 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.http.jdk.ConnectionException; import org.openqa.selenium.remote.service.DriverCommandExecutor; import org.openqa.selenium.remote.tracing.TracedHttpClient; import org.openqa.selenium.remote.tracing.Tracer; @@ -628,6 +629,11 @@ protected Response execute(CommandPayload payload) { } } else if (e instanceof WebDriverException) { toThrow = (WebDriverException) e; + } else if (e instanceof ConnectionException) { + ConnectionException cause = (ConnectionException) e; + toThrow = + new UnreachableBrowserException( + "Error communicating with the remote browser at " + cause.uri(), cause); } else { toThrow = new UnreachableBrowserException( diff --git a/java/src/org/openqa/selenium/remote/http/CloseMessage.java b/java/src/org/openqa/selenium/remote/http/CloseMessage.java index 56487d9ed8d53..01c86c4719f64 100644 --- a/java/src/org/openqa/selenium/remote/http/CloseMessage.java +++ b/java/src/org/openqa/selenium/remote/http/CloseMessage.java @@ -38,4 +38,9 @@ public int code() { public String reason() { return reason; } + + @Override + public String toString() { + return String.format("%s{code=%d, reason=%s}", getClass().getSimpleName(), code, reason); + } } diff --git a/java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java b/java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java new file mode 100644 index 0000000000000..1337db402bf02 --- /dev/null +++ b/java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java @@ -0,0 +1,36 @@ +// 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.jdk; + +import java.io.UncheckedIOException; +import java.net.ConnectException; +import java.net.URI; +import org.openqa.selenium.remote.http.HttpMethod; + +public class ConnectionException extends UncheckedIOException { + private final URI uri; + + public ConnectionException(HttpMethod method, URI uri, ConnectException cause) { + super(String.format("Connection error (%s %s)", method, uri), cause); + this.uri = uri; + } + + public URI uri() { + return uri; + } +} 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 4f6a24df553b8..d2cbee72bb994 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -23,8 +23,8 @@ import java.io.InputStream; import java.io.UncheckedIOException; import java.net.Authenticator; +import java.net.ConnectException; import java.net.PasswordAuthentication; -import java.net.ProtocolException; import java.net.Proxy; import java.net.ProxySelector; import java.net.SocketAddress; @@ -443,18 +443,18 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { long start = System.currentTimeMillis(); BodyHandler responseBodyHandler = BodyHandlers.ofInputStream(); + URI rawUri = messages.getRawUri(req); + HttpMethod method = req.getMethod(); try { - HttpMethod method = req.getMethod(); - URI rawUri = messages.getRawUri(req); - // We need a custom handling of redirects to: // - increase the maximum number of retries to 100 // - avoid a downgrade of POST requests, see the javadoc of j.n.h.HttpClient.Redirect // - not run into https://bugs.openjdk.org/browse/JDK-8304701 for (int i = 0; i < 100; i++) { if (Thread.interrupted()) { - throw new InterruptedException("http request has been interrupted"); + throw new InterruptedException( + String.format("http request has been interrupted: %s %s", method, rawUri)); } java.net.http.HttpRequest request = messages.createRequest(req, method, rawUri); @@ -469,26 +469,8 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { case 302: case 307: case 308: - URI location = - rawUri.resolve( - response - .headers() - .firstValue("location") - .orElseThrow( - () -> - new ProtocolException( - "HTTP " - + response.statusCode() - + " without 'location' header set"))); - - if ("https".equalsIgnoreCase(rawUri.getScheme()) - && !"https".equalsIgnoreCase(location.getScheme())) { - throw new SecurityException("Downgrade from secure to insecure connection."); - } else if ("wss".equalsIgnoreCase(rawUri.getScheme()) - && !"wss".equalsIgnoreCase(location.getScheme())) { - throw new SecurityException("Downgrade from secure to insecure connection."); - } - + URI location = rawUri.resolve(getLocationHeader(response, method, rawUri)); + checkNotDowngrade(rawUri, location); rawUri = location; continue; default: @@ -496,14 +478,19 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { } } - throw new ProtocolException("Too many redirects: 101"); + throw new ProtocolException("Too many redirects: 101", method, rawUri); } catch (HttpTimeoutException e) { - throw new TimeoutException(e); + throw new TimeoutException( + String.format("Timeout when executing request (%s %s)", method, rawUri), e); + } catch (ConnectException e) { + throw new ConnectionException(method, rawUri, e); } catch (IOException e) { - throw new UncheckedIOException(e); + throw new UncheckedIOException( + String.format("Failed to execute request (%s %s)", method, rawUri), e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new WebDriverException(e.getMessage(), e); + throw new WebDriverException( + String.format("%s when executing request (%s %s)", e.getMessage(), method, rawUri), e); } finally { LOG.log( Level.FINE, @@ -512,6 +499,30 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { } } + private void checkNotDowngrade(URI from, URI to) { + if ("https".equalsIgnoreCase(from.getScheme()) && !"https".equalsIgnoreCase(to.getScheme()) + || "wss".equalsIgnoreCase(from.getScheme()) && !"wss".equalsIgnoreCase(to.getScheme())) { + throw new SecurityException( + String.format("Downgrade from secure to insecure connection (%s -> %s)", from, to)); + } + } + + private String getLocationHeader( + java.net.http.HttpResponse response, HttpMethod method, URI uri) + throws ProtocolException { + return response + .headers() + .firstValue("location") + .orElseThrow( + () -> { + String message = + String.format( + "HTTP response with status %s but without \"location\" header", + response.statusCode()); + return new ProtocolException(message, method, uri); + }); + } + @Override public CompletableFuture> sendAsyncNative( java.net.http.HttpRequest request, java.net.http.HttpResponse.BodyHandler handler) { diff --git a/java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java b/java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java new file mode 100644 index 0000000000000..b3458dc5b1b25 --- /dev/null +++ b/java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java @@ -0,0 +1,27 @@ +// 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.jdk; + +import java.net.URI; +import org.openqa.selenium.remote.http.HttpMethod; + +class ProtocolException extends RuntimeException { + public ProtocolException(String message, HttpMethod method, URI uri) { + super(String.format("%s (%s %s)", message, method, uri)); + } +} diff --git a/java/test/logging.properties b/java/test/logging.properties new file mode 100644 index 0000000000000..fe7ba8aee5c4e --- /dev/null +++ b/java/test/logging.properties @@ -0,0 +1,6 @@ +handlers=java.util.logging.ConsoleHandler +java.util.logging.ConsoleHandler.level = FINE +.level=INFO + +#Uncomment to enable DEBUG logs for some specific package: +#org.openqa.selenium.bidi.level=FINE From 3dee8fceaa6c3a0209cbc2cb8f565b6b974db8e8 Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Sun, 18 Jan 2026 14:26:03 +0200 Subject: [PATCH 3/4] mask user info in URL (just in case) --- .../remote/http/jdk/ConnectionException.java | 10 ++-- .../remote/http/jdk/JdkHttpClient.java | 53 +++++++++++++++---- .../remote/http/jdk/ProtocolException.java | 27 ---------- .../remote/http/jdk/JdkHttpClientTest.java | 23 ++++++++ 4 files changed, 71 insertions(+), 42 deletions(-) delete mode 100644 java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java diff --git a/java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java b/java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java index 1337db402bf02..767d41991b574 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java @@ -19,18 +19,16 @@ import java.io.UncheckedIOException; import java.net.ConnectException; -import java.net.URI; -import org.openqa.selenium.remote.http.HttpMethod; public class ConnectionException extends UncheckedIOException { - private final URI uri; + private final String uri; - public ConnectionException(HttpMethod method, URI uri, ConnectException cause) { - super(String.format("Connection error (%s %s)", method, uri), cause); + public ConnectionException(String message, String uri, ConnectException cause) { + super(message, cause); this.uri = uri; } - public URI uri() { + public String uri() { return uri; } } 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 d2cbee72bb994..90bfcbe350d3f 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -25,6 +25,7 @@ import java.net.Authenticator; import java.net.ConnectException; import java.net.PasswordAuthentication; +import java.net.ProtocolException; import java.net.Proxy; import java.net.ProxySelector; import java.net.SocketAddress; @@ -454,7 +455,7 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { for (int i = 0; i < 100; i++) { if (Thread.interrupted()) { throw new InterruptedException( - String.format("http request has been interrupted: %s %s", method, rawUri)); + String.format("http request has been interrupted: %s", describe(req))); } java.net.http.HttpRequest request = messages.createRequest(req, method, rawUri); @@ -478,19 +479,20 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { } } - throw new ProtocolException("Too many redirects: 101", method, rawUri); + throw new ProtocolException(String.format("Too many redirects: 101 (%s)", describe(req))); } catch (HttpTimeoutException e) { throw new TimeoutException( - String.format("Timeout when executing request (%s %s)", method, rawUri), e); + String.format("Timeout when executing request (%s)", describe(req)), e); } catch (ConnectException e) { - throw new ConnectionException(method, rawUri, e); + throw new ConnectionException( + String.format("Connection error (%s)", describe(req)), maskUrlCredentials(rawUri), e); } catch (IOException e) { throw new UncheckedIOException( - String.format("Failed to execute request (%s %s)", method, rawUri), e); + String.format("Failed to execute request (%s)", describe(req)), e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new WebDriverException( - String.format("%s when executing request (%s %s)", e.getMessage(), method, rawUri), e); + String.format("%s when executing request (%s)", e.getMessage(), describe(req)), e); } finally { LOG.log( Level.FINE, @@ -507,6 +509,39 @@ private void checkNotDowngrade(URI from, URI to) { } } + private String describe(HttpRequest req) { + String uri = maskUrlCredentials(messages.getRawUri(req)); + HttpMethod method = req.getMethod(); + return String.format("%s %s", method, uri); + } + + static String maskUrlCredentials(String uri) { + try { + return maskUrlCredentials(new URI(uri)); + } catch (URISyntaxException invalidUri) { + return uri; + } + } + + private static String maskUrlCredentials(URI u) { + if (u.getUserInfo() == null) { + return u.toString(); + } + try { + return new URI( + u.getScheme(), + "***", + u.getHost(), + u.getPort(), + u.getPath(), + u.getQuery(), + u.getFragment()) + .toString(); + } catch (URISyntaxException e) { + return u.toString(); + } + } + private String getLocationHeader( java.net.http.HttpResponse response, HttpMethod method, URI uri) throws ProtocolException { @@ -517,9 +552,9 @@ private String getLocationHeader( () -> { String message = String.format( - "HTTP response with status %s but without \"location\" header", - response.statusCode()); - return new ProtocolException(message, method, uri); + "HTTP response with status %d but without \"location\" header (%s %s)", + response.statusCode(), method, maskUrlCredentials(uri)); + return new ProtocolException(message); }); } diff --git a/java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java b/java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java deleted file mode 100644 index b3458dc5b1b25..0000000000000 --- a/java/src/org/openqa/selenium/remote/http/jdk/ProtocolException.java +++ /dev/null @@ -1,27 +0,0 @@ -// 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.jdk; - -import java.net.URI; -import org.openqa.selenium.remote.http.HttpMethod; - -class ProtocolException extends RuntimeException { - public ProtocolException(String message, HttpMethod method, URI uri) { - super(String.format("%s (%s %s)", message, method, uri)); - } -} diff --git a/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java b/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java index f6b5489a1910f..9c85840c167f3 100644 --- a/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java +++ b/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java @@ -17,6 +17,10 @@ package org.openqa.selenium.remote.http.jdk; +import static org.assertj.core.api.Assertions.assertThat; +import static org.openqa.selenium.remote.http.jdk.JdkHttpClient.maskUrlCredentials; + +import org.junit.jupiter.api.Test; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.internal.HttpClientTestBase; @@ -26,4 +30,23 @@ class JdkHttpClientTest extends HttpClientTestBase { protected HttpClient.Factory createFactory() { return new JdkHttpClient.Factory(); } + + @Test + void maskUrlCredentials_hidesUserInfoInUrl() { + assertThat(maskUrlCredentials("http://user:pass@my.grid.com:4444/wd/hub")) + .isEqualTo("http://***@my.grid.com:4444/wd/hub"); + assertThat(maskUrlCredentials("https://vi:se@myshop.com/wd/hub?email=foo@bar.ee")) + .isEqualTo("https://***@myshop.com/wd/hub?email=foo@bar.ee"); + assertThat(maskUrlCredentials("https://0:@myshop.com/wd/hub")) + .isEqualTo("https://***@myshop.com/wd/hub"); + } + + @Test + void maskUrlCredentials_withoutUserInfo() { + assertThat(maskUrlCredentials("http://localhost:49112/wd/hub")) + .isEqualTo("http://localhost:49112/wd/hub"); + assertThat(maskUrlCredentials("https://localhost/wd/hub")) + .isEqualTo("https://localhost/wd/hub"); + assertThat(maskUrlCredentials("")).isEqualTo(""); + } } From 2eaa657a74f81ab82f1f85ff498eecd2d989d35f Mon Sep 17 00:00:00 2001 From: Andrei Solntsev Date: Tue, 20 Jan 2026 09:10:20 +0200 Subject: [PATCH 4/4] extract even smaller method for readability --- .../openqa/selenium/remote/http/jdk/JdkHttpClient.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 90bfcbe350d3f..a9120c69328bc 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -502,13 +502,17 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { } private void checkNotDowngrade(URI from, URI to) { - if ("https".equalsIgnoreCase(from.getScheme()) && !"https".equalsIgnoreCase(to.getScheme()) - || "wss".equalsIgnoreCase(from.getScheme()) && !"wss".equalsIgnoreCase(to.getScheme())) { + if (isDowngradeFrom("https", from, to) || isDowngradeFrom("wss", from, to)) { throw new SecurityException( String.format("Downgrade from secure to insecure connection (%s -> %s)", from, to)); } } + private boolean isDowngradeFrom(String protocol, URI from, URI to) { + return protocol.equalsIgnoreCase(from.getScheme()) + && !protocol.equalsIgnoreCase(to.getScheme()); + } + private String describe(HttpRequest req) { String uri = maskUrlCredentials(messages.getRawUri(req)); HttpMethod method = req.getMethod();