Skip to content

Commit

Permalink
Fix race condition in endpoint discovery
Browse files Browse the repository at this point in the history
  • Loading branch information
skmcgrail committed Nov 18, 2021
1 parent 64462b8 commit 1bbd698
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
### SDK Enhancements

### SDK Bugs
* `aws/crr`: Fixed a race condition that caused concurrent calls relying on endpoint discovery to share the same `url.URL` reference in their operation's `http.Request`.
15 changes: 15 additions & 0 deletions aws/crr/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ func (e *Endpoint) GetValidAddress() (WeightedAddress, bool) {
continue
}

we.URL = cloneURL(we.URL)

return we, true
}

Expand Down Expand Up @@ -97,3 +99,16 @@ func BuildEndpointKey(params map[string]*string) string {

return strings.Join(values, ".")
}

func cloneURL(u *url.URL) (clone *url.URL) {
clone = &url.URL{}

*clone = *u

if u.User != nil {
user := *u.User
clone.User = &user
}

return clone
}
87 changes: 87 additions & 0 deletions aws/crr/endpoint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//go:build go1.16
// +build go1.16

package crr

import (
"net/url"
"reflect"
"strconv"
"testing"
)

func Test_cloneURL(t *testing.T) {
tests := []struct {
value *url.URL
wantClone *url.URL
}{
{
value: &url.URL{
Scheme: "https",
Opaque: "foo",
User: nil,
Host: "amazonaws.com",
Path: "/",
RawPath: "/",
ForceQuery: true,
RawQuery: "thing=value",
Fragment: "1234",
RawFragment: "1234",
},
wantClone: &url.URL{
Scheme: "https",
Opaque: "foo",
User: nil,
Host: "amazonaws.com",
Path: "/",
RawPath: "/",
ForceQuery: true,
RawQuery: "thing=value",
Fragment: "1234",
RawFragment: "1234",
},
},
{
value: &url.URL{
Scheme: "https",
Opaque: "foo",
User: url.UserPassword("NOT", "VALID"),
Host: "amazonaws.com",
Path: "/",
RawPath: "/",
ForceQuery: true,
RawQuery: "thing=value",
Fragment: "1234",
RawFragment: "1234",
},
wantClone: &url.URL{
Scheme: "https",
Opaque: "foo",
User: url.UserPassword("NOT", "VALID"),
Host: "amazonaws.com",
Path: "/",
RawPath: "/",
ForceQuery: true,
RawQuery: "thing=value",
Fragment: "1234",
RawFragment: "1234",
},
},
}
for i, tt := range tests {
t.Run(strconv.Itoa(i), func(t *testing.T) {
gotClone := cloneURL(tt.value)
if gotClone == tt.value {
t.Errorf("expct clone URL to not be same pointer address")
}
if tt.value.User != nil {
if tt.value.User == gotClone.User {
t.Errorf("expct cloned Userinfo to not be same pointer address")
}
}
if !reflect.DeepEqual(gotClone, tt.wantClone) {
t.Errorf("cloneURL() = %v, want %v", gotClone, tt.wantClone)
}
})
}
}

0 comments on commit 1bbd698

Please sign in to comment.