From de4ab854968e67d2d43e8a4081c1c61112ea97f3 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 18:29:40 +0700 Subject: [PATCH 1/7] remove authentication from url --- .../selenium/remote/http/jdk/JdkHttpClient.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 6fe1ef7c9a47c..b433d9f9f5930 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -108,6 +108,8 @@ public class JdkHttpClient implements HttpClient { Credentials credentials = config.credentials(); String info = config.baseUri().getUserInfo(); + URI baseUri = config.baseUri(); + if (info != null && !info.trim().isEmpty()) { String[] parts = info.split(":", 2); String username = parts[0]; @@ -121,6 +123,20 @@ protected PasswordAuthentication getPasswordAuthentication() { } }; builder = builder.authenticator(authenticator); + + // Remove credentials from URL + try { + config = ClientConfig.defaultConfig().baseUri(new URI( + config.baseUri().getScheme(), + null, + config.baseUri().getHost(), + config.baseUri().getPort(), + config.baseUri().getPath(), + config.baseUri().getQuery(), + config.baseUri().getFragment())); + } catch (URISyntaxException e) { + LOG.log(Level.WARNING, "Could not strip credentials from URI", e); + } } else if (credentials != null) { if (!(credentials instanceof UsernameAndPassword)) { throw new IllegalArgumentException( From 815bc89b30e5aca100224d913366249b02ea7a71 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 18:32:49 +0700 Subject: [PATCH 2/7] added new tests --- .../remote/http/jdk/JdkHttpClient.java | 11 +++- .../remote/http/jdk/JdkHttpClientTest.java | 54 +++++++++++++++++++ 2 files changed, 63 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 b433d9f9f5930..27775744adbf5 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -80,10 +80,12 @@ public class JdkHttpClient implements HttpClient { private final ExecutorService executorService; private final Duration readTimeout; private final Duration connectTimeout; + private final ClientConfig config; JdkHttpClient(ClientConfig config) { Objects.requireNonNull(config, "Client config must be set"); - + + this.config = config; this.messages = new JdkHttpMessages(config); this.readTimeout = config.readTimeout(); this.connectTimeout = config.connectionTimeout(); @@ -338,7 +340,7 @@ public WebSocket send(Message message) { throw new WebDriverException(cause); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new WebDriverException(e.getMessage()); + throw new WebDriverException(e.getMessage(), e); } catch (java.util.concurrent.TimeoutException e) { throw new TimeoutException(e); } finally { @@ -522,6 +524,11 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { } } + // Package-private method for testing + URI getBaseUri() { + return config.baseUri(); + } + @Override public void close() { if (this.client == null) { 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..c0020c5d4c920 100644 --- a/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java +++ b/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java @@ -19,6 +19,13 @@ import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.internal.HttpClientTestBase; +import org.junit.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.isNull; + +import java.net.URI; +import java.net.URISyntaxException; class JdkHttpClientTest extends HttpClientTestBase { @@ -26,4 +33,51 @@ class JdkHttpClientTest extends HttpClientTestBase { protected HttpClient.Factory createFactory() { return new JdkHttpClient.Factory(); } + + @Test + void shouldStripCredentialsFromUrl() throws URISyntaxException { + URI originalUri = new URI("http://admin:password@localhost:4444/wd/hub"); + ClientConfig config = ClientConfig.defaultConfig().baseUri(originalUri); + + JdkHttpClient client = new JdkHttpClient(config); + + // Get the modified URI from the client's config + URI modifiedUri = client.getBaseUri(); + + assertThat(modifiedUri.getUserInfo()).isNull(); + assertThat(modifiedUri.getHost()).isEqualTo("localhost"); + assertThat(modifiedUri.getPort()).isEqualTo(4444); + assertThat(modifiedUri.getPath()).isEqualTo("/wd/hub"); + } + + @Test + void shouldHandleUrlWithoutCredentials() throws URISyntaxException { + URI originalUri = new URI("http://localhost:4444/wd/hub"); + ClientConfig config = ClientConfig.defaultConfig().baseUri(originalUri); + + JdkHttpClient client = new JdkHttpClient(config); + + URI modifiedUri = client.getBaseUri(); + + assertThat(modifiedUri).isEqualTo(originalUri); + } + + @Test + void shouldPreserveUrlComponentsExceptCredentials() throws URISyntaxException { + URI originalUri = new URI( + "https://admin:password@localhost:4444/wd/hub?debug=true#fragment"); + ClientConfig config = ClientConfig.defaultConfig().baseUri(originalUri); + + JdkHttpClient client = new JdkHttpClient(config); + + URI modifiedUri = client.getBaseUri(); + + assertThat(modifiedUri.getScheme()).isEqualTo("https"); + assertThat(modifiedUri.getUserInfo()).isNull(); + assertThat(modifiedUri.getHost()).isEqualTo("localhost"); + assertThat(modifiedUri.getPort()).isEqualTo(4444); + assertThat(modifiedUri.getPath()).isEqualTo("/wd/hub"); + assertThat(modifiedUri.getQuery()).isEqualTo("debug=true"); + assertThat(modifiedUri.getFragment()).isEqualTo("fragment"); + } } From 6e2d8a3ffd741add8ec2e69b455d397e6e06b9b5 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 18:45:38 +0700 Subject: [PATCH 3/7] applied format.sh --- .../remote/http/jdk/JdkHttpClient.java | 25 +++++++++------- .../remote/http/jdk/JdkHttpClientTest.java | 29 +++++++++---------- 2 files changed, 27 insertions(+), 27 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 27775744adbf5..1cb2f7ff112ba 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -84,7 +84,7 @@ public class JdkHttpClient implements HttpClient { JdkHttpClient(ClientConfig config) { Objects.requireNonNull(config, "Client config must be set"); - + this.config = config; this.messages = new JdkHttpMessages(config); this.readTimeout = config.readTimeout(); @@ -111,7 +111,7 @@ public class JdkHttpClient implements HttpClient { Credentials credentials = config.credentials(); String info = config.baseUri().getUserInfo(); URI baseUri = config.baseUri(); - + if (info != null && !info.trim().isEmpty()) { String[] parts = info.split(":", 2); String username = parts[0]; @@ -125,17 +125,20 @@ protected PasswordAuthentication getPasswordAuthentication() { } }; builder = builder.authenticator(authenticator); - + // Remove credentials from URL try { - config = ClientConfig.defaultConfig().baseUri(new URI( - config.baseUri().getScheme(), - null, - config.baseUri().getHost(), - config.baseUri().getPort(), - config.baseUri().getPath(), - config.baseUri().getQuery(), - config.baseUri().getFragment())); + config = + ClientConfig.defaultConfig() + .baseUri( + new URI( + config.baseUri().getScheme(), + null, + config.baseUri().getHost(), + config.baseUri().getPort(), + config.baseUri().getPath(), + config.baseUri().getQuery(), + config.baseUri().getFragment())); } catch (URISyntaxException e) { LOG.log(Level.WARNING, "Could not strip credentials from URI", e); } 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 c0020c5d4c920..ec5a0032a647b 100644 --- a/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java +++ b/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java @@ -17,15 +17,13 @@ package org.openqa.selenium.remote.http.jdk; -import org.openqa.selenium.remote.http.HttpClient; -import org.openqa.selenium.remote.internal.HttpClientTestBase; -import org.junit.Test; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.isNull; import java.net.URI; import java.net.URISyntaxException; +import org.junit.Test; +import org.openqa.selenium.remote.http.HttpClient; +import org.openqa.selenium.remote.internal.HttpClientTestBase; class JdkHttpClientTest extends HttpClientTestBase { @@ -38,12 +36,12 @@ protected HttpClient.Factory createFactory() { void shouldStripCredentialsFromUrl() throws URISyntaxException { URI originalUri = new URI("http://admin:password@localhost:4444/wd/hub"); ClientConfig config = ClientConfig.defaultConfig().baseUri(originalUri); - + JdkHttpClient client = new JdkHttpClient(config); - + // Get the modified URI from the client's config URI modifiedUri = client.getBaseUri(); - + assertThat(modifiedUri.getUserInfo()).isNull(); assertThat(modifiedUri.getHost()).isEqualTo("localhost"); assertThat(modifiedUri.getPort()).isEqualTo(4444); @@ -54,24 +52,23 @@ void shouldStripCredentialsFromUrl() throws URISyntaxException { void shouldHandleUrlWithoutCredentials() throws URISyntaxException { URI originalUri = new URI("http://localhost:4444/wd/hub"); ClientConfig config = ClientConfig.defaultConfig().baseUri(originalUri); - + JdkHttpClient client = new JdkHttpClient(config); - + URI modifiedUri = client.getBaseUri(); - + assertThat(modifiedUri).isEqualTo(originalUri); } @Test void shouldPreserveUrlComponentsExceptCredentials() throws URISyntaxException { - URI originalUri = new URI( - "https://admin:password@localhost:4444/wd/hub?debug=true#fragment"); + URI originalUri = new URI("https://admin:password@localhost:4444/wd/hub?debug=true#fragment"); ClientConfig config = ClientConfig.defaultConfig().baseUri(originalUri); - + JdkHttpClient client = new JdkHttpClient(config); - + URI modifiedUri = client.getBaseUri(); - + assertThat(modifiedUri.getScheme()).isEqualTo("https"); assertThat(modifiedUri.getUserInfo()).isNull(); assertThat(modifiedUri.getHost()).isEqualTo("localhost"); From 560eef403e5c35f2e15c93712359a81aecf0d845 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 19:26:32 +0700 Subject: [PATCH 4/7] fix tests --- .../org/openqa/selenium/remote/http/jdk/JdkHttpClient.java | 4 ++-- .../openqa/selenium/remote/http/jdk/JdkHttpClientTest.java | 6 +++--- 2 files changed, 5 insertions(+), 5 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 1cb2f7ff112ba..c13672023178c 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -80,7 +80,7 @@ public class JdkHttpClient implements HttpClient { private final ExecutorService executorService; private final Duration readTimeout; private final Duration connectTimeout; - private final ClientConfig config; + private ClientConfig config; JdkHttpClient(ClientConfig config) { Objects.requireNonNull(config, "Client config must be set"); @@ -128,7 +128,7 @@ protected PasswordAuthentication getPasswordAuthentication() { // Remove credentials from URL try { - config = + this.config = ClientConfig.defaultConfig() .baseUri( new 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 ec5a0032a647b..7cb49486279bc 100644 --- a/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java +++ b/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java @@ -17,11 +17,12 @@ package org.openqa.selenium.remote.http.jdk; -import static org.hamcrest.MatcherAssert.assertThat; import java.net.URI; import java.net.URISyntaxException; -import org.junit.Test; +import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; +import org.openqa.selenium.remote.http.ClientConfig; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.internal.HttpClientTestBase; @@ -39,7 +40,6 @@ void shouldStripCredentialsFromUrl() throws URISyntaxException { JdkHttpClient client = new JdkHttpClient(config); - // Get the modified URI from the client's config URI modifiedUri = client.getBaseUri(); assertThat(modifiedUri.getUserInfo()).isNull(); From 5fb9c4d22ba571e54a3a5276c03bff295e0d75e1 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 19:27:34 +0700 Subject: [PATCH 5/7] applied format.sh --- .../org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7cb49486279bc..41f1734a248b2 100644 --- a/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java +++ b/java/test/org/openqa/selenium/remote/http/jdk/JdkHttpClientTest.java @@ -17,11 +17,11 @@ package org.openqa.selenium.remote.http.jdk; +import static org.assertj.core.api.Assertions.assertThat; import java.net.URI; import java.net.URISyntaxException; import org.junit.jupiter.api.Test; -import static org.assertj.core.api.Assertions.assertThat; import org.openqa.selenium.remote.http.ClientConfig; import org.openqa.selenium.remote.http.HttpClient; import org.openqa.selenium.remote.internal.HttpClientTestBase; From 558f83c8916eb7c6c6c4d4a3c3ed1f0af5c9fe3d Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Tue, 14 Jan 2025 09:37:24 +0700 Subject: [PATCH 6/7] Optimize code Signed-off-by: Viet Nguyen Duc --- .../remote/http/jdk/JdkHttpClient.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 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 c13672023178c..6fc73728b6778 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -80,12 +80,11 @@ public class JdkHttpClient implements HttpClient { private final ExecutorService executorService; private final Duration readTimeout; private final Duration connectTimeout; - private ClientConfig config; + private final ClientConfig config; JdkHttpClient(ClientConfig config) { Objects.requireNonNull(config, "Client config must be set"); - this.config = config; this.messages = new JdkHttpMessages(config); this.readTimeout = config.readTimeout(); this.connectTimeout = config.connectionTimeout(); @@ -110,7 +109,6 @@ public class JdkHttpClient implements HttpClient { Credentials credentials = config.credentials(); String info = config.baseUri().getUserInfo(); - URI baseUri = config.baseUri(); if (info != null && !info.trim().isEmpty()) { String[] parts = info.split(":", 2); @@ -128,17 +126,15 @@ protected PasswordAuthentication getPasswordAuthentication() { // Remove credentials from URL try { - this.config = - ClientConfig.defaultConfig() - .baseUri( - new URI( - config.baseUri().getScheme(), - null, - config.baseUri().getHost(), - config.baseUri().getPort(), - config.baseUri().getPath(), - config.baseUri().getQuery(), - config.baseUri().getFragment())); + config = config.baseUri( + new URI( + config.baseUri().getScheme(), + null, + config.baseUri().getHost(), + config.baseUri().getPort(), + config.baseUri().getPath(), + config.baseUri().getQuery(), + config.baseUri().getFragment())); } catch (URISyntaxException e) { LOG.log(Level.WARNING, "Could not strip credentials from URI", e); } @@ -174,6 +170,7 @@ protected PasswordAuthentication getPasswordAuthentication() { builder.version(Version.valueOf(version)); } + this.config = config; this.client = builder.build(); } @@ -529,7 +526,7 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException { // Package-private method for testing URI getBaseUri() { - return config.baseUri(); + return this.config.baseUri(); } @Override From dc64346f89825b78d8007e82a7f06330cfb4203b Mon Sep 17 00:00:00 2001 From: Viet Nguyen Duc Date: Tue, 14 Jan 2025 09:53:54 +0700 Subject: [PATCH 7/7] Run format Signed-off-by: Viet Nguyen Duc --- .../remote/http/jdk/JdkHttpClient.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 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 6fc73728b6778..7c7534ece2914 100644 --- a/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java +++ b/java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java @@ -126,15 +126,16 @@ protected PasswordAuthentication getPasswordAuthentication() { // Remove credentials from URL try { - config = config.baseUri( - new URI( - config.baseUri().getScheme(), - null, - config.baseUri().getHost(), - config.baseUri().getPort(), - config.baseUri().getPath(), - config.baseUri().getQuery(), - config.baseUri().getFragment())); + config = + config.baseUri( + new URI( + config.baseUri().getScheme(), + null, + config.baseUri().getHost(), + config.baseUri().getPort(), + config.baseUri().getPath(), + config.baseUri().getQuery(), + config.baseUri().getFragment())); } catch (URISyntaxException e) { LOG.log(Level.WARNING, "Could not strip credentials from URI", e); }