Skip to content

Commit

Permalink
Use path.Clean to clean sigv4 path.
Browse files Browse the repository at this point in the history
sigv4 package already calls EscapeURL, the proper fix would be to use
path.Clean.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
  • Loading branch information
roidelapluie committed Mar 28, 2022
1 parent 6008c06 commit f1cb4b8
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 10 deletions.
7 changes: 4 additions & 3 deletions sigv4/sigv4.go
Expand Up @@ -20,6 +20,7 @@ import (
"io/ioutil"
"net/http"
"net/textproto"
"path"
"sync"
"time"

Expand Down Expand Up @@ -115,9 +116,9 @@ func (rt *sigV4RoundTripper) RoundTrip(req *http.Request) (*http.Response, error
}()
req.Body = ioutil.NopCloser(seeker)

// Escape URL like documented in AWS documentation.
// https://docs.aws.amazon.com/sdk-for-go/api/aws/signer/v4/#pkg-overview
req.URL.Path = req.URL.EscapedPath()
// Clean path like documented in AWS documentation.
// https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
req.URL.Path = path.Clean(req.URL.Path)

// Clone the request and trim out headers that we don't want to sign.
signReq := req.Clone(req.Context())
Expand Down
16 changes: 9 additions & 7 deletions sigv4/sigv4_test.go
Expand Up @@ -64,7 +64,7 @@ func TestSigV4RoundTripper(t *testing.T) {

cli := &http.Client{Transport: rt}

req, err := http.NewRequest(http.MethodPost, "google.com", strings.NewReader("Hello, world!"))
req, err := http.NewRequest(http.MethodPost, "https://example.com", strings.NewReader("Hello, world!"))
require.NoError(t, err)

_, err = cli.Do(req)
Expand All @@ -78,7 +78,7 @@ func TestSigV4RoundTripper(t *testing.T) {
// Perform the same request but with a header that shouldn't included in the
// signature; validate that the Authorization signature matches.
t.Run("Ignored Headers", func(t *testing.T) {
req, err := http.NewRequest(http.MethodPost, "google.com", strings.NewReader("Hello, world!"))
req, err := http.NewRequest(http.MethodPost, "https://example.com", strings.NewReader("Hello, world!"))
require.NoError(t, err)

req.Header.Add("Uber-Trace-Id", "some-trace-id")
Expand All @@ -91,12 +91,14 @@ func TestSigV4RoundTripper(t *testing.T) {
})

t.Run("Escape URL", func(t *testing.T) {
req, err := http.NewRequest(http.MethodPost, "google.com/test//test", strings.NewReader("Hello, world!"))
req, err := http.NewRequest(http.MethodPost, "https://example.com/test//test", strings.NewReader("Hello, world!"))
require.NoError(t, err)
require.Equal(t, "google.com/test//test", req.URL.Path)
require.Equal(t, "/test//test", req.URL.Path)

// Escape URL and check
req.URL.Path = req.URL.EscapedPath()
require.Equal(t, "google.com/test/test", req.URL.Path)
_, err = cli.Do(req)
require.NoError(t, err)
require.NotNil(t, gotReq)

require.Equal(t, "/test/test", gotReq.URL.Path)
})
}

0 comments on commit f1cb4b8

Please sign in to comment.