From 172af2f58676e3d2cc733bc01f229fa4a27d6ba4 Mon Sep 17 00:00:00 2001 From: Sean McGrail Date: Tue, 7 Jan 2020 09:55:25 -0800 Subject: [PATCH] aws: Add config option to unmarshal API response header maps to normalized lower case map keys (#3033) --- CHANGELOG_PENDING.md | 2 + aws/config.go | 7 ++ aws/signer/v4/header_rules.go | 5 +- aws/signer/v4/v4.go | 3 +- internal/strings/strings.go | 11 ++ internal/strings/strings_test.go | 83 ++++++++++++++ private/model/api/api.go | 23 ++++ private/model/api/load.go | 2 + private/protocol/rest/rest_test.go | 113 ++++++++++++++++++++ private/protocol/rest/unmarshal.go | 13 ++- private/protocol/restjson/unmarshal_test.go | 6 ++ service/s3/api.go | 8 ++ 12 files changed, 267 insertions(+), 9 deletions(-) create mode 100644 internal/strings/strings.go create mode 100644 internal/strings/strings_test.go diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 8a1927a39ca..bbea4cef0bf 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,5 +1,7 @@ ### SDK Features ### SDK Enhancements +* `aws`: Add configuration option enable the SDK to unmarshal API response header maps to normalized lower case map keys. ([#3033](https://github.com/aws/aws-sdk-go/pull/3033)) + * Setting `aws.Config.LowerCaseHeaderMaps` to `true` will result in S3's X-Amz-Meta prefixed header to be unmarshaled to lower case Metadata member's map keys. ### SDK Bugs diff --git a/aws/config.go b/aws/config.go index 869db26614d..2def23fa1d1 100644 --- a/aws/config.go +++ b/aws/config.go @@ -165,6 +165,13 @@ type Config struct { // in the ARN, when an ARN is provided as an argument to a bucket parameter. S3UseARNRegion *bool + // Set this to `true` to enable the SDK to unmarshal API response header maps to + // normalized lower case map keys. + // + // For example S3's X-Amz-Meta prefixed header will be unmarshaled to lower case + // Metadata member's map keys. The value of the header in the map is unaffected. + LowerCaseHeaderMaps *bool + // Set this to `true` to disable the EC2Metadata client from overriding the // default http.Client's Timeout. This is helpful if you do not want the // EC2Metadata client to create a new http.Client. This options is only diff --git a/aws/signer/v4/header_rules.go b/aws/signer/v4/header_rules.go index 244c86da054..07ea799fbd3 100644 --- a/aws/signer/v4/header_rules.go +++ b/aws/signer/v4/header_rules.go @@ -1,8 +1,7 @@ package v4 import ( - "net/http" - "strings" + "github.com/aws/aws-sdk-go/internal/strings" ) // validator houses a set of rule needed for validation of a @@ -61,7 +60,7 @@ type patterns []string // been found func (p patterns) IsValid(value string) bool { for _, pattern := range p { - if strings.HasPrefix(http.CanonicalHeaderKey(value), pattern) { + if strings.HasPrefixFold(value, pattern) { return true } } diff --git a/aws/signer/v4/v4.go b/aws/signer/v4/v4.go index 03b5afb937e..b97334c7f28 100644 --- a/aws/signer/v4/v4.go +++ b/aws/signer/v4/v4.go @@ -608,8 +608,7 @@ func (ctx *signingCtx) buildCanonicalHeaders(r rule, header http.Header) { var headers []string headers = append(headers, "host") for k, v := range header { - canonicalKey := http.CanonicalHeaderKey(k) - if !r.IsValid(canonicalKey) { + if !r.IsValid(k) { continue // ignored header } if ctx.SignedHeaderVals == nil { diff --git a/internal/strings/strings.go b/internal/strings/strings.go new file mode 100644 index 00000000000..d008ae27cb3 --- /dev/null +++ b/internal/strings/strings.go @@ -0,0 +1,11 @@ +package strings + +import ( + "strings" +) + +// HasPrefixFold tests whether the string s begins with prefix, interpreted as UTF-8 strings, +// under Unicode case-folding. +func HasPrefixFold(s, prefix string) bool { + return len(s) >= len(prefix) && strings.EqualFold(s[0:len(prefix)], prefix) +} diff --git a/internal/strings/strings_test.go b/internal/strings/strings_test.go new file mode 100644 index 00000000000..34d1769487f --- /dev/null +++ b/internal/strings/strings_test.go @@ -0,0 +1,83 @@ +// +build go1.7 + +package strings + +import ( + "strings" + "testing" +) + +func TestHasPrefixFold(t *testing.T) { + type args struct { + s string + prefix string + } + tests := map[string]struct { + args args + want bool + }{ + "empty strings and prefix": { + args: args{ + s: "", + prefix: "", + }, + want: true, + }, + "strings starts with prefix": { + args: args{ + s: "some string", + prefix: "some", + }, + want: true, + }, + "prefix longer then string": { + args: args{ + s: "some", + prefix: "some string", + }, + }, + "equal length string and prefix": { + args: args{ + s: "short string", + prefix: "short string", + }, + want: true, + }, + "different cases": { + args: args{ + s: "ShOrT StRING", + prefix: "short", + }, + want: true, + }, + "empty prefix not empty string": { + args: args{ + s: "ShOrT StRING", + prefix: "", + }, + want: true, + }, + "mixed-case prefixes": { + args: args{ + s: "SoMe String", + prefix: "sOme", + }, + want: true, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + if got := HasPrefixFold(tt.args.s, tt.args.prefix); got != tt.want { + t.Errorf("HasPrefixFold() = %v, want %v", got, tt.want) + } + }) + } +} + +func BenchmarkHasPrefixFold(b *testing.B) { + HasPrefixFold("SoME string", "sOmE") +} + +func BenchmarkHasPrefix(b *testing.B) { + strings.HasPrefix(strings.ToLower("SoME string"), strings.ToLower("sOmE")) +} diff --git a/private/model/api/api.go b/private/model/api/api.go index ac92b507bb4..77f1366eea0 100644 --- a/private/model/api/api.go +++ b/private/model/api/api.go @@ -951,6 +951,29 @@ func (a *API) writeInputOutputLocationName() { } } +func (a *API) addHeaderMapDocumentation() { + for _, shape := range a.Shapes { + if !shape.UsedAsOutput { + continue + } + for _, shapeRef := range shape.MemberRefs { + if shapeRef.Location == "headers" { + if dLen := len(shapeRef.Documentation); dLen > 0 { + if shapeRef.Documentation[dLen-1] != '\n' { + shapeRef.Documentation += "\n" + } + shapeRef.Documentation += "//" + } + shapeRef.Documentation += ` +// By default unmarshaled keys are written as a map keys in following canonicalized format: +// the first letter and any letter following a hyphen will be capitalized, and the rest as lowercase. +// Set ` + "`aws.Config.LowerCaseHeaderMaps`" + ` to ` + "`true`" + ` to write unmarshaled keys to the map as lowercase. +` + } + } + } +} + func getDeprecatedMessage(msg string, name string) string { if len(msg) == 0 { return name + " has been deprecated" diff --git a/private/model/api/load.go b/private/model/api/load.go index d4baa0221be..2ad72367123 100644 --- a/private/model/api/load.go +++ b/private/model/api/load.go @@ -225,6 +225,8 @@ func (a *API) Setup() error { return err } + a.addHeaderMapDocumentation() + if !a.NoRemoveUnusedShapes { a.removeUnusedShapes() } diff --git a/private/protocol/rest/rest_test.go b/private/protocol/rest/rest_test.go index 46457b27929..e5b50564d94 100644 --- a/private/protocol/rest/rest_test.go +++ b/private/protocol/rest/rest_test.go @@ -1,9 +1,12 @@ +// +build go1.7 + package rest_test import ( "bytes" "io/ioutil" "net/http" + "reflect" "testing" "github.com/aws/aws-sdk-go/aws" @@ -61,3 +64,113 @@ func TestUnsetHeaders(t *testing.T) { t.Fatal(req.Error) } } + +func TestNormalizedHeaders(t *testing.T) { + cases := map[string]struct { + inputValues map[string]*string + outputValues http.Header + expectedInputHeaders http.Header + expectedOutput map[string]*string + normalize bool + }{ + "non-normalized headers": { + inputValues: map[string]*string{ + "baz": aws.String("bazValue"), + "BAR": aws.String("barValue"), + }, + expectedInputHeaders: http.Header{ + "X-Amz-Meta-Baz": []string{"bazValue"}, + "X-Amz-Meta-Bar": []string{"barValue"}, + }, + outputValues: http.Header{ + "X-Amz-Meta-Baz": []string{"bazValue"}, + "X-Amz-Meta-Bar": []string{"barValue"}, + }, + expectedOutput: map[string]*string{ + "Baz": aws.String("bazValue"), + "Bar": aws.String("barValue"), + }, + }, + "normalized headers": { + inputValues: map[string]*string{ + "baz": aws.String("bazValue"), + "BAR": aws.String("barValue"), + }, + expectedInputHeaders: http.Header{ + "X-Amz-Meta-Baz": []string{"bazValue"}, + "X-Amz-Meta-Bar": []string{"barValue"}, + }, + outputValues: http.Header{ + "X-Amz-Meta-Baz": []string{"bazValue"}, + "X-Amz-Meta-Bar": []string{"barValue"}, + }, + expectedOutput: map[string]*string{ + "baz": aws.String("bazValue"), + "bar": aws.String("barValue"), + }, + normalize: true, + }, + } + + for name, tt := range cases { + t.Run(name, func(t *testing.T) { + cfg := &aws.Config{Region: aws.String("us-west-2"), LowerCaseHeaderMaps: &tt.normalize} + c := unit.Session.ClientConfig("testService", cfg) + svc := client.New( + *cfg, + metadata.ClientInfo{ + ServiceName: "testService", + SigningName: c.SigningName, + SigningRegion: c.SigningRegion, + Endpoint: c.Endpoint, + APIVersion: "", + }, + c.Handlers, + ) + + // Handlers + op := &request.Operation{ + Name: "test-operation", + HTTPPath: "/", + } + + input := &struct { + Foo map[string]*string `location:"headers" locationName:"x-amz-meta-" type:"map"` + }{ + Foo: tt.inputValues, + } + + output := &struct { + Foo map[string]*string `location:"headers" locationName:"x-amz-meta-" type:"map"` + }{} + + req := svc.NewRequest(op, input, output) + req.HTTPResponse = &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBuffer(nil)), + Header: tt.outputValues, + } + + // Build Request + rest.Build(req) + if req.Error != nil { + t.Fatal(req.Error) + } + + if e, a := tt.expectedInputHeaders, req.HTTPRequest.Header; !reflect.DeepEqual(e, a) { + t.Errorf("expected %v, but got %v", e, a) + } + + // unmarshal response + rest.UnmarshalMeta(req) + rest.Unmarshal(req) + if req.Error != nil { + t.Fatal(req.Error) + } + + if e, a := tt.expectedOutput, output.Foo; !reflect.DeepEqual(e, a) { + t.Errorf("expected %v, but got %v", e, a) + } + }) + } +} diff --git a/private/protocol/rest/unmarshal.go b/private/protocol/rest/unmarshal.go index 74e361e070d..777eef0e35c 100644 --- a/private/protocol/rest/unmarshal.go +++ b/private/protocol/rest/unmarshal.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/request" + awsStrings "github.com/aws/aws-sdk-go/internal/strings" "github.com/aws/aws-sdk-go/private/protocol" ) @@ -120,7 +121,7 @@ func unmarshalLocationElements(r *request.Request, v reflect.Value) { } case "headers": prefix := field.Tag.Get("locationName") - err := unmarshalHeaderMap(m, r.HTTPResponse.Header, prefix) + err := unmarshalHeaderMap(m, r.HTTPResponse.Header, prefix, aws.BoolValue(r.Config.LowerCaseHeaderMaps)) if err != nil { r.Error = awserr.New(request.ErrCodeSerialization, "failed to decode REST response", err) break @@ -145,7 +146,7 @@ func unmarshalStatusCode(v reflect.Value, statusCode int) { } } -func unmarshalHeaderMap(r reflect.Value, headers http.Header, prefix string) error { +func unmarshalHeaderMap(r reflect.Value, headers http.Header, prefix string, normalize bool) error { if len(headers) == 0 { return nil } @@ -153,8 +154,12 @@ func unmarshalHeaderMap(r reflect.Value, headers http.Header, prefix string) err case map[string]*string: // we only support string map value types out := map[string]*string{} for k, v := range headers { - k = http.CanonicalHeaderKey(k) - if strings.HasPrefix(strings.ToLower(k), strings.ToLower(prefix)) { + if awsStrings.HasPrefixFold(k, prefix) { + if normalize == true { + k = strings.ToLower(k) + } else { + k = http.CanonicalHeaderKey(k) + } out[k[len(prefix):]] = &v[0] } } diff --git a/private/protocol/restjson/unmarshal_test.go b/private/protocol/restjson/unmarshal_test.go index 5794713ab4a..9c5105a7ff0 100644 --- a/private/protocol/restjson/unmarshal_test.go +++ b/private/protocol/restjson/unmarshal_test.go @@ -1708,8 +1708,14 @@ type OutputService10TestShapeOutputService10TestCaseOperation1Input struct { type OutputService10TestShapeOutputService10TestCaseOperation1Output struct { _ struct{} `type:"structure"` + // By default unmarshaled keys are written as a map keys in following canonicalized format: + // the first letter and any letter following a hyphen will be capitalized, and the rest as lowercase. + // Set `aws.Config.LowerCaseHeaderMaps` to `true` to write unmarshaled keys to the map as lowercase. AllHeaders map[string]*string `location:"headers" type:"map"` + // By default unmarshaled keys are written as a map keys in following canonicalized format: + // the first letter and any letter following a hyphen will be capitalized, and the rest as lowercase. + // Set `aws.Config.LowerCaseHeaderMaps` to `true` to write unmarshaled keys to the map as lowercase. PrefixedHeaders map[string]*string `location:"headers" locationName:"X-" type:"map"` } diff --git a/service/s3/api.go b/service/s3/api.go index 91d61f8a561..df70e4e49eb 100644 --- a/service/s3/api.go +++ b/service/s3/api.go @@ -17692,6 +17692,10 @@ type GetObjectOutput struct { LastModified *time.Time `location:"header" locationName:"Last-Modified" type:"timestamp"` // A map of metadata to store with the object in S3. + // + // By default unmarshaled keys are written as a map keys in following canonicalized format: + // the first letter and any letter following a hyphen will be capitalized, and the rest as lowercase. + // Set `aws.Config.LowerCaseHeaderMaps` to `true` to write unmarshaled keys to the map as lowercase. Metadata map[string]*string `location:"headers" locationName:"x-amz-meta-" type:"map"` // This is set to the number of metadata entries not returned in x-amz-meta @@ -18919,6 +18923,10 @@ type HeadObjectOutput struct { LastModified *time.Time `location:"header" locationName:"Last-Modified" type:"timestamp"` // A map of metadata to store with the object in S3. + // + // By default unmarshaled keys are written as a map keys in following canonicalized format: + // the first letter and any letter following a hyphen will be capitalized, and the rest as lowercase. + // Set `aws.Config.LowerCaseHeaderMaps` to `true` to write unmarshaled keys to the map as lowercase. Metadata map[string]*string `location:"headers" locationName:"x-amz-meta-" type:"map"` // This is set to the number of metadata entries not returned in x-amz-meta