Repository: smithy-lang/smithy
Pull request #2371 (2024-08-14 21:23:05)

More httpChecksum trait validation for responses
#### Background
This adds validation to [httpChecksum](https://smithy.io/2.0/aws/aws-core.html#aws-protocols-httpchecksum-trait) trait for response checksumming behavior. When configuring the trait for response checksumming, the list of checksum algorithms to consider must be provided. Else, the client behavior would have no algorithms to validate.

The [docs for response validation](https://smithy.io/2.0/aws/aws-core.html#http-response-checksums) say 
> A client MUST use the list of supported algorithms modeled in the responseAlgorithms property to look up the checksum(s) for which validation MUST be performed.

So, this list should not be empty.

#### Testing
Added tests for this case.

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
diff --git a/docs/source-2.0/aws/aws-core.rst b/docs/source-2.0/aws/aws-core.rst
index 12b55e1f1..523eb17b4 100644
--- a/docs/source-2.0/aws/aws-core.rst
+++ b/docs/source-2.0/aws/aws-core.rst
@@ -958,7 +958,8 @@ The ``httpChecksum`` trait is a structure that contains the following members:
 The ``httpChecksum`` trait MUST define at least one of the request checksumming
 behavior, by setting the ``requestAlgorithmMember`` or
 ``requestChecksumRequired`` property, or the response checksumming behavior, by
-setting the ``requestValidationModeMember`` property.
+setting the ``requestValidationModeMember`` and ``responseAlgorithms``
+properties.
 
 The following is an example of the ``httpChecksum`` trait that defines required
 request checksum behavior with support for the "CRC32C", "CRC32", "SHA1", and
diff --git a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java
index 047b38d68..e09a562e4 100644
--- a/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java
+++ b/smithy-aws-traits/src/main/java/software/amazon/smithy/aws/traits/HttpChecksumTraitValidator.java
@@ -74,7 +74,8 @@ public final class HttpChecksumTraitValidator extends AbstractValidator {
                 trait.isRequestChecksumRequired() || trait.getRequestAlgorithmMember().isPresent();
 
         // Response checksum behavior is defined when either the `requestValidationModeMember`
-        // property or `responseAlgorithms` property are modeled.
+        // property or `responseAlgorithms` property are modeled. Actually both are needed, but this check helps do
+        // deeper validation later.
         boolean isResponseChecksumConfiguration =
                 !trait.getResponseAlgorithms().isEmpty() || trait.getRequestValidationModeMember().isPresent();
 
@@ -222,20 +223,25 @@ public final class HttpChecksumTraitValidator extends AbstractValidator {
     ) {
         List<ValidationEvent> events = new ArrayList<>();
 
-        // Check for header binding conflicts with the output shape.
-        model.getShape(operation.getOutputShape()).flatMap(Shape::asStructureShape).ifPresent(outputShape -> {
-            events.addAll(validateHeaderConflicts(operation, outputShape));
-        });
-
         // Validate requestValidationModeMember is set properly for response behavior.
         if (!trait.getRequestValidationModeMember().isPresent()) {
             events.add(error(operation, trait, "The `httpChecksum` trait must model the"
                     + " `requestValidationModeMember` property to support response checksum behavior."));
-            return events;
         } else {
             validateValidationModeMember(model, trait, operation, input).map(events::add);
         }
 
+        // Validate responseAlgorithms is not empty.
+        if (trait.getResponseAlgorithms().isEmpty()) {
+            events.add(error(operation, trait, "The `httpChecksum` trait must model the"
+                    + " `responseAlgorithms` property to support response checksum behavior."));
+        }
+
+        // Check for header binding conflicts with the output shape.
+        model.getShape(operation.getOutputShape()).flatMap(Shape::asStructureShape).ifPresent(outputShape -> {
+            events.addAll(validateHeaderConflicts(operation, outputShape));
+        });
+
         // Check for header binding conflicts with each error shape.
         if (!operation.getErrors().isEmpty()) {
             for (ShapeId id : operation.getErrors()) {


Test patch:
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors
index fb7502a6e..750e98437 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.errors
@@ -1,5 +1,7 @@
 [SUPPRESSED] smithy.example#NoBehavior: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait
 [SUPPRESSED] smithy.example#NoModeForResponse: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait
 [SUPPRESSED] smithy.example#NoOutputForResponse: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait
+[SUPPRESSED] smithy.example#NoResponseAlgorithms: This shape applies a trait that is unstable: aws.protocols#httpChecksum | UnstableTrait
 [ERROR] smithy.example#NoBehavior: The `httpChecksum` trait must define at least one of the `request` or `response` checksum behaviors. | HttpChecksumTrait
 [ERROR] smithy.example#NoModeForResponse: The `httpChecksum` trait must model the `requestValidationModeMember` property to support response checksum behavior. | HttpChecksumTrait
+[ERROR] smithy.example#NoResponseAlgorithms: The `httpChecksum` trait must model the `responseAlgorithms` property to support response checksum behavior. | HttpChecksumTrait
diff --git a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy
index 800cb375a..24ce4cf32 100644
--- a/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy
+++ b/smithy-aws-traits/src/test/resources/software/amazon/smithy/aws/traits/errorfiles/http-checksum-trait.smithy
@@ -37,6 +37,24 @@ structure NoModeForResponseInput {}
 @output
 structure NoModeForResponseOutput {}
 
+@httpChecksum(
+    requestValidationModeMember: "validationMode"
+)
+@suppress(["UnstableTrait"])
+operation NoResponseAlgorithms {
+    input: NoResponseAlgorithmsInput,
+    output: NoResponseAlgorithmsOutput,
+}
+
+@input
+structure NoResponseAlgorithmsInput {
+    validationMode: ValidationMode
+}
+
+@output
+structure NoResponseAlgorithmsOutput {}
+
+
 @httpChecksum(
     requestAlgorithmMember: "requestAlgorithm",
     requestValidationModeMember: "validationMode",