Skip to content

Commit

Permalink
aws/signer/v4: Fix panic handling of endpoint URL without schemes
Browse files Browse the repository at this point in the history
Fixes #1294 by correcting how GetURIPath parses the Opaque field
of a url.URL value.

Adds test cases for GetURIPath to validate its behavior.
  • Loading branch information
jasdel committed Aug 4, 2022
1 parent a01a5c4 commit faf9de2
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 7 deletions.
8 changes: 8 additions & 0 deletions .changelog/ed6b34a441e64b47affa11a9f5066b23.json
@@ -0,0 +1,8 @@
{
"id": "ed6b34a4-41e6-4b47-affa-11a9f5066b23",
"type": "bugfix",
"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).",
"modules": [
"."
]
}
30 changes: 23 additions & 7 deletions aws/signer/internal/v4/util.go
Expand Up @@ -46,19 +46,35 @@ func StripExcessSpaces(str string) string {
return string(buf[:m])
}

// GetURIPath returns the escaped URI component from the provided URL
// GetURIPath returns the escaped URI component from the provided URL.
func GetURIPath(u *url.URL) string {
var uri string
var uriPath string

if len(u.Opaque) > 0 {
uri = "/" + strings.Join(strings.Split(u.Opaque, "/")[3:], "/")
const schemeSep, pathSep, queryStart = "//", "/", "?"

opaque := u.Opaque
// Cut off the query string if present.
if idx := strings.Index(opaque, queryStart); idx >= 0 {
opaque = opaque[:idx]
}

// Cutout the scheme separator if present.
if strings.Index(opaque, schemeSep) == 0 {
opaque = opaque[len(schemeSep):]
}

// capture URI path starting with first path separator.
if idx := strings.Index(opaque, pathSep); idx >= 0 {
uriPath = opaque[idx:]
}
} else {
uri = u.EscapedPath()
uriPath = u.EscapedPath()
}

if len(uri) == 0 {
uri = "/"
if len(uriPath) == 0 {
uriPath = "/"
}

return uri
return uriPath
}
83 changes: 83 additions & 0 deletions aws/signer/internal/v4/util_test.go
@@ -1,9 +1,92 @@
package v4

import (
"net/http"
"net/url"
"testing"
)

func lazyURLParse(v string) func() (*url.URL, error) {
return func() (*url.URL, error) {
return url.Parse(v)
}
}

func TestGetURIPath(t *testing.T) {
cases := map[string]struct {
getURL func() (*url.URL, error)
expect string
}{
// Cases
"with scheme": {
getURL: lazyURLParse("https://localhost:9000"),
expect: "/",
},
"no port, with scheme": {
getURL: lazyURLParse("https://localhost"),
expect: "/",
},
"without scheme": {
getURL: lazyURLParse("localhost:9000"),
expect: "/",
},
"without scheme, with path": {
getURL: lazyURLParse("localhost:9000/abc123"),
expect: "/abc123",
},
"without scheme, with separator": {
getURL: lazyURLParse("//localhost:9000"),
expect: "/",
},
"no port, without scheme, with separator": {
getURL: lazyURLParse("//localhost"),
expect: "/",
},
"without scheme, with separator, with path": {
getURL: lazyURLParse("//localhost:9000/abc123"),
expect: "/abc123",
},
"no port, without scheme, with separator, with path": {
getURL: lazyURLParse("//localhost/abc123"),
expect: "/abc123",
},
"opaque with query string": {
getURL: lazyURLParse("localhost:9000/abc123?efg=456"),
expect: "/abc123",
},
"failing test": {
getURL: func() (*url.URL, error) {
endpoint := "https://service.region.amazonaws.com"
req, _ := http.NewRequest("POST", endpoint, nil)
u := req.URL

u.Opaque = "//example.org/bucket/key-._~,!@#$%^&*()"

query := u.Query()
query.Set("some-query-key", "value")
u.RawQuery = query.Encode()

return u, nil
},
expect: "/bucket/key-._~,!@#$%^&*()",
},
}

for name, c := range cases {
t.Run(name, func(t *testing.T) {
u, err := c.getURL()
if err != nil {
t.Fatalf("failed to get URL, %v", err)
}

actual := GetURIPath(u)
if e, a := c.expect, actual; e != a {
t.Errorf("expect %v path, got %v", e, a)
}
})
}
}

func TestStripExcessHeaders(t *testing.T) {
vals := []string{
"",
Expand Down

0 comments on commit faf9de2

Please sign in to comment.