From 79b5f39f488c30f60530d58fd9644aeb0427de32 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte <961963+jasdel@users.noreply.github.com> Date: Tue, 1 Mar 2022 14:08:49 -0800 Subject: [PATCH 1/2] service/internal/checksum: Fix opt-in to checksum output validation 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 --- .../checksum/middleware_validate_output.go | 5 ++ .../middleware_validate_output_test.go | 58 +++++++++++++++++-- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/service/internal/checksum/middleware_validate_output.go b/service/internal/checksum/middleware_validate_output.go index 5ca5d49880f..9fde12d86d7 100644 --- 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 index 7f286b75615..e6cf7054e85 100644 --- 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 != "" { From 99706b7db62d58d988a807d4df5a44af74941472 Mon Sep 17 00:00:00 2001 From: Jason Del Ponte <961963+jasdel@users.noreply.github.com> Date: Tue, 1 Mar 2022 14:21:24 -0800 Subject: [PATCH 2/2] add changelog --- .changelog/239ee1b10e4041c484cf14e5e4c25337.json | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changelog/239ee1b10e4041c484cf14e5e4c25337.json diff --git a/.changelog/239ee1b10e4041c484cf14e5e4c25337.json b/.changelog/239ee1b10e4041c484cf14e5e4c25337.json new file mode 100644 index 00000000000..66e92fa01d9 --- /dev/null +++ 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" + ] +} \ No newline at end of file