Skip to content

Commit e7d902e

Browse files
authored
Node WebSocket not working with sub-path option (#13407)
[java] Update subPath for ProxyNodeWebsockets Signed-off-by: Viet Nguyen Duc <[email protected]>
1 parent f8944cd commit e7d902e

File tree

8 files changed

+80
-10
lines changed

8 files changed

+80
-10
lines changed

java/src/org/openqa/selenium/grid/commands/Standalone.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ protected Handlers createHandlers(Config config) {
222222
httpHandler = combine(httpHandler, Route.get("/readyz").to(() -> readinessCheck));
223223
Node node = createNode(config, bus, distributor, combinedHandler);
224224

225-
return new Handlers(httpHandler, new ProxyNodeWebsockets(clientFactory, node));
225+
return new Handlers(httpHandler, new ProxyNodeWebsockets(clientFactory, node, subPath));
226226
}
227227

228228
@Override

java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,20 @@ public class ProxyNodeWebsockets
5656
ImmutableSet.of("goog:chromeOptions", "moz:debuggerAddress", "ms:edgeOptions");
5757
private final HttpClient.Factory clientFactory;
5858
private final Node node;
59+
private final String gridSubPath;
5960

60-
public ProxyNodeWebsockets(HttpClient.Factory clientFactory, Node node) {
61+
public ProxyNodeWebsockets(HttpClient.Factory clientFactory, Node node, String gridSubPath) {
6162
this.clientFactory = Objects.requireNonNull(clientFactory);
6263
this.node = Objects.requireNonNull(node);
64+
this.gridSubPath = gridSubPath;
6365
}
6466

6567
@Override
6668
public Optional<Consumer<Message>> apply(String uri, Consumer<Message> downstream) {
67-
UrlTemplate.Match fwdMatch = FWD_TEMPLATE.match(uri);
68-
UrlTemplate.Match cdpMatch = CDP_TEMPLATE.match(uri);
69-
UrlTemplate.Match bidiMatch = BIDI_TEMPLATE.match(uri);
70-
UrlTemplate.Match vncMatch = VNC_TEMPLATE.match(uri);
69+
UrlTemplate.Match fwdMatch = FWD_TEMPLATE.match(uri, gridSubPath);
70+
UrlTemplate.Match cdpMatch = CDP_TEMPLATE.match(uri, gridSubPath);
71+
UrlTemplate.Match bidiMatch = BIDI_TEMPLATE.match(uri, gridSubPath);
72+
UrlTemplate.Match vncMatch = VNC_TEMPLATE.match(uri, gridSubPath);
7173

7274
if (bidiMatch == null && cdpMatch == null && vncMatch == null && fwdMatch == null) {
7375
return Optional.empty();

java/src/org/openqa/selenium/grid/node/config/NodeOptions.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,21 @@ public boolean isManagedDownloadsEnabled() {
162162
return config.getBool(NODE_SECTION, "enable-managed-downloads").orElse(Boolean.FALSE);
163163
}
164164

165+
public String getGridSubPath() {
166+
return normalizeSubPath(getPublicGridUri().map(URI::getPath).orElse(""));
167+
}
168+
169+
public static String normalizeSubPath(String prefix) {
170+
prefix = prefix.trim();
171+
if (!prefix.startsWith("/")) {
172+
prefix = "/" + prefix; // Prefix with a '/' if absent.
173+
}
174+
if (prefix.endsWith("/")) {
175+
prefix = prefix.substring(0, prefix.length() - 1); // Remove the trailing '/' if present.
176+
}
177+
return prefix;
178+
}
179+
165180
public Node getNode() {
166181
return config.getClass(NODE_SECTION, "implementation", Node.class, DEFAULT_NODE_IMPLEMENTATION);
167182
}

java/src/org/openqa/selenium/grid/node/httpd/NodeServer.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,8 @@ protected Handlers createHandlers(Config config) {
171171

172172
Route httpHandler = Route.combine(node, get("/readyz").to(() -> readinessCheck));
173173

174-
return new Handlers(httpHandler, new ProxyNodeWebsockets(clientFactory, node));
174+
return new Handlers(
175+
httpHandler, new ProxyNodeWebsockets(clientFactory, node, nodeOptions.getGridSubPath()));
175176
}
176177

177178
@Override

java/src/org/openqa/selenium/grid/node/local/LocalNode.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -838,9 +838,7 @@ private Session createExternalSession(
838838
private URI rewrite(String path) {
839839
try {
840840
String scheme = "https".equals(gridUri.getScheme()) ? "wss" : "ws";
841-
if (gridUri.getPath() != null && !gridUri.getPath().equals("/")) {
842-
path = gridUri.getPath() + path;
843-
}
841+
path = NodeOptions.normalizeSubPath(gridUri.getPath()) + path;
844842
return new URI(
845843
scheme, gridUri.getUserInfo(), gridUri.getHost(), gridUri.getPort(), path, null, null);
846844
} catch (URISyntaxException e) {

java/src/org/openqa/selenium/remote/http/UrlTemplate.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,20 @@ public UrlTemplate.Match match(String matchAgainst) {
139139
return compiled.apply(matchAgainst);
140140
}
141141

142+
/**
143+
* @return A {@link Match} with all parameters filled if successful, null otherwise. Remove
144+
* subPath from matchAgainst before matching.
145+
*/
146+
public UrlTemplate.Match match(String matchAgainst, String prefix) {
147+
if (matchAgainst == null || prefix == null) {
148+
return null;
149+
}
150+
if (!prefix.isEmpty() && !prefix.equals("/")) {
151+
matchAgainst = matchAgainst.replaceFirst(prefix, "");
152+
}
153+
return match(matchAgainst);
154+
}
155+
142156
@SuppressWarnings("InnerClassMayBeStatic")
143157
public class Match {
144158
private final String url;

java/test/org/openqa/selenium/grid/node/config/NodeOptionsTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,28 @@ void settingTheHubWithDefaultValueSetsTheGridUrlToTheNonLoopbackAddress() {
673673
.isEqualTo(Optional.of(URI.create(nonLoopbackAddressUrl)));
674674
}
675675

676+
@Test
677+
void settingSubPathForNodeServerExtractFromGridUrl() {
678+
String[] rawConfig =
679+
new String[] {
680+
"[node]", "grid-url = \"http://localhost:4444/mySubPath\"",
681+
};
682+
Config config = new TomlConfig(new StringReader(String.join("\n", rawConfig)));
683+
NodeOptions nodeOptions = new NodeOptions(config);
684+
assertThat(nodeOptions.getGridSubPath()).isEqualTo("/mySubPath");
685+
}
686+
687+
@Test
688+
void settingSubPathForNodeServerExtractFromHub() {
689+
String[] rawConfig =
690+
new String[] {
691+
"[node]", "hub = \"http://0.0.0.0:4444/mySubPath\"",
692+
};
693+
Config config = new TomlConfig(new StringReader(String.join("\n", rawConfig)));
694+
NodeOptions nodeOptions = new NodeOptions(config);
695+
assertThat(nodeOptions.getGridSubPath()).isEqualTo("/mySubPath");
696+
}
697+
676698
@Test
677699
void notSettingSlotMatcherAvailable() {
678700
String[] rawConfig =

java/test/org/openqa/selenium/remote/http/UrlTemplateTest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,24 @@ void itIsFineForTheFirstCharacterToBeAPattern() {
6767
assertThat(match.getParameters()).isEqualTo(ImmutableMap.of("cake", "cheese"));
6868
}
6969

70+
@Test
71+
void shouldMatchAgainstUrlTemplateExcludePrefix() {
72+
UrlTemplate.Match match =
73+
new UrlTemplate("/session/{id}/se/vnc").match("/prefix/session/1234/se/vnc", "/prefix");
74+
75+
assertThat(match.getUrl()).isEqualTo("/session/1234/se/vnc");
76+
assertThat(match.getParameters()).isEqualTo(ImmutableMap.of("id", "1234"));
77+
}
78+
79+
@Test
80+
void shouldMatchAgainstUrlTemplateWithEmptyPrefix() {
81+
UrlTemplate.Match match =
82+
new UrlTemplate("/session/{id}/se/vnc").match("/session/1234/se/vnc", "");
83+
84+
assertThat(match.getUrl()).isEqualTo("/session/1234/se/vnc");
85+
assertThat(match.getParameters()).isEqualTo(ImmutableMap.of("id", "1234"));
86+
}
87+
7088
@Test
7189
void aNullMatchDoesNotCauseANullPointerExceptionToBeThrown() {
7290
assertThat(new UrlTemplate("/").match(null)).isNull();

0 commit comments

Comments
 (0)