Skip to content

Commit fced03a

Browse files
committed
net/url: allow all valid host chars in RawPath
The old code was only allowing the chars we choose not to escape. We sometimes prefer to escape chars that do not strictly need it. Allowing those to be used in RawPath lets people override that preference, which is in fact the whole point of RawPath (new in Go 1.5). While we are here, also allow [ ] in RawPath. This is not strictly spec-compliant, but it is what modern browers do and what at least some people expect, and the [ ] do not cause any ambiguity (the usual reason they would be escaped, as they are part of the RFC gen-delims class). The argument for allowing them now instead of waiting until Go 1.6 is that this way RawPath has one fixed meaning at the time it is introduced, that we should not need to change or expand. Fixes #5684. Change-Id: If9c82a18f522d7ee1d10310a22821ada9286ee5c Reviewed-on: https://go-review.googlesource.com/13258 Reviewed-by: Andrew Gerrand <[email protected]>
1 parent e8be9a1 commit fced03a

File tree

2 files changed

+39
-3
lines changed

2 files changed

+39
-3
lines changed

src/net/url/url.go

+16-2
Original file line numberDiff line numberDiff line change
@@ -550,8 +550,22 @@ func (u *URL) EscapedPath() string {
550550
// It must not contain any bytes that require escaping during path encoding.
551551
func validEncodedPath(s string) bool {
552552
for i := 0; i < len(s); i++ {
553-
if s[i] != '%' && shouldEscape(s[i], encodePath) {
554-
return false
553+
// RFC 3986, Appendix A.
554+
// pchar = unreserved / pct-encoded / sub-delims / ":" / "@".
555+
// shouldEscape is not quite compliant with the RFC,
556+
// so we check the sub-delims ourselves and let
557+
// shouldEscape handle the others.
558+
switch s[i] {
559+
case '!', '$', '&', '\'', '(', ')', '*', '+', ',', ';', '=', ':', '@':
560+
// ok
561+
case '[', ']':
562+
// ok - not specified in RFC 3986 but left alone by modern browsers
563+
case '%':
564+
// ok - percent encoded, will decode
565+
default:
566+
if shouldEscape(s[i], encodePath) {
567+
return false
568+
}
555569
}
556570
}
557571
return true

src/net/url/url_test.go

+23-1
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ var urltests = []URLTest{
392392
},
393393
"",
394394
},
395-
// worst case host
395+
// worst case host, still round trips
396396
{
397397
"scheme://!$&'()*+,;=hello!:port/path",
398398
&URL{
@@ -402,6 +402,28 @@ var urltests = []URLTest{
402402
},
403403
"",
404404
},
405+
// worst case path, still round trips
406+
{
407+
"http://host/!$&'()*+,;=:@[hello]",
408+
&URL{
409+
Scheme: "http",
410+
Host: "host",
411+
Path: "/!$&'()*+,;=:@[hello]",
412+
RawPath: "/!$&'()*+,;=:@[hello]",
413+
},
414+
"",
415+
},
416+
// golang.org/issue/5684
417+
{
418+
"http://example.com/oid/[order_id]",
419+
&URL{
420+
Scheme: "http",
421+
Host: "example.com",
422+
Path: "/oid/[order_id]",
423+
RawPath: "/oid/[order_id]",
424+
},
425+
"",
426+
},
405427
}
406428

407429
// more useful string for debugging than fmt's struct printer

0 commit comments

Comments
 (0)