Merge pull request #192 from JordonPhillips/http-header-danger

Make httpHeader value validation produce dangers
smithy-lang · Oct 24, 2019 · b914ce5 · b914ce5
2 parents 8884b48 + 96f372e
commit b914ce5
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 27 deletions.
diff --git a/docs/source/spec/http.rst b/docs/source/spec/http.rst
@@ -519,7 +519,7 @@ Serialization rules:
 Restricted HTTP headers
 -----------------------
 
-Various HTTP headers are not allowed for the ``httpHeader`` and
+Various HTTP headers are highly discouraged for the ``httpHeader`` and
 ``httpPrefixHeaders`` traits.
 
 .. list-table::

diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpHeaderTrait.java b/smithy-model/src/main/java/software/amazon/smithy/model/traits/HttpHeaderTrait.java
@@ -15,46 +15,22 @@
 
 package software.amazon.smithy.model.traits;
 
-import java.util.Locale;
-import java.util.Set;
 import software.amazon.smithy.model.SourceException;
 import software.amazon.smithy.model.SourceLocation;
 import software.amazon.smithy.model.shapes.ShapeId;
-import software.amazon.smithy.utils.SetUtils;
 
 /**
  * Binds a member to an HTTP header.
  */
 public final class HttpHeaderTrait extends StringTrait {
     public static final ShapeId ID = ShapeId.from("smithy.api#httpHeader");
 
-    private static final Set<String> BLACKLIST = SetUtils.of(
-            "authorization",
-            "connection",
-            "content-length",
-            "expect",
-            "host",
-            "max-forwards",
-            "proxy-authenticate",
-            "server",
-            "te",
-            "trailer",
-            "transfer-encoding",
-            "upgrade",
-            "user-agent",
-            "www-authenticate",
-            "x-forwarded-for");
-
     public HttpHeaderTrait(String value, SourceLocation sourceLocation) {
         super(ID, value, sourceLocation);
 
         if (getValue().isEmpty()) {
             throw new SourceException("httpHeader field name binding must not be empty", getSourceLocation());
         }
-
-        if (BLACKLIST.contains(getValue().toLowerCase(Locale.US))) {
-            throw new SourceException("httpHeader cannot be set to `" + getValue() + "`", getSourceLocation());
-        }
     }
 
     public HttpHeaderTrait(String value) {

diff --git a/...ain/java/software/amazon/smithy/model/validation/validators/HttpHeaderTraitValidator.java b/...ain/java/software/amazon/smithy/model/validation/validators/HttpHeaderTraitValidator.java
@@ -21,25 +21,55 @@
 
 import java.util.List;
 import java.util.Locale;
+import java.util.Set;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.StructureShape;
 import software.amazon.smithy.model.traits.HttpHeaderTrait;
 import software.amazon.smithy.model.traits.Trait;
 import software.amazon.smithy.model.validation.AbstractValidator;
 import software.amazon.smithy.model.validation.ValidationEvent;
 import software.amazon.smithy.model.validation.ValidationUtils;
+import software.amazon.smithy.utils.SetUtils;
 
 /**
  * Validates that httpHeader traits are case-insensitively unique.
  */
 public final class HttpHeaderTraitValidator extends AbstractValidator {
 
+    private static final Set<String> BLACKLIST = SetUtils.of(
+            "authorization",
+            "connection",
+            "content-length",
+            "expect",
+            "host",
+            "max-forwards",
+            "proxy-authenticate",
+            "server",
+            "te",
+            "trailer",
+            "transfer-encoding",
+            "upgrade",
+            "user-agent",
+            "www-authenticate",
+            "x-forwarded-for");
+
     @Override
     public List<ValidationEvent> validate(Model model) {
-        return model.getShapeIndex().shapes(StructureShape.class)
+        List<ValidationEvent> events = model.getShapeIndex().shapes(StructureShape.class)
                 .flatMap(shape -> validateStructure(shape).stream())
                 .collect(Collectors.toList());
+
+        events.addAll(model.getShapeIndex().shapes(MemberShape.class)
+                .flatMap(member -> Trait.flatMapStream(member, HttpHeaderTrait.class))
+                .filter(pair -> BLACKLIST.contains(pair.getRight().getValue().toLowerCase(Locale.US)))
+                .map(pair -> danger(pair.getLeft(), String.format(
+                        "httpHeader cannot be set to `%s`", pair.getRight().getValue()
+                )))
+                .collect(Collectors.toList()));
+
+        return events;
     }
 
     private List<ValidationEvent> validateStructure(StructureShape structure) {

diff --git a/...software/amazon/smithy/model/errorfiles/validators/http-request-response-validator.errors b/...software/amazon/smithy/model/errorfiles/validators/http-request-response-validator.errors
@@ -22,7 +22,7 @@
 [ERROR] ns.foo#KInput: `httpHeader` field name binding conflicts found for the `x-foo` header in the following structure members: `a`, `b` | HttpHeaderTrait
 [ERROR] ns.foo#KInput: `httpQuery` parameter name binding conflicts found for the `foo` parameter in the following structure members: `c`, `d` | HttpQueryTrait
 [ERROR] ns.foo#L: Operation URI, `/k`, conflicts with other operation URIs in the same service: [`ns.foo#K` (/k)] | HttpUriConflict
-[ERROR] ns.foo#MInput$a: Error creating trait `httpHeader`: httpHeader cannot be set to `Authorization` | Model
+[DANGER] ns.foo#MInput$a: httpHeader cannot be set to `Authorization` | HttpHeaderTrait
 [ERROR] ns.foo#NInput$a: This `a` structure member is marked with the `httpLabel` trait, but no corresponding `http` URI label could be found when used as the input of the `ns.foo#N` operation. | HttpLabelTrait
 [ERROR] ns.foo#OInput$a: Members with the `httpLabel` trait must be required. | HttpLabelTrait
 [ERROR] ns.foo#PInput$a: The `a` structure member corresponds to a greedy label when used as the input of the `ns.foo#P` operation. This member targets (integer: `ns.foo#Integer`), but greedy labels must target string shapes. | HttpLabelTrait