Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions dotnet/src/webdriver/Remote/HttpCommandExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ public HttpCommandExecutor(Uri addressOfRemoteServer, TimeSpan timeout, bool ena
/// </summary>
public string UserAgent { get; set; }

/// <summary>
/// Gets the address of the remote end this executor connects to.
/// </summary>
internal Uri RemoteServerUri => this.remoteServerUri;

/// <summary>
/// Gets the repository of objects containing information about commands.
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions dotnet/src/webdriver/WebDriver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,11 @@ protected void StartSession(ICapabilities capabilities)
{
Dictionary<string, object> matchCapabilities = this.GetCapabilitiesDictionary(capabilities);

if (this.CommandExecutor is Remote.HttpCommandExecutor httpExecutor)
{
matchCapabilities["se:remoteUrl"] = httpExecutor.RemoteServerUri.AbsoluteUri;
}

List<object> firstMatchCapabilitiesList = new List<object>();
firstMatchCapabilitiesList.Add(matchCapabilities);

Expand Down
36 changes: 29 additions & 7 deletions java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -1221,10 +1221,12 @@ private Session createExternalSession(
Capabilities toUse =
ImmutableCapabilities.copyOf(requestCapabilities.merge(other.getCapabilities()));

URI baseUri = resolvePublicGridUri(toUse);

// Add se:cdp if necessary to send the cdp url back
if ((isSupportingCdp || toUse.getCapability("se:cdp") != null) && cdpEnabled) {
String cdpPath = String.format("/session/%s/se/cdp", other.getId());
toUse = new PersistentCapabilities(toUse).setCapability("se:cdp", rewrite(cdpPath));
toUse = new PersistentCapabilities(toUse).setCapability("se:cdp", rewrite(cdpPath, baseUri));
} else {
// Remove any se:cdp* from the response, CDP is not supported nor enabled
MutableCapabilities cdpFiltered = new MutableCapabilities();
Expand Down Expand Up @@ -1259,7 +1261,7 @@ private Session createExternalSession(
toUse =
new PersistentCapabilities(toUse)
.setCapability("se:gridWebSocketUrl", uri)
.setCapability("webSocketUrl", rewrite(bidiPath));
.setCapability("webSocketUrl", rewrite(bidiPath, baseUri));
} else {
// Remove any "webSocketUrl" from the response, BiDi is not supported nor enabled
MutableCapabilities bidiFiltered = new MutableCapabilities();
Expand All @@ -1278,23 +1280,43 @@ private Session createExternalSession(
boolean isVncEnabled = toUse.getCapability("se:vncLocalAddress") != null;
if (isVncEnabled) {
String vncPath = String.format("/session/%s/se/vnc", other.getId());
toUse = new PersistentCapabilities(toUse).setCapability("se:vnc", rewrite(vncPath));
toUse = new PersistentCapabilities(toUse).setCapability("se:vnc", rewrite(vncPath, baseUri));
}

return new Session(other.getId(), externalUri, other.getStereotype(), toUse, Instant.now());
}

private URI rewrite(String path) {
private URI rewrite(String path, URI baseUri) {
try {
String scheme = "https".equals(gridUri.getScheme()) ? "wss" : "ws";
path = NodeOptions.normalizeSubPath(gridUri.getPath()) + path;
String scheme = "https".equals(baseUri.getScheme()) ? "wss" : "ws";
path = NodeOptions.normalizeSubPath(baseUri.getPath()) + path;
return new URI(
scheme, gridUri.getUserInfo(), gridUri.getHost(), gridUri.getPort(), path, null, null);
scheme, baseUri.getUserInfo(), baseUri.getHost(), baseUri.getPort(), path, null, null);
} catch (URISyntaxException e) {
throw new RuntimeException(e);
}
}

// A configured grid-url always wins; only when the node falls back to its auto-detected address
// (which may be unreachable behind Docker/proxy) do we use the client-advertised se:remoteUrl.
private URI resolvePublicGridUri(Capabilities caps) {
if (!gridUri.equals(externalUri)) {
return gridUri;
}
Object raw = caps.getCapability("se:remoteUrl");
if (raw instanceof String && !((String) raw).isEmpty()) {
try {
URI uri = new URI((String) raw);
if (uri.getHost() != null) {
return uri;
}
} catch (URISyntaxException e) {
// Fall back to gridUri.
}
}
return gridUri;
}
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Comment thread
qodo-code-review[bot] marked this conversation as resolved.

@Override
public NodeStatus getStatus() {
Set<Slot> slots =
Expand Down
11 changes: 11 additions & 0 deletions java/src/org/openqa/selenium/remote/RemoteWebDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,20 @@ protected void setSessionId(String opaqueKey) {
sessionId = new SessionId(opaqueKey);
}

private Capabilities addRemoteUrl(Capabilities capabilities) {
URI baseUri = clientConfig.baseUri();
if (baseUri == null) {
return capabilities;
}
MutableCapabilities withRemoteUrl = new MutableCapabilities(capabilities);
withRemoteUrl.setCapability("se:remoteUrl", baseUri.toString());
return withRemoteUrl;
}

protected void startSession(Capabilities capabilities) {
checkNonW3CCapabilities(capabilities);
checkChromeW3CFalse(capabilities);
capabilities = addRemoteUrl(capabilities);

Comment thread
titusfortner marked this conversation as resolved.
try {
Response response = execute(DriverCommand.NEW_SESSION(singleton(capabilities)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,14 @@ private Set<String> getClobberedCapabilities() {
private NewSessionPayload getPayload() {
Map<String, Object> roughPayload = new TreeMap<>(metadata);

Map<String, Object> alwaysMatch = new TreeMap<>(additionalCapabilities);
URI baseUri = getBaseUri();
if (baseUri != null) {
alwaysMatch.put("se:remoteUrl", baseUri.toString());
}

Map<String, Object> w3cCaps = new TreeMap<>();
w3cCaps.put("alwaysMatch", additionalCapabilities);
w3cCaps.put("alwaysMatch", alwaysMatch);
if (!requestedCapabilities.isEmpty()) {
w3cCaps.put("firstMatch", requestedCapabilities);
}
Expand Down
130 changes: 130 additions & 0 deletions java/test/org/openqa/selenium/grid/node/NodeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,136 @@ void shouldCreateASessionIfTheCorrectCapabilitiesArePassedToIt() {
assertThat(sessionResponse.getSession()).isNotNull();
}

@Test
void usesClientReachableAddressForProxiedUrlsWhenRemoteUrlProvided() {
Capabilities request =
new ImmutableCapabilities(
"browserName",
"cheese",
"se:vncLocalAddress",
"localhost:5900",
"se:remoteUrl",
"http://localhost:9999");

Either<WebDriverException, CreateSessionResponse> response =
local.newSession(createSessionRequest(request));
assertThatEither(response).isRight();

Session session = response.right().getSession();
assertThat(String.valueOf(session.getCapabilities().getCapability("se:vnc")))
.isEqualTo("ws://localhost:9999/session/" + session.getId() + "/se/vnc");
}

@Test
void preservesCredentialsFromClientAdvertisedRemoteUrl() {
Capabilities request =
new ImmutableCapabilities(
"browserName",
"cheese",
"se:vncLocalAddress",
"localhost:5900",
"se:remoteUrl",
"http://user:secret@localhost:9999");

Either<WebDriverException, CreateSessionResponse> response =
local.newSession(createSessionRequest(request));
assertThatEither(response).isRight();

Session session = response.right().getSession();
assertThat(String.valueOf(session.getCapabilities().getCapability("se:vnc")))
.isEqualTo("ws://user:secret@localhost:9999/session/" + session.getId() + "/se/vnc");
}

@Test
void ignoresRemoteUrlWhenPublicGridUrlIsConfigured() throws URISyntaxException {
URI configuredGridUri = new URI("http://grid.example:4444");

class Handler extends Session implements HttpHandler {
private Handler(Capabilities capabilities) {
super(new SessionId(UUID.randomUUID()), uri, stereotype, capabilities, Instant.now());
}

@Override
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
return new HttpResponse();
}
}

LocalNode node =
LocalNode.builder(tracer, bus, uri, configuredGridUri, registrationSecret)
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
.build();

Capabilities request =
new ImmutableCapabilities(
"browserName",
"cheese",
"se:vncLocalAddress",
"localhost:5900",
"se:remoteUrl",
"http://localhost:9999");

Either<WebDriverException, CreateSessionResponse> response =
node.newSession(createSessionRequest(request));
assertThatEither(response).isRight();

Session session = response.right().getSession();
assertThat(String.valueOf(session.getCapabilities().getCapability("se:vnc")))
.isEqualTo("ws://grid.example:4444/session/" + session.getId() + "/se/vnc");
}

@Test
void preservesCredentialsFromConfiguredPublicGridUrl() throws URISyntaxException {
URI configuredGridUri = new URI("http://user:secret@grid.example:4444");

class Handler extends Session implements HttpHandler {
private Handler(Capabilities capabilities) {
super(new SessionId(UUID.randomUUID()), uri, stereotype, capabilities, Instant.now());
}

@Override
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
return new HttpResponse();
}
}

LocalNode node =
LocalNode.builder(tracer, bus, uri, configuredGridUri, registrationSecret)
.add(caps, new TestSessionFactory((id, c) -> new Handler(c)))
.build();

Capabilities request =
new ImmutableCapabilities(
"browserName",
"cheese",
"se:vncLocalAddress",
"localhost:5900",
"se:remoteUrl",
"http://localhost:9999");

Either<WebDriverException, CreateSessionResponse> response =
node.newSession(createSessionRequest(request));
assertThatEither(response).isRight();

Session session = response.right().getSession();
assertThat(String.valueOf(session.getCapabilities().getCapability("se:vnc")))
.isEqualTo("ws://user:secret@grid.example:4444/session/" + session.getId() + "/se/vnc");
}

@Test
void fallsBackToGridAddressForProxiedUrlsWithoutRemoteUrl() {
Capabilities request =
new ImmutableCapabilities("browserName", "cheese", "se:vncLocalAddress", "localhost:5900");

Either<WebDriverException, CreateSessionResponse> response =
local.newSession(createSessionRequest(request));
assertThatEither(response).isRight();

Session session = response.right().getSession();
assertThat(String.valueOf(session.getCapabilities().getCapability("se:vnc")))
.isEqualTo("ws://localhost:1234/session/" + session.getId() + "/se/vnc");
}

@Test
void shouldRetryIfNoMatchingSlotIsAvailable() {
Node local =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,24 @@ void shouldAllowMetaDataToBeSet() {
assertThat(seen).isTrue();
}

@Test
void advertisesRemoteUrlToTheServer() {
AtomicReference<Object> seen = new AtomicReference<>();

RemoteWebDriver.builder()
.oneOf(new FirefoxOptions())
.address("http://localhost:34576")
.connectingWith(
config ->
req -> {
seen.set(listCapabilities(req).get(0).getCapability("se:remoteUrl"));
return CANNED_SESSION_RESPONSE;
})
.build();

assertThat(seen.get()).isEqualTo("http://localhost:34576");
}

@Test
void doesNotAllowFirstMatchToBeUsedAsAMetadataNameAsItIsConfusing() {
RemoteWebDriverBuilder builder = RemoteWebDriver.builder();
Expand Down
14 changes: 14 additions & 0 deletions java/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static org.openqa.selenium.remote.WebDriverFixture.webDriverExceptionResponder;

import java.io.IOException;
import java.net.URI;
import java.net.URL;
import java.time.Duration;
import java.util.ArrayList;
Expand Down Expand Up @@ -63,6 +64,7 @@
import org.openqa.selenium.WebElement;
import org.openqa.selenium.WindowType;
import org.openqa.selenium.internal.Debug;
import org.openqa.selenium.remote.http.ClientConfig;
import org.openqa.selenium.virtualauthenticator.VirtualAuthenticator;
import org.openqa.selenium.virtualauthenticator.VirtualAuthenticatorOptions;

Expand All @@ -71,6 +73,18 @@ class RemoteWebDriverUnitTest {

private static final String ELEMENT_KEY = "element-6066-11e4-a52e-4f735466cecf";

@Test
void advertisesRemoteUrlWhenStartingRemoteSession() {
RemoteWebDriver driver =
new RemoteWebDriver(
WebDriverFixture.prepareExecutorMock(echoCapabilities),
new ImmutableCapabilities("browserName", "chrome"),
ClientConfig.defaultConfig().baseUri(URI.create("http://grid.example:4444/wd/hub")));

assertThat(driver.getCapabilities().getCapability("se:remoteUrl"))
.isEqualTo("http://grid.example:4444/wd/hub");
}

@Test
void canHandleGetCommand() {
WebDriverFixture fixture = new WebDriverFixture(echoCapabilities, nullValueResponder);
Expand Down
5 changes: 5 additions & 0 deletions javascript/selenium-webdriver/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,11 @@ class Builder {

if (url) {
this.log_.fine('Creating session on remote server')

if (typeof url === 'string') {
capabilities.set('se:remoteUrl', url)
}
Comment on lines +654 to +656

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zero functional impact. The only thenable url source is startSeleniumServer(SELENIUM_SERVER_JAR) (line 647), which starts a local server and resolves to its localhost address. That's precisely the case where se:remoteUrl is pointless
The proposed fix doesn't work Executor.execute serializes the capabilities at lib/http.js:445 (buildRequest) before it awaits the client promise at line 450. So setting se:remoteUrl inside the Promise.resolve(url).then(...) chain runs after the NEW_SESSION body is already built β€” too late


let client = Promise.resolve(url).then((url) => new _http.HttpClient(url, this.agent_, this.proxy_))
let executor = new _http.Executor(client)

Expand Down
10 changes: 10 additions & 0 deletions py/selenium/webdriver/remote/webdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,9 @@ def start_session(self, capabilities: dict) -> None:
Args:
capabilities: A capabilities dict to start the session with.
"""
remote_url = self._remote_url()
if remote_url:
capabilities = {**capabilities, "se:remoteUrl": remote_url}
caps = _create_caps(capabilities)
try:
response = self.execute(Command.NEW_SESSION, caps)["value"]
Expand All @@ -401,6 +404,13 @@ def start_session(self, capabilities: dict) -> None:
self.service.stop()
raise

def _remote_url(self) -> str | None:
"""The address used to reach the Grid, advertised as ``se:remoteUrl`` (None for local drivers)."""
if getattr(self, "service", None) is not None:
return None
client_config = getattr(self.command_executor, "client_config", None)
return getattr(client_config, "remote_server_addr", None) or None

def _wrap_value(self, value):
if isinstance(value, dict):
converted = {}
Expand Down
8 changes: 6 additions & 2 deletions py/test/unit/selenium/webdriver/remote/new_session_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,12 @@ def test_converts_proxy_type_value_to_lowercase_for_w3c(mocker):
proxy = Proxy({"proxyType": ProxyType.MANUAL, "httpProxy": "foo"})
options.proxy = proxy
WebDriver(options=options)
expected_params = {"capabilities": {"firstMatch": [{}], "alwaysMatch": w3c_caps}}
mock.assert_called_with(Command.NEW_SESSION, expected_params)
command, params = mock.call_args[0]
assert command == Command.NEW_SESSION
always_match = params["capabilities"]["alwaysMatch"]
always_match.pop("se:remoteUrl", None)
assert params["capabilities"]["firstMatch"] == [{}]
assert always_match == w3c_caps
Comment thread
titusfortner marked this conversation as resolved.


def test_works_as_context_manager(mocker):
Expand Down
1 change: 1 addition & 0 deletions rb/lib/selenium/webdriver/remote/driver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def initialize(capabilities: nil, options: nil, service: nil, url: nil, http_cli
caps = process_options(options, capabilities)
http_client ||= Remote::Http::Default.new(client_config: client_config)
http_client.server_url = url || client_config&.server_url || "http://#{Platform.localhost}:4444/wd/hub"
caps['se:remoteUrl'] = http_client.client_config.server_url.to_s
super(caps: caps, http_client: http_client, **)
@bridge.file_detector = ->((filename, *)) { File.exist?(filename) && filename.to_s }
command_list = @bridge.command_list
Expand Down
Loading
Loading