Change the HTTP bindings validation to support any non-conflicting URI

docs/source-2.0/spec/http-bindings.rst

@@ -351,9 +351,19 @@ Greedy labels
 
 A :dfn:`greedy label` is a label suffixed with the ``+`` qualifier that can be
 used to match more than one path segment. At most, one greedy label may exist
-in any path pattern, and if present, it MUST be the last label in the pattern.
+in any path pattern, and if present, it SHOULD be the last label in the pattern.
 Greedy labels MUST be bound to a string shape.
 
+.. important::
+
+    Servers implementing :ref:`Specificity Routing
+    <specificity-routing>` MAY support more than one greedy label and
+    not require it to be the last label in the pattern. The validation
+    events are therefore emitted as ``DANGER`` and can
+    suppressed. Most servers don't support having more than one greedy
+    label. Make sure that your server supports it and test that works
+    as expected before suppressing those events.
+
 Given a pattern of ``/my/uri/{label+}`` and an endpoint of ``http://yourhost``:
 
 .. list-table::
@@ -420,10 +430,9 @@ pattern of ``/prefix/{label+}/suffix`` and an endpoint of ``https://yourhost``:
 Pattern Validation and Conflict Avoidance
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Smithy validates the patterns within a service against each other to ensure
-that no two patterns conflict with each other for the same HTTP method. To
-prevent ambiguity when matching requests for different operations, the
-following rules are in place:
+Smithy validates the patterns within a service against each other to
+ensure that no two patterns conflict with each other for the same HTTP
+method. The following rules are in place:
 
 #. All labels MUST be delimited by '/' characters.
 
@@ -435,41 +444,138 @@ following rules are in place:
 #. At most, one greedy label MAY exist per pattern.
 
    - ``/{foo}/{bar+}`` is legal
-   - ``/{foo+}/{bar+}`` is illegal
+   - ``/{foo+}/bar/{baz+}`` is illegal
 
-#. If present, a greedy pattern MUST be the last label in a pattern.
+#. If present, a greedy pattern SHOULD be the last label in a pattern.
 
    - ``/{foo}/{bar+}`` is legal
-   - ``/{foo+}/{bar}`` is illegal
+   - ``/{foo+}/bar/{baz}`` is illegal
 
 #. Patterns MUST NOT be equivalent if they share a host.
 
    - Pattern ``/foo/bar`` and ``/foo/bar`` conflict.
    - Pattern ``/foo/{bar}`` and ``/foo/{baz}`` conflict regardless of any
      constraint traits on the label members.
 
-#. A label and a literal SHOULD NOT both occupy the same segment in patterns
-   which are equivalent to that point if they share a host.
+#. A label and a literal MAY both occupy the same segment in patterns
+   that are equivalent to that point if they share a host. Server
+   implementations MUST route ambiguous requests to the operation with
+   the most specific URI path (see :ref:`Specificity Routing <specificity-routing>`.)
+
+.. _specificity-routing:
+
+Specificity Routing
+~~~~~~~~~~~~~~~~~~~
+
+Specificity routing allows Smithy compliant servers to support
+ambiguous URI patterns and resolve requests at runtime. The algorithm
+chooses the *best-ranked* match using the specificity of the path and
+literal query parameters when present. The core of the algorithm can
+be loosely defined as “a path with a non-label segment is considered
+more specific than one with a label segment in the same
+position. Similarly a segment with a non-greedy label is considered
+more specific than a segment with a greedy label segment in the same
+position.”
+
+The following algorithm is used to compare two paths
+
+Given two ambiguous URI patterns ``A`` and ``B`` with segments ``[A0,
+…, An]`` and ``[B0, …, Bm]`` with query string literals ``[AQ0, …,
+AQp]`` and ``[BQ0, …, BQq]`` (with both ``p`` and ``q`` possibly zero,
+i.e., without query string literals), the following steps are taken to
+compare them, for each index ``x`` from ``0`` to ``min(n, m)``
+
+#. If ``A[x]`` and ``B[x]`` are both literals then continue (the
+   literal values have to be equal otherwise the patterns are not
+   ambiguous)
+
+#. If ``A[x]`` is a literal and ``B[x]`` is a label then ``A`` is more
+   specific than ``B``,
+
+#. If ``A[x]`` is a non-greedy label and ``B[x]`` is a greedy label
+   then ``A`` is more specific than ``B``
+
+#. If ``n > m`` then ``A`` is more specific than ``B``
+
+#. if ``p > q`` then ``A`` is more specific than ``B``
+
+**Routing Example 1**
 
-   - ``/foo/bar/{baz}`` and ``/foo/baz/bam`` can coexist.
-   - ``/foo/bar`` and ``/foo/{baz}/bam`` cannot coexist unless pattern
-     traits prevent ``{baz}`` from evaluating to ``bar`` because the label
-     occupies the same segment of another pattern with the same prefix.
+Given a service with the following URI patterns for the same method
 
-#. A query string literal with no value and a query string literal with an
-   empty value are considered equivalent. For example, ``/foo?baz`` and
-   ``/foo?baz=`` are considered the same route.
+#. ``/abc/bcd/{xyz}``
+#. ``/abc/{xyz}/cde`` 
+#. ``/{xyz}/bcd/cde``
 
-#. Patterns MAY conflict if the operations use different hosts. Different hosts
-   can be configured using the :ref:`endpoint-trait`'s ``hostPrefix`` property.
+.. list-table::
+    :header-rows: 1
+    :widths: 30 10 60
+
+    * - Request URI
+      - Pattern Matched
+      - Reason
+    * - ``/abc/bcd/cde``
+      - Pattern 1
+      - Ambiguous with patterns 2 and 3. The literal ``bcd`` is more
+        specific than the label ``{xyz}``
+    * - ``/abc/foo/cde`` 
+      - Pattern 2 
+      - Ambiguous with pattern 3. The literal segment ``abc`` is more
+        specific than the label ``{xyz}``.
+    * - ``/foo/bcd/cde`` 
+      - Pattern 3
+      - Non-ambiguous.
+
+**Routing Example 2**
+
+Given a service with the following URI patterns for the same method
+
+#. ``/abc/bcd/{xyz}``
+#. ``/abc/{xyz}/cde``
+#. ``/{xyz}/bcd/cde?def=efg``
+
+.. list-table::
+    :header-rows: 1
+    :widths: 30 10 60
 
-   - ``/foo/bar`` and ``/foo/{baz}/bam`` can coexist if one operation has no
-     endpoint trait and the other specifies ``foo.`` as the ``hostPrefix``.
-   - ``/foo/bar`` and ``/foo/{baz}/bam`` can coexist if one operation specifies
-     ``foo.`` as the ``hostPrefix`` and the other specifies ``bar.`` as the
-     ``hostPrefix``.
+    * - Request URI
+      - Pattern Matched
+      - Reason
+    * - ``/abc/bcd/cde?def=efg``
+      - Pattern 1
+      - Ambiguous with numbers 2 and 3. The literal segment ``abc`` is
+        more specific than the label ``{xyz}``. Notice that path
+        specificity wins over query string literals
+    * - ``/abc/foo/cde?def=efg``
+      - Pattern 2 
+      - Ambiguous with pattern 3. The literal segment ``abc`` is more
+        specific than the label ``{xyz}``.
+    * - ``/foo/bcd/cde?def=efg``
+      - Pattern 3
+      - Non-ambiguous.
+
+**Routing Example 3**
+
+Given a service with the following URI patterns for the same method
+
+#. ``/abc/{xyz+}/bcd``
+#. ``/abc/{xyz+}``
 
+.. list-table::
+    :header-rows: 1
+    :widths: 30 10 60
 
+    * - Request URI
+      - Pattern Matched
+      - Reason
+    * - ``/abc/foo/bar/bcd``
+      - Pattern 1
+      - Ambiguous with numbers 2. The literal segment ``bcd`` is
+        more specific than a non-segment.
+    * - ``/abc/foo/bar/baz``
+      - Pattern 2 
+      - Non-ambiguous.
+
 .. smithy-trait:: smithy.api#httpError
 .. _httpError-trait:
 

smithy-model/src/main/java/software/amazon/smithy/model/pattern/SmithyPattern.java

@@ -54,9 +54,7 @@ protected SmithyPattern(Builder builder) {
         segments = Objects.requireNonNull(builder.segments);
 
         checkForDuplicateLabels();
-        if (builder.allowsGreedyLabels) {
-            checkForLabelsAfterGreedyLabels();
-        } else if (segments.stream().anyMatch(Segment::isGreedyLabel)) {
+        if (!builder.allowsGreedyLabels && segments.stream().anyMatch(Segment::isGreedyLabel)) {
             throw new InvalidPatternException("Pattern must not contain a greedy label. Found " + pattern);
         }
     }
@@ -167,25 +165,6 @@ private void checkForDuplicateLabels() {
         });
     }
 
-    private void checkForLabelsAfterGreedyLabels() {
-        // Make sure at most one greedy label exists, and that it is the
-        // last label segment.
-        for (int i = 0; i < segments.size(); i++) {
-            Segment s = segments.get(i);
-            if (s.isGreedyLabel()) {
-                for (int j = i + 1; j < segments.size(); j++) {
-                    if (segments.get(j).isGreedyLabel()) {
-                        throw new InvalidPatternException(
-                                "At most one greedy label segment may exist in a pattern: " + pattern);
-                    } else if (segments.get(j).isLabel()) {
-                        throw new InvalidPatternException(
-                                "A greedy label must be the last label in its pattern: " + pattern);
-                    }
-                }
-            }
-        }
-    }
-
     /**
      * @return Returns a builder used to create a SmithyPattern.
      */
@@ -310,12 +289,26 @@ public String getContent() {
         }
 
         /**
-         * @return True if the segment is a label.
+         * @return True if the segment is a non-label literal.
+         */
+        public boolean isLiteral() {
+            return segmentType == Type.LITERAL;
+        }
+
+        /**
+         * @return True if the segment is a label regardless of whether is greedy or not.
          */
         public boolean isLabel() {
             return segmentType != Type.LITERAL;
         }
 
+        /**
+         * @return True if the segment is a non-greedy label.
+         */
+        public boolean isNonGreedyLabel() {
+            return segmentType == Type.LABEL;
+        }
+
         /**
          * @return True if the segment is a greedy label.
          */

smithy-model/src/main/java/software/amazon/smithy/model/pattern/UriPattern.java

@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import software.amazon.smithy.utils.Pair;
 
@@ -70,8 +71,8 @@ public static UriPattern parse(String uri) {
             throw new InvalidUriPatternException("URI pattern must not contain a fragment. Found " + uri);
         }
 
-        String[] parts = uri.split(java.util.regex.Pattern.quote("?"), 2);
-        String[] unparsedSegments = parts[0].split(java.util.regex.Pattern.quote("/"));
+        String[] parts = uri.split(Pattern.quote("?"), 2);
+        String[] unparsedSegments = parts[0].split(Pattern.quote("/"));
         List<Segment> segments = new ArrayList<>();
         // Skip the first "/" segment, and thus assume offset of 1.
         int offset = 1;
@@ -88,7 +89,7 @@ public static UriPattern parse(String uri) {
             if (parts[1].contains("{") || parts[1].contains("}")) {
                 throw new InvalidUriPatternException("URI labels must not appear in the query string. Found " + uri);
             }
-            for (String kvp : parts[1].split(java.util.regex.Pattern.quote("&"))) {
+            for (String kvp : parts[1].split(Pattern.quote("&"))) {
                 String[] parameterParts = kvp.split("=", 2);
                 String actualKey = parameterParts[0];
                 if (queryLiterals.containsKey(actualKey)) {
@@ -127,30 +128,29 @@ public Optional<String> getQueryLiteralValue(String parameter) {
      * @return Returns true if there is a conflict.
      */
     public boolean conflictsWith(UriPattern otherPattern) {
-        if (!getConflictingLabelSegmentsMap(otherPattern).isEmpty()) {
-            return true;
-        }
-
         List<Segment> segments = getSegments();
         List<Segment> otherSegments = otherPattern.getSegments();
 
-        // By now we know there are no label conflicts, so one uri has more
-        // segments than the other then they don't conflict.
+        // If one uri has more segments than the other, they don't conflict.
         if (segments.size() != otherSegments.size()) {
             return false;
         }
 
-        // Now we need to check for the differences  in the static segments of the uri.
+        // We check if the patterns are equivalent, if so then there's a conflict.
         for (int i = 0; i < segments.size(); i++) {
             Segment segment = segments.get(i);
             Segment otherSegment = otherSegments.get(i);
-            // We've already checked for label conflicts, so we can skip them here.
-            if (segment.isLabel() || otherSegment.isLabel()) {
+            if (segment.isGreedyLabel() && otherSegment.isGreedyLabel()) {
+                continue;
+            }
+            if (segment.isNonGreedyLabel() && otherSegment.isNonGreedyLabel()) {
                 continue;
             }
-            if (!segment.getContent().equals(otherSegment.getContent())) {
-                return false;
+            if (segment.isLiteral() && otherSegment.isLiteral()
+                && segment.getContent().equals(otherSegment.getContent())) {
+                continue;
             }
+            return false;
         }
 
         // At this point, the path portions are equivalent. If the query

smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/HttpUriGreedyLabelValidator.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file 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 software.amazon.smithy.model.validation.validators;
+
+import java.util.ArrayList;
+import java.util.List;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.pattern.SmithyPattern;
+import software.amazon.smithy.model.pattern.UriPattern;
+import software.amazon.smithy.model.shapes.Shape;
+import software.amazon.smithy.model.traits.HttpTrait;
+import software.amazon.smithy.model.validation.AbstractValidator;
+import software.amazon.smithy.model.validation.ValidationEvent;
+
+/**
+ * Validates that at most one greedy label is present in the pattern, and, if any greedy label is present that it's
+ * the last label in the pattern. This validation emits DANGER events which can be suppressed if the server allows
+ * any of these. Some servers do, but most don't.
+ */
+public class HttpUriGreedyLabelValidator extends AbstractValidator {
+
+    private static final String MULTIPLE_GREEDY_LABELS = "MultipleGreedyLabels";
+    private static final String GREEDY_LABEL_IS_NOT_LAST_LABEL = "GreedyLabelIsNotLastLabel";
+
+    @Override
+    public List<ValidationEvent> validate(Model model) {
+        List<ValidationEvent> events = new ArrayList<>();
+
+        for (Shape shape : model.getShapesWithTrait(HttpTrait.class)) {
+            HttpTrait trait = shape.expectTrait(HttpTrait.class);
+            UriPattern pattern = trait.getUri();
+
+            List<SmithyPattern.Segment> segments = pattern.getSegments();
+            // Make sure at most one greedy label exists, and that it is the
+            // last label segment.
+            for (int i = 0; i < segments.size(); i++) {
+                SmithyPattern.Segment s = segments.get(i);
+                if (s.isGreedyLabel()) {
+                    for (int j = i + 1; j < segments.size(); j++) {
+                        if (segments.get(j).isGreedyLabel()) {
+                            events.add(danger(shape, trait,
+                                              "At most one greedy label segment may exist in a pattern: " + pattern,
+                                              MULTIPLE_GREEDY_LABELS));
+                        }
+                        if (segments.get(j).isLabel()) {
+                            events.add(danger(shape, trait,
+                                              "A greedy label must be the last label in its pattern: " + pattern,
+                                              GREEDY_LABEL_IS_NOT_LAST_LABEL));
+                        }
+                    }
+                }
+            }
+        }
+        return events;
+    }
+}