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

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")

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