Repository: google/go-safeweb (go)

Issue:
Simple file server (no dispatcher, no interceptors)
Applications frequently serve static files as a response (`.js`, `.css`, images, etc) and, for these, developers  implement unexported function to detect and set the `Content-Type`, based on the files' extension, omitting the `charset` parameter often. We might want to do this properly in the `safehttp.DefaultDispatcher`.

Pull request description:
FileServer implementation.
In our threat model we assume that the directory passed to the
`FileServer` contains exclusively fully-trusted content.

- a `FileServerResponse` is generated when the underlying `http.FileServer`
calls `WriteHeader(200)`; the response is written as any other safe
response (e.g. goes through interceptors)
- the `DefaultDispatcher` sets the Content-Type according to what
`FileServerResponse` sets, but doesn't actually write the content due to
the nature of `http.FileServer` (many direct calls to
`http.ResponseWriter.Write([]byte)`)
- all errors coming from `http.FileServer` are converted to
`safehttp.StatusNotFound` in order to prevent information leaks

Run `go mod tidy` too.

Fixes #187.

Patch:
diff --git a/go.mod b/go.mod
index d686ce7..f0664bf 100644
--- a/go.mod
+++ b/go.mod
@@ -5,5 +5,5 @@ go 1.14
 require (
 	github.com/google/go-cmp v0.5.0
 	github.com/google/safehtml v0.0.2
-	golang.org/x/net v0.0.0-20200707034311-ab3426394381
+	golang.org/x/net v0.0.0-20210410081132-afb366fc7cd1
 )
diff --git a/go.sum b/go.sum
index 560b7a5..1101f97 100644
--- a/go.sum
+++ b/go.sum
@@ -1,21 +1,16 @@
 github.com/google/go-cmp v0.5.0 h1:/QaMHBdZ26BB3SSst0Iwl10Epc+xhTquomWX0oZEB6w=
 github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
-github.com/google/go-cmp v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=
-github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM=
 github.com/google/safehtml v0.0.2 h1:ZOt2VXg4x24bW0m2jtzAOkhoXV0iM8vNKc0paByCZqM=
 github.com/google/safehtml v0.0.2/go.mod h1:L4KWwDsUJdECRAEpZoBn3O64bQaywRscowZjJAzjHnU=
-golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
-golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
-golang.org/x/net v0.0.0-20200707034311-ab3426394381 h1:VXak5I6aEWmAXeQjA+QSZzlgNrpq9mjcfDemuexIKsU=
-golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
-golang.org/x/net v0.0.0-20200822124328-c89045814202 h1:VvcQYSHwXgi7W+TpUR6A9g6Up98WAHf3f/ulnJ62IyA=
-golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
-golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/net v0.0.0-20210410081132-afb366fc7cd1 h1:4qWs8cYYH6PoEFy4dfhDFgoMGkwAcETd+MmPdCPMzUc=
+golang.org/x/net v0.0.0-20210410081132-afb366fc7cd1/go.mod h1:9tjilg8BloeKEkVJvy7fQ90B1CfIiPueXVOjqfkSzI8=
+golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20210330210617-4fbd30eecc44/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M=
+golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/safehttp/default_dispatcher.go b/safehttp/default_dispatcher.go
index bacce2a..bd716d7 100644
--- a/safehttp/default_dispatcher.go
+++ b/safehttp/default_dispatcher.go
@@ -41,6 +41,8 @@ func (DefaultDispatcher) ContentType(resp Response) (string, error) {
 		return "text/html; charset=utf-8", nil
 	case JSONResponse:
 		return "application/json; charset=utf-8", nil
+	case FileServerResponse:
+		return x.ContentType(), nil
 	default:
 		return "", fmt.Errorf("%T is not a safe response type, a Content-Type cannot be provided", resp)
 	}
@@ -78,6 +80,9 @@ func (DefaultDispatcher) Write(rw http.ResponseWriter, resp Response) error {
 	case safehtml.HTML:
 		_, err := io.WriteString(rw, x.String())
 		return err
+	case FileServerResponse:
+		// The http package will take care of writing the file body.
+		return nil
 	default:
 		return fmt.Errorf("%T is not a safe response type and it cannot be written", resp)
 	}
diff --git a/safehttp/fileserver.go b/safehttp/fileserver.go
new file mode 100644
index 0000000..71a6205
--- /dev/null
+++ b/safehttp/fileserver.go
@@ -0,0 +1,124 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package safehttp
+
+import (
+	"errors"
+	"net/http"
+)
+
+// FileServer returns a handler that serves HTTP requests with the contents of
+// the file system rooted at root.
+func FileServer(root string) Handler {
+	fileServer := http.FileServer(http.Dir(root))
+
+	return HandlerFunc(func(rw ResponseWriter, req *IncomingRequest) Result {
+		fsrw := &fileServerResponseWriter{flight: rw.(*flight), header: http.Header{}}
+		fileServer.ServeHTTP(fsrw, req.req)
+		return fsrw.result
+	})
+}
+
+type fileServerResponseWriter struct {
+	flight *flight
+	result Result
+
+	// We don't allow direct access to the flight's underlying http.Header. We
+	// just copy over the contents on a call to WriteHeader, with the exception
+	// of the Content-Type header.
+	header http.Header
+
+	// Once WriteHeader is called, any subsequent calls to it are no-ops.
+	committed bool
+
+	// If the first call to WriteHeader is not a 200 OK, we call
+	// flight.WriteError with a 404 StatusCode and make further calls to Write
+	// no-ops in order to not leak information about the filesystem.
+	errored bool
+}
+
+func (fsrw *fileServerResponseWriter) Header() http.Header {
+	return fsrw.header
+}
+
+func (fsrw *fileServerResponseWriter) Write(b []byte) (int, error) {
+	if !fsrw.committed {
+		fsrw.WriteHeader(int(StatusOK))
+	}
+
+	if fsrw.errored {
+		// Let the framework handle the error.
+		return 0, errors.New("discarded")
+	}
+	return fsrw.flight.rw.Write(b)
+}
+
+func (fsrw *fileServerResponseWriter) WriteHeader(statusCode int) {
+	if fsrw.committed {
+		// We've already committed to a response. The headers and status code
+		// were written. Ignore this call.
+		return
+	}
+	fsrw.committed = true
+
+	headers := fsrw.flight.Header()
+	ct := "application/octet-stream; charset=utf-8"
+	if len(fsrw.header["Content-Type"]) > 0 {
+		ct = fsrw.header["Content-Type"][0]
+	}
+	// Content-Type should have been set by the http.FileServer.
+	// Note: Add or Set might panic if a header has been already claimed. This
+	// is intended behavior.
+	for k, v := range fsrw.header {
+		if len(v) == 0 {
+			continue
+		}
+		if k == "Content-Type" {
+			// Skip setting the Content-Type. The Dispatcher handles it.
+			continue
+		}
+		headers.Del(k)
+		for _, vv := range v {
+			headers.Add(k, vv)
+		}
+	}
+
+	if statusCode != int(StatusOK) {
+		fsrw.errored = true
+		// We are writing 404 for every error to avoid leaking information about
+		// the filesystem.
+		fsrw.result = fsrw.flight.WriteError(StatusNotFound)
+		return
+	}
+
+	fsrw.result = fsrw.flight.Write(FileServerResponse{
+		Path:        fsrw.flight.req.URL.Path(),
+		contentType: ct,
+	})
+}
+
+// FileServerResponse represents a FileServer response.
+type FileServerResponse struct {
+	// The URL path.
+	Path string
+
+	// private, to not allow modifications
+	contentType string
+}
+
+// ContentType is the Content-Type of the response.
+func (resp FileServerResponse) ContentType() string {
+	return resp.contentType
+}


Test patch:
diff --git a/safehttp/fileserver_test.go b/safehttp/fileserver_test.go
new file mode 100644
index 0000000..8fad7b4
--- /dev/null
+++ b/safehttp/fileserver_test.go
@@ -0,0 +1,89 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package safehttp_test
+
+import (
+	"io/ioutil"
+	"net/http/httptest"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-safeweb/safehttp"
+	"github.com/google/go-safeweb/safehttp/safehttptest"
+)
+
+func TestFileServer(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "go-safehttp-test")
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(): %v", err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	if err := ioutil.WriteFile(tmpDir+"/foo.html", []byte("<h1>Hello world</h1>"), 0644); err != nil {
+		t.Fatalf("ioutil.WriteFile: %v", err)
+	}
+
+	tests := []struct {
+		name     string
+		path     string
+		wantCode safehttp.StatusCode
+		wantCT   string
+		wantBody string
+	}{
+		{
+			name:     "missing file",
+			path:     "failure",
+			wantCode: 404,
+			wantCT:   "text/plain; charset=utf-8",
+			wantBody: "Not Found\n",
+		},
+		{
+			name:     "file",
+			path:     "foo.html",
+			wantCode: 200,
+			wantCT:   "text/html; charset=utf-8",
+			wantBody: "<h1>Hello world</h1>",
+		},
+	}
+
+	mb := &safehttp.ServeMuxConfig{}
+	mb.Handle("/", safehttp.MethodGet, safehttp.FileServer(tmpDir))
+	m := mb.Mux()
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var b strings.Builder
+			rr := safehttptest.NewTestResponseWriter(&b)
+
+			req := httptest.NewRequest(safehttp.MethodGet, "https://test.science/"+tt.path, nil)
+			m.ServeHTTP(rr, req)
+
+			if got, want := rr.Status(), tt.wantCode; got != tt.wantCode {
+				t.Errorf("status code got: %v want: %v", got, want)
+			}
+			if got := rr.Header().Get("Content-Type"); tt.wantCT != got {
+				t.Errorf("Content-Type: got %q want %q", got, tt.wantCT)
+			}
+			if diff := cmp.Diff(tt.wantBody, b.String()); diff != "" {
+				t.Errorf("Response body diff (-want,+got): \n%s", diff)
+			}
+		})
+	}
+}
+
+// TODO(kele): Add tests including interceptors once we have
+// https://github.com/google/go-safeweb/issues/261.