net/url: warn that JoinPath arguments should be escaped

Fixes #75799

Change-Id: I483f7b1129799d8dd7f359a04e6ebc1b6a5d0b08
Reviewed-on: https://go-review.googlesource.com/c/go/+/719000
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/net/url/url.go b/src/net/url/url.go
index 6dc57eab85dd5cb740d4d828d76de2d1278db365..3acd202c2477f64a709fc1167cb20b9db153d5ba 100644 (file)
--- a/src/net/url/url.go
+++ b/src/net/url/url.go
@@ -1205,7 +1205,13 @@ func (u *URL) UnmarshalBinary(text []byte) error {
 // JoinPath returns a new [URL] with the provided path elements joined to
 // any existing path and the resulting path cleaned of any ./ or ../ elements.
 // Any sequences of multiple / characters will be reduced to a single /.
+// Path elements must already be in escaped form, as produced by [PathEscape].
 func (u *URL) JoinPath(elem ...string) *URL {
+       url, _ := u.joinPath(elem...)
+       return url
+}
+
+func (u *URL) joinPath(elem ...string) (*URL, error) {
        elem = append([]string{u.EscapedPath()}, elem...)
        var p string
        if !strings.HasPrefix(elem[0], "/") {
@@ -1222,8 +1228,8 @@ func (u *URL) JoinPath(elem ...string) *URL {
                p += "/"
        }
        url := *u
-       url.setPath(p)
-       return &url
+       err := url.setPath(p)
+       return &url, err
 }
 
 // validUserinfo reports whether s is a valid userinfo string per RFC 3986
@@ -1281,11 +1287,15 @@ func stringContainsCTLByte(s string) bool {
 
 // JoinPath returns a [URL] string with the provided path elements joined to
 // the existing path of base and the resulting path cleaned of any ./ or ../ elements.
+// Path elements must already be in escaped form, as produced by [PathEscape].
 func JoinPath(base string, elem ...string) (result string, err error) {
        url, err := Parse(base)
        if err != nil {
                return
        }
-       result = url.JoinPath(elem...).String()
-       return
+       res, err := url.joinPath(elem...)
+       if err != nil {
+               return "", err
+       }
+       return res.String(), nil
 }

diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go
index 3ced91fc4c08368a55710a288e0331d5c226ebc9..bb48bb6bee627eb2cade9c2202dd9e0d63841a3c 100644 (file)
--- a/src/net/url/url_test.go
+++ b/src/net/url/url_test.go
@@ -2159,11 +2159,6 @@ func TestJoinPath(t *testing.T) {
                        elem: []string{"../go"},
                        out:  "https://go.googlesource.com/go",
                },
-               {
-                       base: "https://go.googlesource.com",
-                       elem: []string{"../go"},
-                       out:  "https://go.googlesource.com/go",
-               },
                {
                        base: "https://go.googlesource.com",
                        elem: []string{"../go", "../../go", "../../../go"},
@@ -2228,6 +2223,10 @@ func TestJoinPath(t *testing.T) {
                        elem: []string{"/go"},
                        out:  "https://go.googlesource.com/a/b/go",
                },
+               {
+                       base: "https://go.googlesource.com/",
+                       elem: []string{"100%"},
+               },
                {
                        base: "/",
                        elem: nil,
@@ -2269,17 +2268,25 @@ func TestJoinPath(t *testing.T) {
                if tt.out == "" {
                        wantErr = "non-nil error"
                }
-               if out, err := JoinPath(tt.base, tt.elem...); out != tt.out || (err == nil) != (tt.out != "") {
+               out, err := JoinPath(tt.base, tt.elem...)
+               if out != tt.out || (err == nil) != (tt.out != "") {
                        t.Errorf("JoinPath(%q, %q) = %q, %v, want %q, %v", tt.base, tt.elem, out, err, tt.out, wantErr)
                }
-               var out string
+
                u, err := Parse(tt.base)
-               if err == nil {
-                       u = u.JoinPath(tt.elem...)
-                       out = u.String()
+               if err != nil {
+                       if tt.out != "" {
+                               t.Errorf("Parse(%q) = %v", tt.base, err)
+                       }
+                       continue
                }
-               if out != tt.out || (err == nil) != (tt.out != "") {
-                       t.Errorf("Parse(%q).JoinPath(%q) = %q, %v, want %q, %v", tt.base, tt.elem, out, err, tt.out, wantErr)
+               if tt.out == "" {
+                       // URL.JoinPath doesn't return an error, so leave it unchanged
+                       tt.out = tt.base
+               }
+               out = u.JoinPath(tt.elem...).String()
+               if out != tt.out {
+                       t.Errorf("Parse(%q).JoinPath(%q) = %q, want %q", tt.base, tt.elem, out, tt.out)
                }
        }
 }