Fix greedy label matching

fixes #464
smithy-lang · Dec 15, 2021 · cd709f7 · cd709f7
1 parent 59dbc85
commit cd709f7
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 20 deletions.
diff --git a/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.spec.ts b/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.spec.ts
@@ -36,7 +36,12 @@ describe("simple matching", () => {
     }),
     new UriSpec(
       "GET",
-      [{ type: "path_literal", value: "mg" }, { type: "greedy" }, { type: "path_literal", value: "z" }],
+      [
+        { type: "path_literal", value: "mg" },
+        { type: "greedy" },
+        { type: "path_literal", value: "y" },
+        { type: "path_literal", value: "z" },
+      ],
       [],
       { service: "Test", operation: "MiddleGreedy" }
     ),
@@ -71,8 +76,10 @@ describe("simple matching", () => {
       new HttpRequest({ method: "GET", path: "/greedy/a/b/c/d", query: { abc: "def" } }),
     ],
     "Test#MiddleGreedy": [
-      new HttpRequest({ method: "GET", path: "/mg/a/z" }),
-      new HttpRequest({ method: "GET", path: "/mg/a/b/c/d/z", query: { abc: "def" } }),
+      new HttpRequest({ method: "GET", path: "/mg/a/y/z" }),
+      new HttpRequest({ method: "GET", path: "/mg/a/b/c/d/y/z", query: { abc: "def" } }),
+      new HttpRequest({ method: "GET", path: "/mg/a/b/y/c/d/y/z", query: { abc: "def" } }),
+      new HttpRequest({ method: "GET", path: "/mg/a/b/y/z/d/y/z", query: { abc: "def" } }),
     ],
     "Test#Delete": [
       new HttpRequest({ method: "DELETE", path: "/", query: { foo: "bar", baz: "quux" } }),
@@ -103,6 +110,10 @@ describe("simple matching", () => {
     new HttpRequest({ method: "GET", path: "/mg" }),
     new HttpRequest({ method: "GET", path: "/mg/q" }),
     new HttpRequest({ method: "GET", path: "/mg/z" }),
+    new HttpRequest({ method: "GET", path: "/mg/y/z" }),
+    new HttpRequest({ method: "GET", path: "/mg/a/z" }),
+    new HttpRequest({ method: "GET", path: "/mg/a/y/z/a" }),
+    new HttpRequest({ method: "GET", path: "/mg/a/y/a" }),
     new HttpRequest({ method: "GET", path: "/mg/a/b/z/c" }),
     new HttpRequest({ method: "DELETE", path: "/", query: { foo: "bar" } }),
     new HttpRequest({ method: "DELETE", path: "/", query: { baz: "quux" } }),

diff --git a/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.ts b/smithy-typescript-ssdk-libs/server-common/src/httpbinding/mux.ts
@@ -78,7 +78,7 @@ export class UriSpec<S extends string, O extends string> {
     const requestPathSegments = req.path.split("/").filter((s) => s.length > 0);
 
     let requestPathIdx = 0;
-    path_loop: for (let i = 0; i < this.pathSegments.length; i++) {
+    for (let i = 0; i < this.pathSegments.length; i++) {
       if (requestPathIdx === requestPathSegments.length) {
         // there are more pathSegments but we have reached the end of the requestPath
         return false;
@@ -88,24 +88,16 @@ export class UriSpec<S extends string, O extends string> {
         return false;
       }
       if (pathSegment.type === "greedy") {
-        if (i === this.pathSegments.length - 1) {
-          // greedy label at the end of pathSegments swallows the remaining path segments
-          requestPathIdx = requestPathSegments.length;
-          break path_loop;
-        }
-        let matched = false;
-        if (i < this.pathSegments.length - 1) {
-          const nextSegment = this.pathSegments[i + 1];
-          while (!matched && ++requestPathIdx < requestPathSegments.length) {
-            if (this.matchesSegment(requestPathSegments[requestPathIdx], nextSegment)) {
-              matched = true;
-            }
-          }
-        }
-        if (!matched) {
-          // the next segment after our greedy label did not match any of the remaining request segments
+        // greedy labels match greedily, and a greedy label cannot be followed by another greedy label
+        // so just consume as many segments as needed to leave an equal number of segments
+        // at the end of the request path as we have segments in the pattern
+        const remainingSegments = this.pathSegments.length - i - 1;
+        const newRequestPathIdx = requestPathSegments.length - remainingSegments;
+        if (requestPathIdx === newRequestPathIdx) {
+          // greedy labels must consume at least one segment
           return false;
         }
+        requestPathIdx = newRequestPathIdx;
       } else {
         requestPathIdx++;
       }