From ad5d91c17a3c0bc4acf9e4036b050517972432f0 Mon Sep 17 00:00:00 2001 From: Kale Blankenship Date: Wed, 21 Sep 2016 19:03:06 -0700 Subject: [PATCH] net/url: prefix relative paths containing ":" in the first segment with "./" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/fs.go | 2 +- src/net/http/fs_test.go | 11 +++++---- src/net/url/url.go | 11 +++++++++ src/net/url/url_test.go | 51 +++++++++++++++++++++++++++++++++-------- 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/src/net/http/fs.go b/src/net/http/fs.go index ce674c42ed..969ca65b69 100644 --- a/src/net/http/fs.go +++ b/src/net/http/fs.go @@ -90,7 +90,7 @@ func dirList(w ResponseWriter, f File) { // part of the URL path, and not indicate the start of a query // string or fragment. url := url.URL{Path: name} - fmt.Fprintf(w, "%s\n", url.String(), htmlReplacer.Replace(name)) + fmt.Fprintf(w, "%s\n", url.String(), htmlReplacer.Replace(name)) } fmt.Fprintf(w, "\n") } diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go index e39c3a83c7..bc40cc7a52 100644 --- a/src/net/http/fs_test.go +++ b/src/net/http/fs_test.go @@ -270,10 +270,11 @@ func TestFileServerEscapesNames(t *testing.T) { tests := []struct { name, escaped string }{ - {`simple_name`, `simple_name`}, - {`"'<>&`, `"'<>&`}, - {`?foo=bar#baz`, `?foo=bar#baz`}, - {`?foo`, `<combo>?foo`}, + {`simple_name`, `simple_name`}, + {`"'<>&`, `"'<>&`}, + {`?foo=bar#baz`, `?foo=bar#baz`}, + {`?foo`, `<combo>?foo`}, + {`foo:bar`, `foo:bar`}, } // 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) { t.Fatalf("read Body: %v", err) } s := string(b) - if !strings.Contains(s, "a\nb") { + if !strings.Contains(s, "a\nb") { t.Errorf("output appears to be unsorted:\n%s", s) } } diff --git a/src/net/url/url.go b/src/net/url/url.go index fb70dbac0d..d77e9295dd 100644 --- a/src/net/url/url.go +++ b/src/net/url/url.go @@ -713,6 +713,17 @@ func (u *URL) String() string { if path != "" && path[0] != '/' && u.Host != "" { buf.WriteByte('/') } + if buf.Len() == 0 { + // 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. + if i := strings.IndexByte(path, ':'); i > -1 && strings.IndexByte(path[:i], '/') == -1 { + buf.WriteString("./") + } + } buf.WriteString(path) } if u.ForceQuery || u.RawQuery != "" { diff --git a/src/net/url/url_test.go b/src/net/url/url_test.go index a48da73e4a..6eac198448 100644 --- a/src/net/url/url_test.go +++ b/src/net/url/url_test.go @@ -676,6 +676,44 @@ func TestParseRequestURI(t *testing.T) { } } +var stringURLTests = []struct { + url URL + want string +}{ + // No leading slash on path should prepend slash on String() call + { + url: URL{ + Scheme: "http", + Host: "www.google.com", + Path: "search", + }, + want: "http://www.google.com/search", + }, + // Relative path with first element containing ":" should be prepended with "./", golang.org/issue/17184 + { + url: URL{ + Path: "this:that", + }, + want: "./this:that", + }, + // Relative path with second element containing ":" should not be prepended with "./" + { + url: URL{ + Path: "here/this:that", + }, + want: "here/this:that", + }, + // Non-relative path with first element containing ":" should not be prepended with "./" + { + url: URL{ + Scheme: "http", + Host: "www.google.com", + Path: "this:that", + }, + want: "http://www.google.com/this:that", + }, +} + func TestURLString(t *testing.T) { for _, tt := range urltests { u, err := Parse(tt.in) @@ -693,15 +731,10 @@ func TestURLString(t *testing.T) { } } - // No leading slash on path should prepend - // slash on String() call - noslash := URL{ - Scheme: "http", - Host: "www.google.com", - Path: "search", - } - if got, want := noslash.String(), "http://www.google.com/search"; got != want { - t.Errorf("No slash\ngot %q\nwant %q", got, want) + for _, tt := range stringURLTests { + if got := tt.url.String(); got != tt.want { + t.Errorf("%+v.String() = %q; want %q", tt.url, got, tt.want) + } } }