Create Path from URI with correct decoding and deprecate from String

2 files changed, 55 insertions, 18 deletions

diff --git a/container-core/src/main/java/com/yahoo/restapi/Path.java b/container-core/src/main/java/com/yahoo/restapi/Path.java
index 764fa64f954..fe65245fd15 100644
--- a/container-core/src/main/java/com/yahoo/restapi/Path.java
+++ b/container-core/src/main/java/com/yahoo/restapi/Path.java
@@ -1,17 +1,22 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.restapi;
 
+import java.net.URI;
+import java.net.URLDecoder;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.stream.Stream;
 
 /**
  * A path which is able to match strings containing bracketed placeholders and return the
- * values given at the placeholders.
+ * values given at the placeholders. The path is split on '/', and each part is then URL decoded.
  * 
- * E.g a path /a/1/bar/fuz
- * will match /a/{foo}/bar/{b}
- * and return foo=1 and b=fuz
+ * E.g a path /a/1/bar/fuz/baz/%62%2f
+ * will match /a/{foo}/bar/{b}/baz/{c}
+ * and return foo=1, b=fuz, and c=c/
  * 
  * Only full path elements may be placeholders, i.e /a{bar} is not interpreted as one.
  * 
@@ -21,7 +26,7 @@ import java.util.Map;
  * Note that for convenience in common use this has state which changes as a side effect of each matches
  * invocation. It is therefore for single thread use.
  *
- * A optional prefix can be used to match the path spec against an alternative path. This
+ * An optional prefix can be used to match the path spec against an alternative path. This
  * is used when you have alternative paths mapped to the same resource.
  *
  * @author bratseth
@@ -37,18 +42,36 @@ public class Path {
     private final Map<String, String> values = new HashMap<>();
     private String rest = "";
 
+    /**
+     * @deprecated use {@link #Path(URI)} for correct handling of URL encoded paths.
+     */
+    @Deprecated
     public Path(String path) {
-        this.optionalPrefix = "";
-        this.pathString = path;
-        this.elements = path.split("/");
+        this(path, "");
     }
 
+    /**
+     * @deprecated use {@link #Path(URI, String)} for correct handling of URL encoded paths.
+     */
+    @Deprecated
     public Path(String path, String optionalPrefix) {
         this.optionalPrefix = optionalPrefix;
         this.pathString = path;
         this.elements = path.split("/");
     }
 
+    public Path(URI uri) {
+        this(uri, "");
+    }
+
+    public Path(URI uri, String optionalPrefix) {
+        this.optionalPrefix = optionalPrefix;
+        this.pathString = uri.getRawPath();
+        this.elements = Stream.of(this.pathString.split("/"))
+                              .map(part -> URLDecoder.decode(part, StandardCharsets.UTF_8))
+                              .toArray(String[]::new);
+    }
+
     private boolean matchesInner(String pathSpec) {
         values.clear();
         String[] specElements = pathSpec.split("/");
diff --git a/container-core/src/test/java/com/yahoo/restapi/PathTest.java b/container-core/src/test/java/com/yahoo/restapi/PathTest.java
index 886b3ba9c87..8d5a9bd6591 100644
--- a/container-core/src/test/java/com/yahoo/restapi/PathTest.java
+++ b/container-core/src/test/java/com/yahoo/restapi/PathTest.java
@@ -3,6 +3,8 @@ package com.yahoo.restapi;
 
 import org.junit.Test;
 
+import java.net.URI;
+
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertEquals;
@@ -15,7 +17,7 @@ public class PathTest {
     @Test
     public void testWithPrefix() {
         // Test that a path with a prefix matches spec without the prefix
-        Path path = new Path("/ball/a/1/bar/fuz", "/ball");
+        Path path = new Path(URI.create("/ball/a/1/bar/fuz"), "/ball");
         assertTrue(path.matches("/a/{foo}/bar/{b}"));
         assertEquals("1", path.get("foo"));
         assertEquals("fuz", path.get("b"));
@@ -27,11 +29,11 @@ public class PathTest {
 
     @Test
     public void testPath() {
-        assertFalse(new Path("").matches("/a/{foo}/bar/{b}"));
-        assertFalse(new Path("///").matches("/a/{foo}/bar/{b}"));
-        assertFalse(new Path("///foo").matches("/a/{foo}/bar/{b}"));
-        assertFalse(new Path("///bar/").matches("/a/{foo}/bar/{b}"));
-        Path path = new Path("/a/1/bar/fuz");
+        assertFalse(new Path(URI.create("")).matches("/a/{foo}/bar/{b}"));
+        assertFalse(new Path(URI.create("///")).matches("/a/{foo}/bar/{b}"));
+        assertFalse(new Path(URI.create("///foo")).matches("/a/{foo}/bar/{b}"));
+        assertFalse(new Path(URI.create("///bar/")).matches("/a/{foo}/bar/{b}"));
+        Path path = new Path(URI.create("/a/1/bar/fuz"));
         assertTrue(path.matches("/a/{foo}/bar/{b}"));
         assertEquals("1", path.get("foo"));
         assertEquals("fuz", path.get("b"));
@@ -40,7 +42,7 @@ public class PathTest {
     @Test
     public void testPathWithRest() {
         {
-            Path path = new Path("/a/1/bar/fuz/");
+            Path path = new Path(URI.create("/a/1/bar/fuz/"));
             assertTrue(path.matches("/a/{foo}/bar/{b}/{*}"));
             assertEquals("1", path.get("foo"));
             assertEquals("fuz", path.get("b"));
@@ -48,7 +50,7 @@ public class PathTest {
         }
 
         {
-            Path path = new Path("/a/1/bar/fuz/kanoo");
+            Path path = new Path(URI.create("/a/1/bar/fuz/kanoo"));
             assertTrue(path.matches("/a/{foo}/bar/{b}/{*}"));
             assertEquals("1", path.get("foo"));
             assertEquals("fuz", path.get("b"));
@@ -56,7 +58,7 @@ public class PathTest {
         }
 
         {
-            Path path = new Path("/a/1/bar/fuz/kanoo/trips");
+            Path path = new Path(URI.create("/a/1/bar/fuz/kanoo/trips"));
             assertTrue(path.matches("/a/{foo}/bar/{b}/{*}"));
             assertEquals("1", path.get("foo"));
             assertEquals("fuz", path.get("b"));
@@ -64,7 +66,7 @@ public class PathTest {
         }
 
         {
-            Path path = new Path("/a/1/bar/fuz/kanoo/trips/");
+            Path path = new Path(URI.create("/a/1/bar/fuz/kanoo/trips/"));
             assertTrue(path.matches("/a/{foo}/bar/{b}/{*}"));
             assertEquals("1", path.get("foo"));
             assertEquals("fuz", path.get("b"));
@@ -72,4 +74,16 @@ public class PathTest {
         }
     }
 
+    @Test
+    public void testUrlEncodedPath() {
+        assertTrue(new Path(URI.create("/a/%62/c")).matches("/a/b/c"));
+        assertTrue(new Path(URI.create("/a/%2e%2e/c")).matches("/a/../c"));
+        assertFalse(new Path(URI.create("/a/b%2fc")).matches("/a/b/c"));
+
+        Path path = new Path(URI.create("/%61/%2f/%63"));
+        assertTrue(path.matches("/a/{slash}/{c}"));
+        assertEquals("/", path.get("slash"));
+        assertEquals("c", path.get("c"));
+    }
+
 }