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 <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/net/http/fs.go b/src/net/http/fs.go
index ce674c42ed84482bc807cb9798c053ad57121607..969ca65b69a243e77bcfb6c827e725b2e360f239 100644 (file)
--- 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, "<a href=\"./%s\">%s</a>\n", url.String(), htmlReplacer.Replace(name))
+               fmt.Fprintf(w, "<a href=\"%s\">%s</a>\n", url.String(), htmlReplacer.Replace(name))
        }
        fmt.Fprintf(w, "</pre>\n")
 }

diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go
index e39c3a83c7b3c3c4d7c9a37401b8f6c477436f0a..bc40cc7a520a7c0370fafa67bcb1a69c3f04dedc 100644 (file)
--- 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`, `<a href="./simple_name">simple_name</a>`},
-               {`"'<>&`, `<a href="./%22%27%3C%3E&">&#34;&#39;&lt;&gt;&amp;</a>`},
-               {`?foo=bar#baz`, `<a href="./%3Ffoo=bar%23baz">?foo=bar#baz</a>`},
-               {`<combo>?foo`, `<a href="./%3Ccombo%3E%3Ffoo">&lt;combo&gt;?foo</a>`},
+               {`simple_name`, `<a href="simple_name">simple_name</a>`},
+               {`"'<>&`, `<a href="%22%27%3C%3E&">&#34;&#39;&lt;&gt;&amp;</a>`},
+               {`?foo=bar#baz`, `<a href="%3Ffoo=bar%23baz">?foo=bar#baz</a>`},
+               {`<combo>?foo`, `<a href="%3Ccombo%3E%3Ffoo">&lt;combo&gt;?foo</a>`},
+               {`foo:bar`, `<a href="./foo:bar">foo:bar</a>`},
        }
 
        // 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 href=\"./a\">a</a>\n<a href=\"./b\">b</a>") {
+       if !strings.Contains(s, "<a href=\"a\">a</a>\n<a href=\"b\">b</a>") {
                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 fb70dbac0de795951023aa12424463e8141ba6ec..d77e9295ddef28572eba3867a0ce315466f43937 100644 (file)
--- 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 a48da73e4ab74953808aba12863cd4426aecd2da..6eac198448d14d7b894ce467dd2799a2a0640e73 100644 (file)
--- 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)
+               }
        }
 }