[go] net/url: reject colon in first segment of relative path in Parse
471 views
Skip to first unread message
Russ Cox (Gerrit)
Oct 20, 2016, 4:56:58 PM10/20/16
to Quentin Smith, Ian Lance Taylor, Russ Cox, golang-co...@googlegroups.com
Reviewers: Quentin Smith
Russ Cox uploaded a change:
https://go-review.googlesource.com/31582
net/url: reject colon in first segment of relative path in Parse
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes #16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
---
M src/net/url/url.go
M src/net/url/url_test.go
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/net/url/url.go b/src/net/url/url.go
index 525dbee..76db300 100644
--- a/src/net/url/url.go
+++ b/src/net/url/url.go
@@ -503,6 +503,19 @@
if viaRequest {
return nil, errors.New("invalid URI for request")
}
+
+ // Avoid confusion with malformed schemes, like cache_object:foo/bar.
+ // See golang.org/issue/16822.
+ //
+ // RFC 3986, §3.3:
+ // In addition, a URI reference (Section 4.1) may be a relative-path
reference,
+ // in which case the first path segment cannot contain a colon (":")
character.
+ colon := strings.Index(rest, ":")
+ slash := strings.Index(rest, "/")
+ if colon >= 0 && (slash < 0 || colon < slash) {
+ // First path segment has colon. Not allowed in relative URL.
+ return nil, errors.New("first path segment in URL cannot contain colon")
+ }
}
if (url.Scheme != "" || !viaRequest && !strings.HasPrefix(rest, "///"))
&& strings.HasPrefix(rest, "//") {
diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go
index 344ecdc..31658ba 100644
--- a/src/net/url/url_test.go
+++ b/src/net/url/url_test.go
@@ -1381,7 +1381,7 @@
}
}
-func TestParseAuthority(t *testing.T) {
+func TestParseErrors(t *testing.T) {
tests := []struct {
in string
wantErr bool
@@ -1401,9 +1401,13 @@
{"http://%41:8080/", true}, // not allowed: % encoding only for
non-ASCII
{"mysql://x@y(z:123)/foo", false}, // golang.org/issue/12023
{"mysql://x@y(1.2.3.4:123)/foo", false},
- {"mysql://x@y([2001:db8::1]:123)/foo", false},
+
{"http://[]%20%48%54%54%50%2f%31%2e%31%0a%4d%79%48%65%61%64%65%72%3a%20%31%32%33%0a%0a/",
true}, // golang.org/issue/11208
{"http://a b.com/",
true},
// no space in host name please
+ {"cache_object://foo",
true},
// scheme cannot have _, relative path cannot have : in first segment
+ {"cache_object:foo", true},
+ {"cache_object:foo/bar", true},
+ {"cache_object/:foo/bar", false},
}
for _, tt := range tests {
u, err := Parse(tt.in)
--
https://go-review.googlesource.com/31582
Gerrit-Reviewer: Quentin Smith <que...@golang.org>
Russ Cox uploaded a change:
https://go-review.googlesource.com/31582
net/url: reject colon in first segment of relative path in Parse
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes #16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
---
M src/net/url/url.go
M src/net/url/url_test.go
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/net/url/url.go b/src/net/url/url.go
index 525dbee..76db300 100644
--- a/src/net/url/url.go
+++ b/src/net/url/url.go
@@ -503,6 +503,19 @@
if viaRequest {
return nil, errors.New("invalid URI for request")
}
+
+ // Avoid confusion with malformed schemes, like cache_object:foo/bar.
+ // See golang.org/issue/16822.
+ //
+ // RFC 3986, §3.3:
+ // In addition, a URI reference (Section 4.1) may be a relative-path
reference,
+ // in which case the first path segment cannot contain a colon (":")
character.
+ colon := strings.Index(rest, ":")
+ slash := strings.Index(rest, "/")
+ if colon >= 0 && (slash < 0 || colon < slash) {
+ // First path segment has colon. Not allowed in relative URL.
+ return nil, errors.New("first path segment in URL cannot contain colon")
+ }
}
if (url.Scheme != "" || !viaRequest && !strings.HasPrefix(rest, "///"))
&& strings.HasPrefix(rest, "//") {
diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go
index 344ecdc..31658ba 100644
--- a/src/net/url/url_test.go
+++ b/src/net/url/url_test.go
@@ -1381,7 +1381,7 @@
}
}
-func TestParseAuthority(t *testing.T) {
+func TestParseErrors(t *testing.T) {
tests := []struct {
in string
wantErr bool
@@ -1401,9 +1401,13 @@
{"http://%41:8080/", true}, // not allowed: % encoding only for
non-ASCII
{"mysql://x@y(z:123)/foo", false}, // golang.org/issue/12023
{"mysql://x@y(1.2.3.4:123)/foo", false},
- {"mysql://x@y([2001:db8::1]:123)/foo", false},
+
{"http://[]%20%48%54%54%50%2f%31%2e%31%0a%4d%79%48%65%61%64%65%72%3a%20%31%32%33%0a%0a/",
true}, // golang.org/issue/11208
{"http://a b.com/",
true},
// no space in host name please
+ {"cache_object://foo",
true},
// scheme cannot have _, relative path cannot have : in first segment
+ {"cache_object:foo", true},
+ {"cache_object:foo/bar", true},
+ {"cache_object/:foo/bar", false},
}
for _, tt := range tests {
u, err := Parse(tt.in)
--
https://go-review.googlesource.com/31582
Gerrit-Reviewer: Quentin Smith <que...@golang.org>
Gobot Gobot (Gerrit)
Oct 20, 2016, 4:57:51 PM10/20/16
to Russ Cox, Quentin Smith, golang-co...@googlegroups.com
Gobot Gobot has posted comments on this change.
net/url: reject colon in first segment of relative path in Parse
Patch Set 1:
TryBots beginning. Status page: http://farmer.golang.org/try?commit=b11d8b5b
Gerrit-HasComments: No
net/url: reject colon in first segment of relative path in Parse
TryBots beginning. Status page: http://farmer.golang.org/try?commit=b11d8b5b
Gerrit-HasComments: No
Gobot Gobot (Gerrit)
Oct 20, 2016, 5:04:26 PM10/20/16
to Russ Cox, Quentin Smith, golang-co...@googlegroups.com
Gobot Gobot has posted comments on this change.
net/url: reject colon in first segment of relative path in Parse
Patch Set 1: TryBot-Result+1
TryBots are happy.
--
https://go-review.googlesource.com/31582
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Quentin Smith (Gerrit)
Oct 20, 2016, 5:40:12 PM10/20/16
to Russ Cox, Quentin Smith, Gobot Gobot, golang-co...@googlegroups.com
Quentin Smith has posted comments on this change.
net/url: reject colon in first segment of relative path in Parse
Patch Set 1: Code-Review+2
(1 comment)
https://go-review.googlesource.com/#/c/31582/1/src/net/url/url_test.go
File src/net/url/url_test.go:
PS1, Line 1408:
Trailing whitespace to remove on these and subsequent lines.
--
https://go-review.googlesource.com/31582
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Quentin Smith <que...@golang.org>
Gerrit-HasComments: Yes
net/url: reject colon in first segment of relative path in Parse
(1 comment)
https://go-review.googlesource.com/#/c/31582/1/src/net/url/url_test.go
File src/net/url/url_test.go:
PS1, Line 1408:
Trailing whitespace to remove on these and subsequent lines.
--
https://go-review.googlesource.com/31582
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Quentin Smith <que...@golang.org>
Russ Cox (Gerrit)
Oct 24, 2016, 12:04:37 PM10/24/16
to Russ Cox, Gobot Gobot, Quentin Smith, golang-co...@googlegroups.com
Reviewers: Gobot Gobot, Quentin Smith
Russ Cox uploaded a new patch set:
Russ Cox uploaded a new patch set:
https://go-review.googlesource.com/31582
net/url: reject colon in first segment of relative path in Parse
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes #16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
---
M src/net/url/url.go
M src/net/url/url_test.go
2 files changed, 19 insertions(+), 2 deletions(-)
net/url: reject colon in first segment of relative path in Parse
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes #16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
---
M src/net/url/url.go
M src/net/url/url_test.go
2 files changed, 19 insertions(+), 2 deletions(-)
Russ Cox (Gerrit)
Oct 24, 2016, 12:04:50 PM10/24/16
to Russ Cox, golang-...@googlegroups.com, Quentin Smith, Gobot Gobot, golang-co...@googlegroups.com
Russ Cox has submitted this change and it was merged.
net/url: reject colon in first segment of relative path in Parse
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes #16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
Reviewed-on: https://go-review.googlesource.com/31582
Reviewed-by: Quentin Smith <que...@golang.org>
Quentin Smith: Looks good to me, approved
--
https://go-review.googlesource.com/31582
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Quentin Smith <que...@golang.org>
Gerrit-Reviewer: Russ Cox <r...@golang.org>
net/url: reject colon in first segment of relative path in Parse
RFC 3986 §3.3 disallows relative URL paths in which the first segment
contains a colon, presumably to avoid confusion with scheme:foo syntax,
which is exactly what happened in #16822.
Fixes #16822.
Change-Id: Ie4449e1dd21c5e56e3b126e086c3a0b05da7ff24
Reviewed-by: Quentin Smith <que...@golang.org>
---
M src/net/url/url.go
M src/net/url/url_test.go
2 files changed, 19 insertions(+), 2 deletions(-)
Approvals:
M src/net/url/url.go
M src/net/url/url_test.go
2 files changed, 19 insertions(+), 2 deletions(-)
Quentin Smith: Looks good to me, approved
--
https://go-review.googlesource.com/31582
Gerrit-Reviewer: Gobot Gobot <go...@golang.org>
Gerrit-Reviewer: Quentin Smith <que...@golang.org>
Benoit Sigoure (Gerrit)
Feb 22, 2017, 5:43:19 PM2/22/17
to Russ Cox, Quentin Smith, Gobot Gobot, golang-co...@googlegroups.com
Benoit Sigoure posted comments on this change.
Patch set 3:
(1 comment)
colon := strings.Index(rest, ":") slash := strings.Index(rest, "/")
May as well use IndexByte() here instead.
To view, visit change 31582. To unsubscribe, visit settings.
Brad Fitzpatrick (Gerrit)
Feb 22, 2017, 7:33:56 PM2/22/17
to Russ Cox, Brad Fitzpatrick, Benoit Sigoure, Quentin Smith, Gobot Gobot, golang-co...@googlegroups.com
Brad Fitzpatrick posted comments on this change.
colon := strings.Index(rest, ":") slash := strings.Index(rest, "/")
May as well use IndexByte() here instead.
This isn't performance-sensitive code where the overhead of two function calls is worth the extra verbosity or caring either way. If it had been written that way to begin with, sure, but it's not worth changing.
To view, visit change 31582. To unsubscribe, visit settings.
Reply all
Reply to author
Forward
0 new messages