The W3C spec is pretty clear about what's allowed in new session

To be exact, only things that look like they're peotocol extensions and the handful of allowed tags are to be allowed into the firstMatch and anyMatch parameters.
SeleniumHQ · Mar 28, 2017 · f948e6d · f948e6d
1 parent 8fde866
commit f948e6d
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 8 deletions.
diff --git a/java/client/src/org/openqa/selenium/remote/ProtocolHandshake.java b/java/client/src/org/openqa/selenium/remote/ProtocolHandshake.java
@@ -56,8 +56,10 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -67,13 +69,22 @@ public class ProtocolHandshake {
   private final static Logger LOG = Logger.getLogger(ProtocolHandshake.class.getName());
 
   /**
-   * Capability names that should never be sent across the wire to a w3c compliant remote end.
+   * Patterns that are acceptable to send to a w3c remote end.
    */
-  private final Set<String> UNUSED_W3C_NAMES = ImmutableSet.<String>builder()
-      .add("firefox_binary")
-      .add("firefox_profile")
-      .add("marionette")
-      .build();
+  private final Predicate<String> ACCEPTED_W3C_PATTERNS = Stream.of(
+      "^[\\w-]+:.*$",
+      "^acceptInsecureCerts$",
+      "^browserName$",
+      "^browserVersion$",
+      "^platformName$",
+      "^pageLoadStrategy$",
+      "^proxy$",
+      "^setWindowRect$",
+      "^timeouts$",
+      "^unhandledPromptBehavior$")
+      .map(Pattern::compile)
+      .map(Pattern::asPredicate)
+      .reduce(identity -> false, Predicate::or);
 
   public Result createSession(HttpClient client, Command command)
     throws IOException {
@@ -246,7 +257,7 @@ private void streamW3CProtocolParameters(
         .flatMap(Collection::stream)
         .filter(entry -> !excludedKeys.contains(entry.getKey()))
         .filter(entry -> entry.getValue() != null)
-        .filter(entry -> !UNUSED_W3C_NAMES.contains(entry.getKey()))  // We never want to send this
+        .filter(entry -> ACCEPTED_W3C_PATTERNS.test(entry.getKey()))
         .distinct()
         .collect(Collector.of(
             JsonObject::new,
@@ -264,7 +275,7 @@ private void streamW3CProtocolParameters(
         .map(map -> {
           JsonObject json = new JsonObject();
           for (Map.Entry<String, ?> entry : map.entrySet()) {
-            if (!UNUSED_W3C_NAMES.contains(entry.getKey())) {
+            if (ACCEPTED_W3C_PATTERNS.test(entry.getKey())) {
               json.add(entry.getKey(), gson.toJsonTree(entry.getValue()));
             }
           }

diff --git a/java/client/test/org/openqa/selenium/remote/ProtocolHandshakeTest.java b/java/client/test/org/openqa/selenium/remote/ProtocolHandshakeTest.java
@@ -20,6 +20,7 @@
 import static java.net.HttpURLConnection.HTTP_OK;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.ImmutableList;
@@ -33,9 +34,15 @@
 import org.openqa.selenium.remote.http.HttpClient;
 import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.http.HttpResponse;
+import org.openqa.selenium.remote.http.W3CHttpCommandCodec;
 
 import java.io.IOException;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 @SuppressWarnings("unchecked")
 @RunWith(JUnit4.class)
@@ -187,6 +194,49 @@ public void shouldAddBothGeckoDriverAndW3CCapabilitiesToRootCapabilitiesProperty
     assertTrue(capabilities.containsKey("firstMatch"));
   }
 
+  @Test
+  public void shouldNotIncludeNonProtocolExtensionKeys() throws IOException {
+    DesiredCapabilities caps = new DesiredCapabilities();
+    caps.setCapability("se:option", "cheese");
+    caps.setCapability("option", "I like sausages");
+    caps.setCapability("browserName", "amazing cake browser");
+
+    Map<String, Object> params = ImmutableMap.of("desiredCapabilities", caps);
+    Command command = new Command(null, DriverCommand.NEW_SESSION, params);
+
+    HttpResponse response = new HttpResponse();
+    response.setStatus(HTTP_OK);
+    response.setContent(
+        "{\"sessionId\": \"23456789\", \"status\": 0, \"value\": {}}".getBytes(UTF_8));
+    RecordingHttpClient client = new RecordingHttpClient(response);
+
+    new ProtocolHandshake().createSession(client, command);
+
+    HttpRequest request = client.getRequest();
+    Map<String, Object> handshakeRequest = new Gson().fromJson(
+        request.getContentString(),
+        new TypeToken<Map<String, Object>>() {}.getType());
+
+    Object rawCaps = handshakeRequest.get("capabilities");
+    assertTrue(rawCaps instanceof Map);
+
+    Map<?, ?> capabilities = (Map<?, ?>) rawCaps;
+
+    Map<?, ?> always = (Map<?, ?>) capabilities.get("alwaysMatch");
+    List<Map<?, ?>> first = (List<Map<?, ?>>) capabilities.get("firstMatch");
+
+    // We don't care where they are, but we want to see "se:option" and not "option"
+    Set<String> keys = new HashSet(always.keySet());
+    keys.addAll(first.stream()
+                    .map(Map::keySet)
+                    .flatMap(Collection::stream)
+                    .map(String::valueOf)
+                    .collect(Collectors.toSet()));
+    assertTrue(keys.contains("browserName"));
+    assertTrue(keys.contains("se:option"));
+    assertFalse(keys.contains("options"));
+  }
+
   class RecordingHttpClient implements HttpClient {
 
     private final HttpResponse response;