Skip to content

Commit 2e91bf1

Browse files
authored
aws/signer/v4: Fix panic handling of endpoint URL without schemes (#1754)
Fixes #1294 by correcting how GetURIPath parses the Opaque field of a url.URL value. Adds test cases for GetURIPath to validate its behavior.
1 parent 8d6ca65 commit 2e91bf1

File tree

3 files changed

+114
-7
lines changed

3 files changed

+114
-7
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"id": "ed6b34a4-41e6-4b47-affa-11a9f5066b23",
3+
"type": "bugfix",
4+
"description": "aws/signer/v4: Fixes a panic in SDK's handling of endpoint URLs with ports by correcting how URL path is parsed from opaque URLs. Fixes [#1294](https://github.com/aws/aws-sdk-go-v2/issues/1294).",
5+
"modules": [
6+
"."
7+
]
8+
}

aws/signer/internal/v4/util.go

+23-7
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,35 @@ func StripExcessSpaces(str string) string {
4646
return string(buf[:m])
4747
}
4848

49-
// GetURIPath returns the escaped URI component from the provided URL
49+
// GetURIPath returns the escaped URI component from the provided URL.
5050
func GetURIPath(u *url.URL) string {
51-
var uri string
51+
var uriPath string
5252

5353
if len(u.Opaque) > 0 {
54-
uri = "/" + strings.Join(strings.Split(u.Opaque, "/")[3:], "/")
54+
const schemeSep, pathSep, queryStart = "//", "/", "?"
55+
56+
opaque := u.Opaque
57+
// Cut off the query string if present.
58+
if idx := strings.Index(opaque, queryStart); idx >= 0 {
59+
opaque = opaque[:idx]
60+
}
61+
62+
// Cutout the scheme separator if present.
63+
if strings.Index(opaque, schemeSep) == 0 {
64+
opaque = opaque[len(schemeSep):]
65+
}
66+
67+
// capture URI path starting with first path separator.
68+
if idx := strings.Index(opaque, pathSep); idx >= 0 {
69+
uriPath = opaque[idx:]
70+
}
5571
} else {
56-
uri = u.EscapedPath()
72+
uriPath = u.EscapedPath()
5773
}
5874

59-
if len(uri) == 0 {
60-
uri = "/"
75+
if len(uriPath) == 0 {
76+
uriPath = "/"
6177
}
6278

63-
return uri
79+
return uriPath
6480
}

aws/signer/internal/v4/util_test.go

+83
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,92 @@
11
package v4
22

33
import (
4+
"net/http"
5+
"net/url"
46
"testing"
57
)
68

9+
func lazyURLParse(v string) func() (*url.URL, error) {
10+
return func() (*url.URL, error) {
11+
return url.Parse(v)
12+
}
13+
}
14+
15+
func TestGetURIPath(t *testing.T) {
16+
cases := map[string]struct {
17+
getURL func() (*url.URL, error)
18+
expect string
19+
}{
20+
// Cases
21+
"with scheme": {
22+
getURL: lazyURLParse("https://localhost:9000"),
23+
expect: "/",
24+
},
25+
"no port, with scheme": {
26+
getURL: lazyURLParse("https://localhost"),
27+
expect: "/",
28+
},
29+
"without scheme": {
30+
getURL: lazyURLParse("localhost:9000"),
31+
expect: "/",
32+
},
33+
"without scheme, with path": {
34+
getURL: lazyURLParse("localhost:9000/abc123"),
35+
expect: "/abc123",
36+
},
37+
"without scheme, with separator": {
38+
getURL: lazyURLParse("//localhost:9000"),
39+
expect: "/",
40+
},
41+
"no port, without scheme, with separator": {
42+
getURL: lazyURLParse("//localhost"),
43+
expect: "/",
44+
},
45+
"without scheme, with separator, with path": {
46+
getURL: lazyURLParse("//localhost:9000/abc123"),
47+
expect: "/abc123",
48+
},
49+
"no port, without scheme, with separator, with path": {
50+
getURL: lazyURLParse("//localhost/abc123"),
51+
expect: "/abc123",
52+
},
53+
"opaque with query string": {
54+
getURL: lazyURLParse("localhost:9000/abc123?efg=456"),
55+
expect: "/abc123",
56+
},
57+
"failing test": {
58+
getURL: func() (*url.URL, error) {
59+
endpoint := "https://service.region.amazonaws.com"
60+
req, _ := http.NewRequest("POST", endpoint, nil)
61+
u := req.URL
62+
63+
u.Opaque = "//example.org/bucket/key-._~,!@#$%^&*()"
64+
65+
query := u.Query()
66+
query.Set("some-query-key", "value")
67+
u.RawQuery = query.Encode()
68+
69+
return u, nil
70+
},
71+
expect: "/bucket/key-._~,!@#$%^&*()",
72+
},
73+
}
74+
75+
for name, c := range cases {
76+
t.Run(name, func(t *testing.T) {
77+
u, err := c.getURL()
78+
if err != nil {
79+
t.Fatalf("failed to get URL, %v", err)
80+
}
81+
82+
actual := GetURIPath(u)
83+
if e, a := c.expect, actual; e != a {
84+
t.Errorf("expect %v path, got %v", e, a)
85+
}
86+
})
87+
}
88+
}
89+
790
func TestStripExcessHeaders(t *testing.T) {
891
vals := []string{
992
"",

0 commit comments

Comments
 (0)