Fix: URI path element replace issue due to elements name overlap (#553)

* fix path replace issue of duplicate key prefix * add more comment to clarify element indexing * remove extra changelog * add error test case --------- Co-authored-by: Tianyi Wang <[email protected]>
aws · Nov 14, 2024 · 84c6c7e · 84c6c7e
1 parent 253cd26
commit 84c6c7e
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 16 deletions.
diff --git a/encoding/httpbinding/path_replace.go b/encoding/httpbinding/path_replace.go
@@ -22,33 +22,33 @@ func bufCap(b []byte, n int) []byte {
 // replacePathElement replaces a single element in the path []byte.
 // Escape is used to control whether the value will be escaped using Amazon path escape style.
 func replacePathElement(path, fieldBuf []byte, key, val string, escape bool) ([]byte, []byte, error) {
-	fieldBuf = bufCap(fieldBuf, len(key)+3) // { <key> [+] }
+	// search for "{<key>}". If not found, search for the greedy version "{<key>+}". If none are found, return error
+	fieldBuf = bufCap(fieldBuf, len(key)+2) // { <key> }
 	fieldBuf = append(fieldBuf, uriTokenStart)
 	fieldBuf = append(fieldBuf, key...)
+	fieldBuf = append(fieldBuf, uriTokenStop)
 
 	start := bytes.Index(path, fieldBuf)
-	end := start + len(fieldBuf)
-	if start < 0 || len(path[end:]) == 0 {
-		// TODO what to do about error?
-		return path, fieldBuf, fmt.Errorf("invalid path index, start=%d,end=%d. %s", start, end, path)
-	}
-
 	encodeSep := true
-	if path[end] == uriTokenSkip {
-		// '+' token means do not escape slashes
+	if start < 0 {
+		fieldBuf = bufCap(fieldBuf, len(key)+3) // { <key> [+] }
+		fieldBuf = append(fieldBuf, uriTokenStart)
+		fieldBuf = append(fieldBuf, key...)
+		fieldBuf = append(fieldBuf, uriTokenSkip)
+		fieldBuf = append(fieldBuf, uriTokenStop)
+
+		start = bytes.Index(path, fieldBuf)
+		if start < 0 {
+			return path, fieldBuf, fmt.Errorf("invalid path index, start=%d. %s", start, path)
+		}
 		encodeSep = false
-		end++
 	}
+	end := start + len(fieldBuf)
 
 	if escape {
 		val = EscapePath(val, encodeSep)
 	}
 
-	if path[end] != uriTokenStop {
-		return path, fieldBuf, fmt.Errorf("invalid path element, does not contain token stop, %s", path)
-	}
-	end++
-
 	fieldBuf = bufCap(fieldBuf, len(val))
 	fieldBuf = append(fieldBuf, val...)
 

diff --git a/encoding/httpbinding/path_replace_test.go b/encoding/httpbinding/path_replace_test.go
@@ -9,6 +9,7 @@ func TestPathReplace(t *testing.T) {
 	cases := []struct {
 		Orig, ExpPath, ExpRawPath []byte
 		Key, Val                  string
+		ExpectErr                 bool
 	}{
 		{
 			Orig:       []byte("/{bucket}/{key+}"),
@@ -40,6 +41,23 @@ func TestPathReplace(t *testing.T) {
 			ExpRawPath: []byte("/reallylongvaluegoesheregrowingarray/{key+}"),
 			Key:        "bucket", Val: "reallylongvaluegoesheregrowingarray",
 		},
+		{
+			Orig:       []byte("/{namespace}/{name}"),
+			ExpPath:    []byte("/{namespace}/value"),
+			ExpRawPath: []byte("/{namespace}/value"),
+			Key:        "name", Val: "value",
+		},
+		{
+			Orig:       []byte("/{name}/{namespace}"),
+			ExpPath:    []byte("/value/{namespace}"),
+			ExpRawPath: []byte("/value/{namespace}"),
+			Key:        "name", Val: "value",
+		},
+		{
+			Orig: []byte("/{namespace}/{name+}"),
+			Key:  "nam", Val: "value",
+			ExpectErr: true,
+		},
 	}
 
 	var buffer [64]byte
@@ -50,8 +68,17 @@ func TestPathReplace(t *testing.T) {
 
 		path, _, err := replacePathElement(c.Orig, buffer[:0], c.Key, c.Val, false)
 		if err != nil {
-			t.Fatalf("expected no error, got %v", err)
+			if !c.ExpectErr {
+				t.Fatalf("expected no error, got %v", err)
+			}
+		} else if c.ExpectErr {
+			t.Fatalf("expect error, got none")
 		}
+
+		if c.ExpectErr {
+			return
+		}
+
 		rawPath, _, err := replacePathElement(origRaw, buffer[:0], c.Key, c.Val, true)
 		if err != nil {
 			t.Fatalf("expected no error, got %v", err)