private/protocol/rest: Use RawPath instead of Opaque (aws#993)

It was discovered the bug golang/go#16847 prevents usage of AWS services using HTTP 2, such as AWS X-Ray with the AWS SDK for Go because Go 1.6.2 - 1.7.4 does not correctly build HTTP2 request when the URL uses Opaque to set the escaped version of the URL path. This has been fixed int he Go 1.8 branch, but this change will ensure users using the current version of Go will be able to connect to AWS services using HTTP2.

With this change the SDK will no longer with with the unsupported Go version 1.4.

.travis.yml

@@ -3,13 +3,12 @@ language: go
 sudo: false
 
 go:
-  - 1.4
   - 1.5
   - 1.6
   - 1.7
   - tip
 
-# Use Go 1.5's vendoring experiment for 1.5 tests. 1.4 tests will use the tip of the dependencies repo.
+# Use Go 1.5's vendoring experiment for 1.5 tests.
 env:
   - GO15VENDOREXPERIMENT=1
 

Makefile

@@ -70,14 +70,7 @@ smoke-tests: get-deps-tests
 performance: get-deps-tests
 	AWS_TESTING_LOG_RESULTS=${log-detailed} AWS_TESTING_REGION=$(region) AWS_TESTING_DB_TABLE=$(table) gucumber -go-tags "integration" ./awstesting/performance
 
-sandbox-tests: sandbox-test-go14 sandbox-test-go15 sandbox-test-go15-novendorexp sandbox-test-go16 sandbox-test-go17 sandbox-test-go18 sandbox-test-gotip
-
-sandbox-build-go14:
-	docker build -f ./awstesting/sandbox/Dockerfile.test.go1.4 -t "aws-sdk-go-1.4" .
-sandbox-go14: sandbox-build-go14
-	docker run -i -t aws-sdk-go-1.4 bash
-sandbox-test-go14: sandbox-build-go14
-	docker run -t aws-sdk-go-1.4
+sandbox-tests: sandbox-test-go15 sandbox-test-go15-novendorexp sandbox-test-go16 sandbox-test-go17 sandbox-test-go18 sandbox-test-gotip
 
 sandbox-build-go15:
 	docker build -f ./awstesting/sandbox/Dockerfile.test.go1.5 -t "aws-sdk-go-1.5" .

aws/signer/v4/functional_1_5_test.go

@@ -38,3 +38,30 @@ func TestStandaloneSign(t *testing.T) {
 		assert.Equal(t, c.EscapedURI, req.URL.EscapedPath())
 	}
 }
+
+func TestStandaloneSign_RawPath(t *testing.T) {
+	creds := unit.Session.Config.Credentials
+	signer := v4.NewSigner(creds)
+
+	for _, c := range standaloneSignCases {
+		host := fmt.Sprintf("https://%s.%s.%s.amazonaws.com",
+			c.SubDomain, c.Region, c.Service)
+
+		req, err := http.NewRequest("GET", host, nil)
+		assert.NoError(t, err)
+
+		// URL.EscapedPath() will be used by the signer to get the
+		// escaped form of the request's URI path.
+		req.URL.Path = c.OrigURI
+		req.URL.RawPath = c.EscapedURI
+		req.URL.RawQuery = c.OrigQuery
+
+		_, err = signer.Sign(req, nil, c.Service, c.Region, time.Unix(0, 0))
+		assert.NoError(t, err)
+
+		actual := req.Header.Get("Authorization")
+		assert.Equal(t, c.ExpSig, actual)
+		assert.Equal(t, c.OrigURI, req.URL.Path)
+		assert.Equal(t, c.EscapedURI, req.URL.EscapedPath())
+	}
+}

aws/signer/v4/v4.go

@@ -42,6 +42,14 @@
 // the URL.Opaque or URL.RawPath. The SDK will use URL.Opaque first and then
 // call URL.EscapedPath() if Opaque is not set.
 //
+// If signing a request intended for HTTP2 server, and you're using Go 1.6.2
+// through 1.7.4 you should use the URL.RawPath as the pre-escaped form of the
+// request URL. https://github.com/golang/go/issues/16847 points to a bug in
+// Go pre 1.8 that failes to make HTTP2 requests using absolute URL in the HTTP
+// message. URL.Opaque generally will force Go to make requests with absolute URL.
+// URL.RawPath does not do this, but RawPath must be a valid escaping of Path
+// or url.EscapedPath will ignore the RawPath escaping.
+//
 // Test `TestStandaloneSign` provides a complete example of using the signer
 // outside of the SDK and pre-escaping the URI path.
 package v4

private/protocol/rest/build.go

@@ -65,6 +65,11 @@ func BuildAsGET(r *request.Request) {
 func buildLocationElements(r *request.Request, v reflect.Value, buildGETQuery bool) {
 	query := r.HTTPRequest.URL.Query()
 
+	// Setup the raw path to match the base path pattern. This is needed
+	// so that when the path is mutated a custom escaped version can be
+	// stored in RawPath that will be used by the Go client.
+	r.HTTPRequest.URL.RawPath = r.HTTPRequest.URL.Path
+
 	for i := 0; i < v.NumField(); i++ {
 		m := v.Field(i)
 		if n := v.Type().Field(i).Name; n[0:1] == strings.ToLower(n[0:1]) {
@@ -107,7 +112,9 @@ func buildLocationElements(r *request.Request, v reflect.Value, buildGETQuery bo
 	}
 
 	r.HTTPRequest.URL.RawQuery = query.Encode()
-	updatePath(r.HTTPRequest.URL, r.HTTPRequest.URL.Path, aws.BoolValue(r.Config.DisableRestProtocolURICleaning))
+	if !aws.BoolValue(r.Config.DisableRestProtocolURICleaning) {
+		cleanPath(r.HTTPRequest.URL)
+	}
 }
 
 func buildBody(r *request.Request, v reflect.Value) {
@@ -171,10 +178,11 @@ func buildURI(u *url.URL, v reflect.Value, name string) error {
 		return awserr.New("SerializationError", "failed to encode REST request", err)
 	}
 
-	uri := u.Path
-	uri = strings.Replace(uri, "{"+name+"}", EscapePath(value, true), -1)
-	uri = strings.Replace(uri, "{"+name+"+}", EscapePath(value, false), -1)
-	u.Path = uri
+	u.Path = strings.Replace(u.Path, "{"+name+"}", value, -1)
+	u.Path = strings.Replace(u.Path, "{"+name+"+}", value, -1)
+
+	u.RawPath = strings.Replace(u.RawPath, "{"+name+"}", EscapePath(value, true), -1)
+	u.RawPath = strings.Replace(u.RawPath, "{"+name+"+}", EscapePath(value, false), -1)
 
 	return nil
 }
@@ -208,27 +216,17 @@ func buildQueryString(query url.Values, v reflect.Value, name string) error {
 	return nil
 }
 
-func updatePath(url *url.URL, urlPath string, disableRestProtocolURICleaning bool) {
-	scheme, query := url.Scheme, url.RawQuery
+func cleanPath(u *url.URL) {
+	hasSlash := strings.HasSuffix(u.Path, "/")
 
-	hasSlash := strings.HasSuffix(urlPath, "/")
+	// clean up path, removing duplicate `/`
+	u.Path = path.Clean(u.Path)
+	u.RawPath = path.Clean(u.RawPath)
 
-	// clean up path
-	if !disableRestProtocolURICleaning {
-		urlPath = path.Clean(urlPath)
+	if hasSlash && !strings.HasSuffix(u.Path, "/") {
+		u.Path += "/"
+		u.RawPath += "/"
 	}
-	if hasSlash && !strings.HasSuffix(urlPath, "/") {
-		urlPath += "/"
-	}
-
-	// get formatted URL minus scheme so we can build this into Opaque
-	url.Scheme, url.Path, url.RawQuery = "", "", ""
-	s := url.String()
-	url.Scheme = scheme
-	url.RawQuery = query
-
-	// build opaque URI
-	url.Opaque = s + urlPath
 }
 
 // EscapePath escapes part of a URL path in Amazon style

private/protocol/rest/build_test.go

@@ -1,30 +1,63 @@
 package rest
 
 import (
+	"net/http"
 	"net/url"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/request"
 )
 
-func TestUpdatePathWithRaw(t *testing.T) {
+func TestCleanPath(t *testing.T) {
 	uri := &url.URL{
+		Path:   "//foo//bar",
 		Scheme: "https",
 		Host:   "host",
 	}
-	updatePath(uri, "//foo//bar", true)
+	cleanPath(uri)
 
-	expected := "https://host//foo//bar"
-	assert.Equal(t, expected, uri.String())
+	expected := "https://host/foo/bar"
+	if a, e := uri.String(), expected; a != e {
+		t.Errorf("expect %q URI, got %q", e, a)
+	}
 }
 
-func TestUpdatePathNoRaw(t *testing.T) {
-	uri := &url.URL{
-		Scheme: "https",
-		Host:   "host",
+func TestMarshalPath(t *testing.T) {
+	in := struct {
+		Bucket *string `location:"uri" locationName:"bucket"`
+		Key    *string `location:"uri" locationName:"key"`
+	}{
+		Bucket: aws.String("mybucket"),
+		Key:    aws.String("my/cool+thing space/object世界"),
+	}
+
+	expectURL := `/mybucket/my/cool+thing space/object世界`
+	expectEscapedURL := `/mybucket/my/cool%2Bthing%20space/object%E4%B8%96%E7%95%8C`
+
+	req := &request.Request{
+		HTTPRequest: &http.Request{
+			URL: &url.URL{Scheme: "https", Host: "exmaple.com", Path: "/{bucket}/{key+}"},
+		},
+		Params: &in,
+	}
+
+	Build(req)
+
+	if req.Error != nil {
+		t.Fatalf("unexpected error, %v", req.Error)
+	}
+
+	if a, e := req.HTTPRequest.URL.Path, expectURL; a != e {
+		t.Errorf("expect %q URI, got %q", e, a)
+	}
+
+	if a, e := req.HTTPRequest.URL.RawPath, expectEscapedURL; a != e {
+		t.Errorf("expect %q escaped URI, got %q", e, a)
+	}
+
+	if a, e := req.HTTPRequest.URL.EscapedPath(), expectEscapedURL; a != e {
+		t.Errorf("expect %q escaped URI, got %q", e, a)
 	}
-	updatePath(uri, "//foo//bar", false)
 
-	expected := "https://host/foo/bar"
-	assert.Equal(t, expected, uri.String())
 }

service/route53/customizations.go

@@ -25,6 +25,6 @@ func init() {
 var reSanitizeURL = regexp.MustCompile(`\/%2F\w+%2F`)
 
 func sanitizeURL(r *request.Request) {
-	r.HTTPRequest.URL.Opaque =
-		reSanitizeURL.ReplaceAllString(r.HTTPRequest.URL.Opaque, "/")
+	r.HTTPRequest.URL.RawPath =
+		reSanitizeURL.ReplaceAllString(r.HTTPRequest.URL.RawPath, "/")
 }