Skip to content

Commit ad5d91c

Browse files
vcabbagebradfitz
authored andcommitted
net/url: prefix relative paths containing ":" in the first segment with "./"
This change modifies URL.String to prepend "./" to a relative URL which contains a colon in the first path segment. Per RFC 3986 §4.2: > A path segment that contains a colon character (e.g., "this:that") > cannot be used as the first segment of a relative-path reference, as > it would be mistaken for a scheme name. Such a segment must be > preceded by a dot-segment (e.g., "./this:that") to make a relative- > path reference. https://go-review.googlesource.com/27440 corrects the behavior for http.FileServer, but URL.String will still return an invalid URL. This CL reverts the changes to http.FileServer as they are unnecessary with this fix. Fixes #17184 Change-Id: I9211ae20f82c91b785d1b079b2cd766487d94225 Reviewed-on: https://go-review.googlesource.com/29610 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent cddddbc commit ad5d91c

File tree

4 files changed

+60
-15
lines changed

4 files changed

+60
-15
lines changed

src/net/http/fs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func dirList(w ResponseWriter, f File) {
9090
// part of the URL path, and not indicate the start of a query
9191
// string or fragment.
9292
url := url.URL{Path: name}
93-
fmt.Fprintf(w, "<a href=\"./%s\">%s</a>\n", url.String(), htmlReplacer.Replace(name))
93+
fmt.Fprintf(w, "<a href=\"%s\">%s</a>\n", url.String(), htmlReplacer.Replace(name))
9494
}
9595
fmt.Fprintf(w, "</pre>\n")
9696
}

src/net/http/fs_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,10 +270,11 @@ func TestFileServerEscapesNames(t *testing.T) {
270270
tests := []struct {
271271
name, escaped string
272272
}{
273-
{`simple_name`, `<a href="./simple_name">simple_name</a>`},
274-
{`"'<>&`, `<a href="./%22%27%3C%3E&">&#34;&#39;&lt;&gt;&amp;</a>`},
275-
{`?foo=bar#baz`, `<a href="./%3Ffoo=bar%23baz">?foo=bar#baz</a>`},
276-
{`<combo>?foo`, `<a href="./%3Ccombo%3E%3Ffoo">&lt;combo&gt;?foo</a>`},
273+
{`simple_name`, `<a href="simple_name">simple_name</a>`},
274+
{`"'<>&`, `<a href="%22%27%3C%3E&">&#34;&#39;&lt;&gt;&amp;</a>`},
275+
{`?foo=bar#baz`, `<a href="%3Ffoo=bar%23baz">?foo=bar#baz</a>`},
276+
{`<combo>?foo`, `<a href="%3Ccombo%3E%3Ffoo">&lt;combo&gt;?foo</a>`},
277+
{`foo:bar`, `<a href="./foo:bar">foo:bar</a>`},
277278
}
278279

279280
// We put each test file in its own directory in the fakeFS so we can look at it in isolation.
@@ -349,7 +350,7 @@ func TestFileServerSortsNames(t *testing.T) {
349350
t.Fatalf("read Body: %v", err)
350351
}
351352
s := string(b)
352-
if !strings.Contains(s, "<a href=\"./a\">a</a>\n<a href=\"./b\">b</a>") {
353+
if !strings.Contains(s, "<a href=\"a\">a</a>\n<a href=\"b\">b</a>") {
353354
t.Errorf("output appears to be unsorted:\n%s", s)
354355
}
355356
}

src/net/url/url.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,17 @@ func (u *URL) String() string {
713713
if path != "" && path[0] != '/' && u.Host != "" {
714714
buf.WriteByte('/')
715715
}
716+
if buf.Len() == 0 {
717+
// RFC 3986 §4.2
718+
// A path segment that contains a colon character (e.g., "this:that")
719+
// cannot be used as the first segment of a relative-path reference, as
720+
// it would be mistaken for a scheme name. Such a segment must be
721+
// preceded by a dot-segment (e.g., "./this:that") to make a relative-
722+
// path reference.
723+
if i := strings.IndexByte(path, ':'); i > -1 && strings.IndexByte(path[:i], '/') == -1 {
724+
buf.WriteString("./")
725+
}
726+
}
716727
buf.WriteString(path)
717728
}
718729
if u.ForceQuery || u.RawQuery != "" {

src/net/url/url_test.go

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,44 @@ func TestParseRequestURI(t *testing.T) {
676676
}
677677
}
678678

679+
var stringURLTests = []struct {
680+
url URL
681+
want string
682+
}{
683+
// No leading slash on path should prepend slash on String() call
684+
{
685+
url: URL{
686+
Scheme: "http",
687+
Host: "www.google.com",
688+
Path: "search",
689+
},
690+
want: "http://www.google.com/search",
691+
},
692+
// Relative path with first element containing ":" should be prepended with "./", golang.org/issue/17184
693+
{
694+
url: URL{
695+
Path: "this:that",
696+
},
697+
want: "./this:that",
698+
},
699+
// Relative path with second element containing ":" should not be prepended with "./"
700+
{
701+
url: URL{
702+
Path: "here/this:that",
703+
},
704+
want: "here/this:that",
705+
},
706+
// Non-relative path with first element containing ":" should not be prepended with "./"
707+
{
708+
url: URL{
709+
Scheme: "http",
710+
Host: "www.google.com",
711+
Path: "this:that",
712+
},
713+
want: "http://www.google.com/this:that",
714+
},
715+
}
716+
679717
func TestURLString(t *testing.T) {
680718
for _, tt := range urltests {
681719
u, err := Parse(tt.in)
@@ -693,15 +731,10 @@ func TestURLString(t *testing.T) {
693731
}
694732
}
695733

696-
// No leading slash on path should prepend
697-
// slash on String() call
698-
noslash := URL{
699-
Scheme: "http",
700-
Host: "www.google.com",
701-
Path: "search",
702-
}
703-
if got, want := noslash.String(), "http://www.google.com/search"; got != want {
704-
t.Errorf("No slash\ngot %q\nwant %q", got, want)
734+
for _, tt := range stringURLTests {
735+
if got := tt.url.String(); got != tt.want {
736+
t.Errorf("%+v.String() = %q; want %q", tt.url, got, tt.want)
737+
}
705738
}
706739
}
707740

0 commit comments

Comments
 (0)