From 84fcaf2d5a6652f1c527f64c2be7f258166cd2c7 Mon Sep 17 00:00:00 2001 From: Jaykumar Gosar <5666661+gosar@users.noreply.github.com> Date: Fri, 16 Aug 2024 10:11:59 -0700 Subject: [PATCH] More httpChecksum trait validation for responses (#2371) --- docs/source-2.0/aws/aws-core.rst | 3 ++- .../traits/HttpChecksumTraitValidator.java | 20 ++++++++++++------- .../errorfiles/http-checksum-trait.errors | 2 ++ .../errorfiles/http-checksum-trait.smithy | 18 +++++++++++++++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/docs/source-2.0/aws/aws-core.rst b/docs/source-2.0/aws/aws-core.rst index 12b55e1f10d..523eb17b48f 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 047b38d6815..e09a562e451 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 @@ private List validateOperation(Model model, OperationShape oper 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 @@ private List validateResponseChecksumConfiguration( ) { List 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()) { 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 fb7502a6eb9..750e98437f0 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 800cb375ace..24ce4cf32c6 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",