Merge pull request #280 from aws/jasdel/fixup/URLPath

Updates the Smithy SDK client httpbinding serializers to use the JoinPath and JoinRawQuery utilities to ensure paths and query are joined correctly.

Related to # aws/aws-sdk-go-v2#1191

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/SmithyGoDependency.java

@@ -118,7 +118,7 @@ private static GoDependency relativePackage(
     private static final class Versions {
         private static final String GO_STDLIB = "1.15";
         private static final String GO_CMP = "v0.5.4";
-        private static final String SMITHY_GO = "v1.2.0";
+        private static final String SMITHY_GO = "v1.2.1-0.20210330205207-0917d08124fa";
         private static final String GO_JMESPATH = "v0.4.0";
     }
 }

codegen/smithy-go-codegen/src/main/java/software/amazon/smithy/go/codegen/integration/HttpBindingProtocolGenerator.java

@@ -218,13 +218,8 @@ private void generateOperationSerializerMiddleware(GenerationContext context, Op
 
             writer.write("");
             writer.write("opPath, opQuery := httpbinding.SplitURI($S)", httpTrait.getUri());
-            writer.write("request.URL.Path = opPath");
-            writer.openBlock("if len(request.URL.RawQuery) > 0 {", "", () -> {
-                writer.write("request.URL.RawQuery = \"&\" + opQuery");
-                writer.openBlock("} else {", "}", () -> {
-                    writer.write("request.URL.RawQuery = opQuery");
-                });
-            });
+            writer.write("request.URL.Path = smithyhttp.JoinPath(request.URL.Path, opPath)");
+            writer.write("request.URL.RawQuery = smithyhttp.JoinRawQuery(request.URL.RawQuery, opQuery)");
             writer.write("request.Method = $S", httpTrait.getMethod());
             writer.write("restEncoder, err := httpbinding.NewEncoder(request.URL.Path, request.URL.RawQuery, "
                     + "request.Header)");

transport/http/url.go

@@ -0,0 +1,44 @@
+package http
+
+import "strings"
+
+// JoinPath returns an absolute URL path composed of the two paths provided.
+// Enforces that the returned path begins with '/'. If added path is empty the
+// returned path suffix will match the first parameter suffix.
+func JoinPath(a, b string) string {
+	if len(a) == 0 {
+		a = "/"
+	} else if a[0] != '/' {
+		a = "/" + a
+	}
+
+	if len(b) != 0 && b[0] == '/' {
+		b = b[1:]
+	}
+
+	if len(b) != 0 && len(a) > 1 && a[len(a)-1] != '/' {
+		a = a + "/"
+	}
+
+	return a + b
+}
+
+// JoinRawQuery returns an absolute raw query expression. Any duplicate '&'
+// will be collapsed to single separator between values.
+func JoinRawQuery(a, b string) string {
+	a = strings.TrimFunc(a, isAmpersand)
+	b = strings.TrimFunc(b, isAmpersand)
+
+	if len(a) == 0 {
+		return b
+	}
+	if len(b) == 0 {
+		return a
+	}
+
+	return a + "&" + b
+}
+
+func isAmpersand(v rune) bool {
+	return v == '&'
+}

transport/http/url_test.go

@@ -0,0 +1,124 @@
+package http
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestJoinPath(t *testing.T) {
+	cases := []struct {
+		A, B   string
+		Expect string
+	}{
+		0: {
+			A: "", B: "/bar",
+			Expect: "/bar",
+		},
+		1: {
+			A: "/", B: "/bar",
+			Expect: "/bar",
+		},
+		2: {
+			A: "/foo/", B: "/bar",
+			Expect: "/foo/bar",
+		},
+		3: {
+			A: "foo/", B: "/bar",
+			Expect: "/foo/bar",
+		},
+		4: {
+			A: "/foo/", B: "bar",
+			Expect: "/foo/bar",
+		},
+		5: {
+			A: "foo", B: "/bar",
+			Expect: "/foo/bar",
+		},
+		6: {
+			A: "", B: "",
+			Expect: "/",
+		},
+		7: {
+			A: "foo", B: "",
+			Expect: "/foo",
+		},
+		8: {
+			A: "foo/", B: "",
+			Expect: "/foo/",
+		},
+		9: {
+			A: "foo//", B: "//bar",
+			Expect: "/foo///bar",
+		},
+	}
+
+	for i, c := range cases {
+		t.Run(fmt.Sprintf("%d:%s,%s:%s", i, c.A, c.B, c.Expect), func(t *testing.T) {
+			actual := JoinPath(c.A, c.B)
+			if e, a := c.Expect, actual; e != a {
+				t.Errorf("expect %v path, got %v", e, a)
+			}
+		})
+	}
+}
+
+func TestJoinRawQuery(t *testing.T) {
+	cases := []struct {
+		A, B   string
+		Expect string
+	}{
+		0: {
+			A: "", B: "bar",
+			Expect: "bar",
+		},
+		1: {
+			A: "foo", B: "bar",
+			Expect: "foo&bar",
+		},
+		2: {
+			A: "foo&", B: "bar",
+			Expect: "foo&bar",
+		},
+		3: {
+			A: "foo", B: "",
+			Expect: "foo",
+		},
+		4: {
+			A: "", B: "&bar",
+			Expect: "bar",
+		},
+		5: {
+			A: "foo&", B: "&bar",
+			Expect: "foo&bar",
+		},
+		6: {
+			A: "", B: "",
+			Expect: "",
+		},
+		7: {
+			A: "foo&baz", B: "bar",
+			Expect: "foo&baz&bar",
+		},
+		8: {
+			A: "foo", B: "baz&bar",
+			Expect: "foo&baz&bar",
+		},
+		9: {
+			A: "&foo&", B: "&baz&bar&",
+			Expect: "foo&baz&bar",
+		},
+		10: {
+			A: "&foo&&&", B: "&&&baz&&&bar&",
+			Expect: "foo&baz&&&bar",
+		},
+	}
+
+	for i, c := range cases {
+		t.Run(fmt.Sprintf("%d:%s,%s:%s", i, c.A, c.B, c.Expect), func(t *testing.T) {
+			actual := JoinRawQuery(c.A, c.B)
+			if e, a := c.Expect, actual; e != a {
+				t.Errorf("expect %v query, got %v", e, a)
+			}
+		})
+	}
+}