service/internal/checksum: Fix opt-in to checksum output validation (#…

…1607) Updates the SDK's checksum validation logic to require opt-in to output response payload validation. The SDK was always preforming output response payload checksum validation, not respecting the output validation model option. Fixes #1606
aws · Mar 7, 2022 · dfc3d8c · dfc3d8c
1 parent e643282
commit dfc3d8c
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 4 deletions.
diff --git a/.changelog/239ee1b10e4041c484cf14e5e4c25337.json b/.changelog/239ee1b10e4041c484cf14e5e4c25337.json
@@ -0,0 +1,8 @@
+{
+    "id": "239ee1b1-0e40-41c4-84cf-14e5e4c25337",
+    "type": "feature",
+    "description": " Updates the SDK's checksum validation logic to require opt-in to output response payload validation. The SDK was always preforming output response payload checksum validation, not respecting the output validation model option. Fixes [#1606](https://github.com/aws/aws-sdk-go-v2/issues/1606)",
+    "modules": [
+        "service/internal/checksum"
+    ]
+}
diff --git a/service/internal/checksum/middleware_validate_output.go b/service/internal/checksum/middleware_validate_output.go
@@ -66,6 +66,11 @@ func (m *validateOutputPayloadChecksum) HandleDeserialize(
 		return out, metadata, err
 	}
 
+	// If there is no validation mode specified nothing is supported.
+	if mode := getContextOutputValidationMode(ctx); mode != "ENABLED" {
+		return out, metadata, err
+	}
+
 	response, ok := out.RawResponse.(*smithyhttp.Response)
 	if !ok {
 		return out, metadata, &smithy.DeserializationError{

diff --git a/service/internal/checksum/middleware_validate_output_test.go b/service/internal/checksum/middleware_validate_output_test.go
@@ -23,6 +23,7 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 	cases := map[string]struct {
 		response                 *smithyhttp.Response
 		validateOptions          func(*validateOutputPayloadChecksum)
+		modifyContext            func(context.Context) context.Context
 		expectHaveAlgorithmsUsed bool
 		expectAlgorithmsUsed     []string
 		expectErr                string
@@ -31,6 +32,9 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 		expectPayload            []byte
 	}{
 		"success": {
+			modifyContext: func(ctx context.Context) context.Context {
+				return setContextOutputValidationMode(ctx, "ENABLED")
+			},
 			response: &smithyhttp.Response{
 				Response: &http.Response{
 					StatusCode: 200,
@@ -47,6 +51,9 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 			expectPayload:            []byte("hello world"),
 		},
 		"failure": {
+			modifyContext: func(ctx context.Context) context.Context {
+				return setContextOutputValidationMode(ctx, "ENABLED")
+			},
 			response: &smithyhttp.Response{
 				Response: &http.Response{
 					StatusCode: 200,
@@ -61,6 +68,9 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 			expectReadErr: "checksum did not match",
 		},
 		"read error": {
+			modifyContext: func(ctx context.Context) context.Context {
+				return setContextOutputValidationMode(ctx, "ENABLED")
+			},
 			response: &smithyhttp.Response{
 				Response: &http.Response{
 					StatusCode: 200,
@@ -75,6 +85,9 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 			expectReadErr: "some read error",
 		},
 		"unsupported algorithm": {
+			modifyContext: func(ctx context.Context) context.Context {
+				return setContextOutputValidationMode(ctx, "ENABLED")
+			},
 			response: &smithyhttp.Response{
 				Response: &http.Response{
 					StatusCode: 200,
@@ -89,7 +102,39 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 			expectLogged:  "no supported checksum",
 			expectPayload: []byte("hello world"),
 		},
+		"no output validation model": {
+			response: &smithyhttp.Response{
+				Response: &http.Response{
+					StatusCode: 200,
+					Header: func() http.Header {
+						h := http.Header{}
+						return h
+					}(),
+					Body: ioutil.NopCloser(strings.NewReader("hello world")),
+				},
+			},
+			expectPayload: []byte("hello world"),
+		},
+		"unknown output validation model": {
+			modifyContext: func(ctx context.Context) context.Context {
+				return setContextOutputValidationMode(ctx, "something else")
+			},
+			response: &smithyhttp.Response{
+				Response: &http.Response{
+					StatusCode: 200,
+					Header: func() http.Header {
+						h := http.Header{}
+						return h
+					}(),
+					Body: ioutil.NopCloser(strings.NewReader("hello world")),
+				},
+			},
+			expectPayload: []byte("hello world"),
+		},
 		"success ignore multipart checksum": {
+			modifyContext: func(ctx context.Context) context.Context {
+				return setContextOutputValidationMode(ctx, "ENABLED")
+			},
 			response: &smithyhttp.Response{
 				Response: &http.Response{
 					StatusCode: 200,
@@ -109,6 +154,9 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 			expectPayload:            []byte("hello world"),
 		},
 		"success skip ignore multipart checksum": {
+			modifyContext: func(ctx context.Context) context.Context {
+				return setContextOutputValidationMode(ctx, "ENABLED")
+			},
 			response: &smithyhttp.Response{
 				Response: &http.Response{
 					StatusCode: 200,
@@ -136,6 +184,10 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 					fmt.Fprintf(&logged, format, v...)
 				}))
 
+			if c.modifyContext != nil {
+				ctx = c.modifyContext(ctx)
+			}
+
 			validateOutput := validateOutputPayloadChecksum{
 				Algorithms: []Algorithm{
 					AlgorithmSHA1, AlgorithmCRC32, AlgorithmCRC32C,
@@ -187,10 +239,8 @@ func TestValidateOutputPayloadChecksum(t *testing.T) {
 				return
 			}
 
-			if c.expectLogged != "" {
-				if e, a := c.expectLogged, logged.String(); !strings.Contains(a, e) {
-					t.Errorf("expected %q logged in:\n%s", e, a)
-				}
+			if e, a := c.expectLogged, logged.String(); !strings.Contains(a, e) || !((e == "") == (a == "")) {
+				t.Errorf("expected %q logged in:\n%s", e, a)
 			}
 
 			if diff := cmp.Diff(string(c.expectPayload), string(actualPayload)); diff != "" {