net/url: `JoinPath` may silently drop unescaped path

[ShoshinNikita]

go version: go1.24, go1.25, dev

What did you do?

// Go Playground - https://go.dev/play/p/joiLsJ4cjNh?v=gotip

func main() {
  u, err := url.JoinPath("/qwerty", "a", "b", "100%")
  if err != nil {
    panic(err)
  }
  fmt.Println(u)
  // Expected: /qwerty/a/b/100%25
  // Got:      /qwerty
}

This behavior is caused by (*URL).JoinPath ignoring an error returned by (*URL).setPath - invalid URL escape "%" in this case.

func (u *URL) JoinPath(elem ...string) *URL {
	...
	url := *u
	url.setPath(p) // <--
	return &url
}

Potential fix

I have a potential fix for this bug:

func (u *URL) JoinPath(elem ...string) *URL {
+	for i := range elem {
+		if _, err := unescape(elem[i], encodePath); err != nil {
+			elem[i] = escape(elem[i], encodePath)
+		}
+	}
+
	elem = append([]string{u.EscapedPath()}, elem...)
	...
	return &url
}

It passes all existing tests in TestJoinPath, plus a new ones:

{
	base: "/",
	elem: []string{"a", "b", "100%"},
	out:  "/a/b/100%25",
},
{
	base: "/",
	elem: []string{"/a/b/100%"},
	out:  "/a/b/100%25",
},

Though, I am not sure whether this fix breaks any existing code or has any side-effects.

[gabyhelp]

Related Issues

• net/url: new JoinPath function and method use Path instead of EscapedPath() #53763 (closed)
• net/url: JoinPath doesn't strip relative path components in all circumstances #54385 (closed)
• net/url: add JoinPath, URL.JoinPath #47005 (closed)
• net/url: ResolveReference doesn't handled path escaping #16947 (closed)
• net/url: JoinPath() drops trailing slashes #52074 (closed)
• net/url: `unescape()` logic doesn't copy invalid bytes following % as expected by most recent spec #56732
• net/url: PathEscape treatment of reserved characters is inconsistent #27559
• net/url: Inconsistent .Path #66047 (closed)

Related Code Changes

• net/url: strip relative path components for JoinPath

Related Discussions

• url.ParseRequestURI validation of path

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[seankhliao]

I think it may break #58605 (comment)

[ShoshinNikita]

I believe this fix won't affect behavior described in #58605 - it modifies only arguments of *URL.JoinPath. So, the first argument of net/url.JoinPath - base - will stay the same

[seankhliao]

given that url.Parse doesn't support these broken urls, I'm not sure we should make JoinPath support them either.

[prattmic]

cc @neild @rsc

[ShoshinNikita]

given that url.Parse doesn't support these broken urls

That's true, but url.Parse returns an error, while (*URL).JoinPath just silently does nothing, and I find this behavior extremely surprising and error-prone. Another option would be to just document this, but I find this solution lacking.

There's a 3rd option for net/url.JoinPath - return an error when elem contains a malformed path. It may break some existing code, but I believe it's acceptable - I found plenty of examples in release notes where existing functions started to return errors for invalid inputs. However, I don't like the idea of (*URL).JoinPath and net/url.JoinPath behaving differently.