Make the prefixed routes more robust

This allows redirects to be set properly. *phew*

Author: shs96c

java/client/src/org/openqa/selenium/remote/http/Route.java

@@ -18,10 +18,10 @@
 package org.openqa.selenium.remote.http;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import org.openqa.selenium.json.Json;
 
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -32,12 +32,14 @@
 import java.util.stream.StreamSupport;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
 import static org.openqa.selenium.remote.http.Contents.utf8String;
 import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
 import static org.openqa.selenium.remote.http.HttpMethod.GET;
 import static org.openqa.selenium.remote.http.HttpMethod.POST;
+import static org.openqa.selenium.remote.http.UrlPath.ROUTE_PREFIX_KEY;
 
 public abstract class Route implements HttpHandler, Routable {
 
@@ -234,17 +236,32 @@ public Route to(Route route) {
 
   private static class NestedRoute extends Route {
 
+    private final String[] prefixPaths;
     private final String prefix;
     private final Route route;
 
     private NestedRoute(String prefix, Route route) {
-      this.prefix = Objects.requireNonNull(prefix, "Prefix must be set.");
+      this.prefixPaths = Objects.requireNonNull(prefix, "Prefix must be set.").split("/");
+      this.prefix = prefix;
       this.route = Objects.requireNonNull(route, "Target for requests must be set.");
     }
 
     @Override
     public boolean matches(HttpRequest request) {
-      return request.getUri().startsWith(prefix) && route.matches(transform(request));
+      return hasPrefix(request) && route.matches(transform(request));
+    }
+
+    private boolean hasPrefix(HttpRequest request) {
+      String[] parts = request.getUri().split("/");
+      if (parts.length < prefixPaths.length) {
+        return false;
+      }
+      for (int i = 0; i < prefixPaths.length; i++) {
+        if (!prefixPaths[i].equals(parts[i])) {
+          return false;
+        }
+      }
+      return true;
     }
 
     @Override
@@ -254,7 +271,7 @@ protected HttpResponse handle(HttpRequest req) {
 
     private HttpRequest transform(HttpRequest request) {
       // Strip the prefix from the existing request and forward it.
-      String unprefixed = request.getUri().startsWith(prefix) ?
+      String unprefixed = hasPrefix(request) ?
                           request.getUri().substring(prefix.length()) :
                           request.getUri();
 
@@ -270,6 +287,16 @@ private HttpRequest transform(HttpRequest request) {
       request.getAttributeNames().forEach(
           attr -> toForward.setAttribute(attr, request.getAttribute(attr)));
 
+      // Don't forget to register our prefix
+      Object rawPrefixes = request.getAttribute(ROUTE_PREFIX_KEY);
+      if (!(rawPrefixes instanceof List)) {
+        rawPrefixes = new LinkedList<>();
+      }
+      List<String> prefixes = Stream.concat(((List<?>) rawPrefixes).stream(), Stream.of(prefix))
+        .map(String::valueOf)
+        .collect(toImmutableList());
+      toForward.setAttribute(ROUTE_PREFIX_KEY, prefixes);
+
       request.getQueryParameterNames().forEach(name -> {
         request.getQueryParameters(name).forEach(value -> toForward.addQueryParameter(name, value));
       });
@@ -287,7 +314,7 @@ private static class CombinedRoute extends Route {
     private CombinedRoute(Stream<Routable> routes) {
       // We want later routes to have a greater chance of being called so that we can override
       // routes as necessary.
-      allRoutes = routes.collect(ImmutableList.toImmutableList()).reverse();
+      allRoutes = routes.collect(toImmutableList()).reverse();
       Preconditions.checkArgument(!allRoutes.isEmpty(), "At least one route must be specified.");
     }
 

java/client/src/org/openqa/selenium/remote/http/UrlPath.java

@@ -0,0 +1,51 @@
+// 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;
+
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+public class UrlPath {
+
+  static final String ROUTE_PREFIX_KEY = "selenium.route";
+
+  private UrlPath() {
+    // Utility class
+  }
+
+  public static String relativeToServer(HttpRequest req, String location) {
+    Objects.requireNonNull(location, "Location must be set");
+
+    return location;
+  }
+
+  public static String relativeToContext(HttpRequest req, String location) {
+    Objects.requireNonNull(location, "Location to redirect to must be set");
+
+    Object rawPrefix = req.getAttribute(ROUTE_PREFIX_KEY);
+    String prefix;
+    if (rawPrefix instanceof List) {
+      prefix = ((List<?>) rawPrefix).stream().map(String::valueOf).collect(Collectors.joining());
+    } else {
+      prefix = "";
+    }
+
+    return prefix + location;
+  }
+}

java/client/test/org/openqa/selenium/remote/http/PrefixedRouteTest.java

@@ -0,0 +1,103 @@
+// 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;
+
+import org.junit.Test;
+
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+import static org.openqa.selenium.remote.http.HttpMethod.GET;
+
+public class PrefixedRouteTest {
+
+  @Test
+  public void pathWithoutPrefixIsNotMatched() {
+    Route route = Route.prefix("/cheese").to(Route.matching(req -> true).to(() -> req -> new HttpResponse()));
+
+    assertThat(route.matches(new HttpRequest(GET, "/cake"))).isFalse();
+  }
+
+  @Test
+  public void pathWithPrefixIsMatched() {
+    Route route = Route.prefix("/cheese").to(Route.matching(req -> true).to(() -> req -> new HttpResponse()));
+
+    assertThat(route.matches(new HttpRequest(GET, "/cheese/cake"))).isTrue();
+  }
+
+  @Test
+  public void pathWhichCoincidentallyStartsWithThePrefixIsNotMacthed() {
+    Route route = Route.prefix("/cheese").to(Route.matching(req -> true).to(() -> req -> new HttpResponse()));
+
+    assertThat(route.matches(new HttpRequest(GET, "/cheeseandpeas"))).isFalse();
+  }
+
+  @Test
+  public void pathWhichIsJustThePrefixMatches() {
+    Route route = Route.prefix("/cheese").to(Route.matching(req -> true).to(() -> req -> new HttpResponse()));
+
+    assertThat(route.matches(new HttpRequest(GET, "/cheese"))).isTrue();
+  }
+
+  @Test
+  public void pathWhichIsJustThePrefixAndATrailingSlashMatches() {
+    Route route = Route.prefix("/cheese").to(Route.matching(req -> true).to(() -> req -> new HttpResponse()));
+
+    assertThat(route.matches(new HttpRequest(GET, "/cheese/"))).isTrue();
+  }
+
+  @Test
+  public void pathWhichDoesMatchHasPrefixAsAttributeWhenHandling() {
+    AtomicReference<String> path = new AtomicReference<>();
+    AtomicReference<List<?>> parts = new AtomicReference<>();
+
+    Route route = Route.prefix("/cheese").to(Route.matching(req -> true)
+      .to(() -> req -> {
+        path.set(req.getUri());
+        parts.set((List<?>) req.getAttribute(UrlPath.ROUTE_PREFIX_KEY));
+        return new HttpResponse();
+      }));
+
+    route.execute(new HttpRequest(GET, "/cheese/and/peas"));
+
+    assertThat(path.get()).isEqualTo("/and/peas");
+    assertThat(parts.get()).isEqualTo(List.of("/cheese"));
+  }
+
+  @Test
+  public void nestingPrefixesAlsoCausesPathStoredInAttributeToBeExtended() {
+    AtomicReference<String> path = new AtomicReference<>();
+    AtomicReference<List<?>> parts = new AtomicReference<>();
+
+    Route route = Route.prefix("/cheese").to(
+      Route.prefix("/and").to(
+        Route.matching(req -> true)
+          .to(() -> req -> {
+            path.set(req.getUri());
+            parts.set((List<?>) req.getAttribute(UrlPath.ROUTE_PREFIX_KEY));
+            return new HttpResponse();
+          })));
+
+    route.execute(new HttpRequest(GET, "/cheese/and/peas"));
+
+    assertThat(path.get()).isEqualTo("/peas");
+    assertThat(parts.get()).isEqualTo(List.of("/cheese", "/and"));
+  }
+}

java/client/test/org/openqa/selenium/remote/http/UrlPathTest.java

@@ -0,0 +1,53 @@
+// 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;
+
+import org.junit.Test;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.openqa.selenium.remote.http.HttpMethod.GET;
+import static org.openqa.selenium.remote.http.UrlPath.ROUTE_PREFIX_KEY;
+
+public class UrlPathTest {
+
+  @Test
+  public void shouldAssumeARegularHttpRequestHasNoPrefix() {
+    HttpRequest req = new HttpRequest(GET, "/cheese");
+
+    String absolute = UrlPath.relativeToServer(req, "/cake");
+    assertThat(absolute).isEqualTo("/cake");
+
+    String relative = UrlPath.relativeToContext(req, "/cake");
+    assertThat(relative).isEqualTo("/cake");
+  }
+
+  @Test
+  public void shouldRedirectARequestWithAPrefixAttribute() {
+    HttpRequest req = new HttpRequest(GET, "/cake");
+    req.setAttribute(ROUTE_PREFIX_KEY, List.of("/cheese"));
+
+    String absolute = UrlPath.relativeToServer(req, "/cake");
+    assertThat(absolute).isEqualTo("/cake");
+
+    String relative = UrlPath.relativeToContext(req, "/cake");
+    assertThat(relative).isEqualTo("/cheese/cake");
+  }
+
+}