Deprecate url path cleaning during http request serialization

CHANGELOG_PENDING.md

@@ -3,3 +3,7 @@
 ### SDK Enhancements
 
 ### SDK Bugs
+* `rest`: Remove unnecessary path normalization behavior.
+    * This behavior would incorrectly mutate request paths with URI-encoded characters, potentially resulting in misrouted requests.
+* `config`: Deprecate `DisableRestProtocolURICleaning` config setting.
+    * This setting no longer has any effect. REST-protocol paths will now never be normalized after serialization.

aws/config.go

@@ -252,19 +252,8 @@ type Config struct {
 	// and specify a Retryer instead.
 	SleepDelay func(time.Duration)
 
-	// DisableRestProtocolURICleaning will not clean the URL path when making rest protocol requests.
-	// Will default to false. This would only be used for empty directory names in s3 requests.
-	//
-	// Example:
-	//    sess := session.Must(session.NewSession(&aws.Config{
-	//         DisableRestProtocolURICleaning: aws.Bool(true),
-	//    }))
-	//
-	//    svc := s3.New(sess)
-	//    out, err := svc.GetObject(&s3.GetObjectInput {
-	//    	Bucket: aws.String("bucketname"),
-	//    	Key: aws.String("//foo//bar//moo"),
-	//    })
+	// Deprecated: This setting no longer has any effect.
+	// RESTful paths are no longer cleaned after request serialization.
 	DisableRestProtocolURICleaning *bool
 
 	// EnableEndpointDiscovery will allow for endpoint discovery on operations that
@@ -497,13 +486,6 @@ func (c *Config) WithLowerCaseHeaderMaps(t bool) *Config {
 	return c
 }
 
-// WithDisableRestProtocolURICleaning sets a config DisableRestProtocolURICleaning value
-// returning a Config pointer for chaining.
-func (c *Config) WithDisableRestProtocolURICleaning(t bool) *Config {
-	c.DisableRestProtocolURICleaning = &t
-	return c
-}
-
 // MergeIn merges the passed in configs into the existing config object.
 func (c *Config) MergeIn(cfgs ...*Config) {
 	for _, other := range cfgs {
@@ -608,10 +590,6 @@ func mergeInConfig(dst *Config, other *Config) {
 		dst.SleepDelay = other.SleepDelay
 	}
 
-	if other.DisableRestProtocolURICleaning != nil {
-		dst.DisableRestProtocolURICleaning = other.DisableRestProtocolURICleaning
-	}
-
 	if other.EnforceShouldRetryCheck != nil {
 		dst.EnforceShouldRetryCheck = other.EnforceShouldRetryCheck
 	}

aws/config_test.go

@@ -49,22 +49,21 @@ func TestCopyReturnsNewInstance(t *testing.T) {
 var mergeTestZeroValueConfig = Config{}
 
 var mergeTestConfig = Config{
-	Credentials:                    testCredentials,
-	Endpoint:                       String("MergeTestEndpoint"),
-	Region:                         String("MERGE_TEST_AWS_REGION"),
-	DisableSSL:                     Bool(true),
-	HTTPClient:                     http.DefaultClient,
-	LogLevel:                       LogLevel(LogDebug),
-	Logger:                         NewDefaultLogger(),
-	MaxRetries:                     Int(10),
-	DisableParamValidation:         Bool(true),
-	DisableComputeChecksums:        Bool(true),
-	DisableEndpointHostPrefix:      Bool(true),
-	EnableEndpointDiscovery:        Bool(true),
-	EnforceShouldRetryCheck:        Bool(true),
-	DisableRestProtocolURICleaning: Bool(true),
-	S3ForcePathStyle:               Bool(true),
-	LowerCaseHeaderMaps:            Bool(true),
+	Credentials:               testCredentials,
+	Endpoint:                  String("MergeTestEndpoint"),
+	Region:                    String("MERGE_TEST_AWS_REGION"),
+	DisableSSL:                Bool(true),
+	HTTPClient:                http.DefaultClient,
+	LogLevel:                  LogLevel(LogDebug),
+	Logger:                    NewDefaultLogger(),
+	MaxRetries:                Int(10),
+	DisableParamValidation:    Bool(true),
+	DisableComputeChecksums:   Bool(true),
+	DisableEndpointHostPrefix: Bool(true),
+	EnableEndpointDiscovery:   Bool(true),
+	EnforceShouldRetryCheck:   Bool(true),
+	S3ForcePathStyle:          Bool(true),
+	LowerCaseHeaderMaps:       Bool(true),
 }
 
 var mergeTests = []struct {

private/protocol/rest/build.go

@@ -9,7 +9,6 @@ import (
 	"math"
 	"net/http"
 	"net/url"
-	"path"
 	"reflect"
 	"strconv"
 	"strings"
@@ -134,9 +133,6 @@ func buildLocationElements(r *request.Request, v reflect.Value, buildGETQuery bo
 	}
 
 	r.HTTPRequest.URL.RawQuery = query.Encode()
-	if !aws.BoolValue(r.Config.DisableRestProtocolURICleaning) {
-		cleanPath(r.HTTPRequest.URL)
-	}
 }
 
 func buildBody(r *request.Request, v reflect.Value) {
@@ -244,19 +240,6 @@ func buildQueryString(query url.Values, v reflect.Value, name string, tag reflec
 	return nil
 }
 
-func cleanPath(u *url.URL) {
-	hasSlash := strings.HasSuffix(u.Path, "/")
-
-	// clean up path, removing duplicate `/`
-	u.Path = path.Clean(u.Path)
-	u.RawPath = path.Clean(u.RawPath)
-
-	if hasSlash && !strings.HasSuffix(u.Path, "/") {
-		u.Path += "/"
-		u.RawPath += "/"
-	}
-}
-
 // EscapePath escapes part of a URL path in Amazon style
 func EscapePath(path string, encodeSep bool) string {
 	var buf bytes.Buffer

private/protocol/rest/build_test.go

@@ -14,17 +14,55 @@ import (
 	"github.com/aws/aws-sdk-go/aws/request"
 )
 
-func TestCleanPath(t *testing.T) {
-	uri := &url.URL{
-		Path:   "//foo//bar",
-		Scheme: "https",
-		Host:   "host",
+func TestBuildURI(t *testing.T) {
+	cases := map[string]struct {
+		Topic         *string
+		ExpectPath    string
+		ExpectRawPath string
+	}{
+		"path without prefix slash": {
+			Topic:         aws.String("devices/123/test"),
+			ExpectPath:    "/topics/devices/123/test",
+			ExpectRawPath: "/topics/devices%2F123%2Ftest",
+		},
+		"path containing single prefix slashes": {
+			Topic:         aws.String("/devices/123/test"),
+			ExpectPath:    "/topics//devices/123/test",
+			ExpectRawPath: "/topics/%2Fdevices%2F123%2Ftest",
+		},
+		"path containing prefix multi-slashes": {
+			Topic:         aws.String("///devices/123/test"),
+			ExpectPath:    "/topics////devices/123/test",
+			ExpectRawPath: "/topics/%2F%2F%2Fdevices%2F123%2Ftest",
+		},
 	}
-	cleanPath(uri)
 
-	expected := "https://host/foo/bar"
-	if a, e := uri.String(), expected; a != e {
-		t.Errorf("expect %q URI, got %q", e, a)
+	for _, c := range cases {
+		uri := &url.URL{
+			Path:   "/topics/{topic}",
+			Scheme: "https",
+			Host:   "host",
+		}
+
+		req := &request.Request{
+			HTTPRequest: &http.Request{
+				URL: uri,
+			},
+			Params: &struct {
+				Topic *string `location:"uri" locationName:"topic" type:"string" required:"true"`
+			}{
+				c.Topic,
+			},
+		}
+
+		Build(req)
+
+		if a, e := uri.Path, c.ExpectPath; a != e {
+			t.Errorf("expect %q Path, got %q", e, a)
+		}
+		if a, e := uri.RawPath, c.ExpectRawPath; a != e {
+			t.Errorf("expect %q RawPath, got %q", e, a)
+		}
 	}
 }