net/http: escape contents of the directory indexes generated by FileServer

      Previously, filenames containing special characters could:
      1) Escape the <a> tag, with a file called something like: ">foo
      2) Break the links in the index by prematurely ending the path portion
      of the url, with a file called: foo?bar

      In order to avoid a forbidden dependency on the html package, I'm
      using htmlReplacer from net/http/server.go, which is equivalent to
      html.EscapeString.

      This change also expands fakeFile.Readdir to better emulate
os.File.Readdir.

R=golang-codereviews, rsc, gobot, bradfitz, josharian, mikioh.mikioh
CC=golang-codereviews
https://golang.org/cl/37440043

diff --git a/src/pkg/net/http/fs.go b/src/pkg/net/http/fs.go
index 042e6da113b02a7f39d79306497bef425b9e944b..9df5cc481897d98ac628b23252bcee0c0a910510 100644 (file)
--- a/src/pkg/net/http/fs.go
+++ b/src/pkg/net/http/fs.go
@@ -13,6 +13,7 @@ import (
        "mime"
        "mime/multipart"
        "net/textproto"
+       "net/url"
        "os"
        "path"
        "path/filepath"
@@ -75,8 +76,11 @@ func dirList(w ResponseWriter, f File) {
                        if d.IsDir() {
                                name += "/"
                        }
-                       // TODO htmlescape
-                       fmt.Fprintf(w, "<a href=\"%s\">%s</a>\n", name, name)
+                       // name may contain '?' or '#', which must be escaped to remain
+                       // 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, "</pre>\n")

diff --git a/src/pkg/net/http/fs_test.go b/src/pkg/net/http/fs_test.go
index ae54edf0cf504fc9d80414a2a0246155ce078b78..f968565f9b4db430e5eca95c2ef7063642616971 100644 (file)
--- a/src/pkg/net/http/fs_test.go
+++ b/src/pkg/net/http/fs_test.go
@@ -227,6 +227,54 @@ func TestFileServerCleans(t *testing.T) {
        }
 }
 
+func TestFileServerEscapesNames(t *testing.T) {
+       defer afterTest(t)
+       const dirListPrefix = "<pre>\n"
+       const dirListSuffix = "\n</pre>\n"
+       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>`},
+       }
+
+       // We put each test file in its own directory in the fakeFS so we can look at it in isolation.
+       fs := make(fakeFS)
+       for i, test := range tests {
+               testFile := &fakeFileInfo{basename: test.name}
+               fs[fmt.Sprintf("/%d", i)] = &fakeFileInfo{
+                       dir:     true,
+                       modtime: time.Unix(1000000000, 0).UTC(),
+                       ents:    []*fakeFileInfo{testFile},
+               }
+               fs[fmt.Sprintf("/%d/%s", i, test.name)] = testFile
+       }
+
+       ts := httptest.NewServer(FileServer(&fs))
+       defer ts.Close()
+       for i, test := range tests {
+               url := fmt.Sprintf("%s/%d", ts.URL, i)
+               res, err := Get(url)
+               if err != nil {
+                       t.Fatalf("test %q: Get: %v", test.name, err)
+               }
+               b, err := ioutil.ReadAll(res.Body)
+               if err != nil {
+                       t.Fatalf("test %q: read Body: %v", test.name, err)
+               }
+               s := string(b)
+               if !strings.HasPrefix(s, dirListPrefix) || !strings.HasSuffix(s, dirListSuffix) {
+                       t.Errorf("test %q: listing dir, full output is %q, want prefix %q and suffix %q", test.name, s, dirListPrefix, dirListSuffix)
+               }
+               if trimmed := strings.TrimSuffix(strings.TrimPrefix(s, dirListPrefix), dirListSuffix); trimmed != test.escaped {
+                       t.Errorf("test %q: listing dir, filename escaped to %q, want %q", test.name, trimmed, test.escaped)
+               }
+               res.Body.Close()
+       }
+}
+
 func mustRemoveAll(dir string) {
        err := os.RemoveAll(dir)
        if err != nil {
@@ -457,8 +505,9 @@ func (f *fakeFileInfo) Mode() os.FileMode {
 
 type fakeFile struct {
        io.ReadSeeker
-       fi   *fakeFileInfo
-       path string // as opened
+       fi     *fakeFileInfo
+       path   string // as opened
+       entpos int
 }
 
 func (f *fakeFile) Close() error               { return nil }
@@ -468,10 +517,20 @@ func (f *fakeFile) Readdir(count int) ([]os.FileInfo, error) {
                return nil, os.ErrInvalid
        }
        var fis []os.FileInfo
-       for _, fi := range f.fi.ents {
-               fis = append(fis, fi)
+
+       limit := f.entpos + count
+       if count <= 0 || limit > len(f.fi.ents) {
+               limit = len(f.fi.ents)
+       }
+       for ; f.entpos < limit; f.entpos++ {
+               fis = append(fis, f.fi.ents[f.entpos])
+       }
+
+       if len(fis) == 0 && count > 0 {
+               return fis, io.EOF
+       } else {
+               return fis, nil
        }
-       return fis, nil
 }
 
 type fakeFS map[string]*fakeFileInfo
@@ -480,7 +539,6 @@ func (fs fakeFS) Open(name string) (File, error) {
        name = path.Clean(name)
        f, ok := fs[name]
        if !ok {
-               println("fake filesystem didn't find file", name)
                return nil, os.ErrNotExist
        }
        return &fakeFile{ReadSeeker: strings.NewReader(f.contents), fi: f, path: name}, nil