net/http: remove misleading response headers on error

ServeContent API is to set some headers you want to see in the response
before calling ServeContent. But if there is an error, those headers
should be removed otherwise they might confused the client.

Removing those headers is useful in general in the case of an error,
so we remove them in http.Error.

Fixes #50905.

Change-Id: If8d2786c1512906ac93e6b388df6ab1c5ecd1ea9
GitHub-Last-Rev: 32b6f045a791cf7bc391f018452a05cc872041ba
GitHub-Pull-Request: golang/go#64312
Reviewed-on: https://go-review.googlesource.com/c/go/+/544019
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go
index 70a4b8982f4fe454461521c30e3ef3ddcbfbaa36..4891c28768ca611c0421f4df653d11cb3f9aa6f6 100644 (file)
--- a/src/net/http/fs_test.go
+++ b/src/net/http/fs_test.go
@@ -27,6 +27,7 @@ import (
        "reflect"
        "regexp"
        "runtime"
+       "strconv"
        "strings"
        "testing"
        "testing/fstest"
@@ -1694,3 +1695,64 @@ func testFileServerDirWithRootFile(t *testing.T, mode testMode) {
                testDirFile(t, FileServerFS(os.DirFS("testdata/index.html")))
        })
 }
+
+func TestServeContentHeadersWithError(t *testing.T) {
+       contents := []byte("content")
+       ts := newClientServerTest(t, http1Mode, HandlerFunc(func(w ResponseWriter, r *Request) {
+               w.Header().Set("Content-Type", "application/octet-stream")
+               w.Header().Set("Content-Length", strconv.Itoa(len(contents)))
+               w.Header().Set("Content-Encoding", "gzip")
+               w.Header().Set("Etag", `"abcdefgh"`)
+               w.Header().Set("Last-Modified", "Wed, 21 Oct 2015 07:28:00 GMT")
+               w.Header().Set("Cache-Control", "immutable")
+               w.Header().Set("Other-Header", "test")
+               ServeContent(w, r, "", time.Time{}, bytes.NewReader(contents))
+       })).ts
+       defer ts.Close()
+
+       req, err := NewRequest("GET", ts.URL, nil)
+       if err != nil {
+               t.Fatal(err)
+       }
+       req.Header.Set("Range", "bytes=100-10000")
+
+       c := ts.Client()
+       res, err := c.Do(req)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       out, _ := io.ReadAll(res.Body)
+       res.Body.Close()
+
+       if g, e := res.StatusCode, 416; g != e {
+               t.Errorf("got status = %d; want %d", g, e)
+       }
+       if g, e := string(out), "invalid range: failed to overlap\n"; g != e {
+               t.Errorf("got body = %q; want %q", g, e)
+       }
+       if g, e := res.Header.Get("Content-Type"), "text/plain; charset=utf-8"; g != e {
+               t.Errorf("got content-type = %q, want %q", g, e)
+       }
+       if g, e := res.Header.Get("Content-Length"), strconv.Itoa(len(out)); g != e {
+               t.Errorf("got content-length = %q, want %q", g, e)
+       }
+       if g, e := res.Header.Get("Content-Encoding"), ""; g != e {
+               t.Errorf("got content-encoding = %q, want %q", g, e)
+       }
+       if g, e := res.Header.Get("Etag"), ""; g != e {
+               t.Errorf("got etag = %q, want %q", g, e)
+       }
+       if g, e := res.Header.Get("Last-Modified"), ""; g != e {
+               t.Errorf("got last-modified = %q, want %q", g, e)
+       }
+       if g, e := res.Header.Get("Cache-Control"), "no-cache"; g != e {
+               t.Errorf("got cache-control = %q, want %q", g, e)
+       }
+       if g, e := res.Header.Get("Content-Range"), "bytes */7"; g != e {
+               t.Errorf("got content-range = %q, want %q", g, e)
+       }
+       if g, e := res.Header.Get("Other-Header"), "test"; g != e {
+               t.Errorf("got other-header = %q, want %q", g, e)
+       }
+}

diff --git a/src/net/http/server.go b/src/net/http/server.go
index b0a2a1d888225c442f8990895838ac3e16825df6..fa953d842ec156b9e8db7650ab3ef47eb9f0939d 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -2173,9 +2173,20 @@ func (f HandlerFunc) ServeHTTP(w ResponseWriter, r *Request) {
 // writes are done to w.
 // The error message should be plain text.
 func Error(w ResponseWriter, error string, code int) {
-       w.Header().Del("Content-Length")
-       w.Header().Set("Content-Type", "text/plain; charset=utf-8")
-       w.Header().Set("X-Content-Type-Options", "nosniff")
+       h := w.Header()
+       // We delete headers which might be valid for some other content,
+       // but not anymore for the error content.
+       h.Del("Content-Length")
+       h.Del("Content-Encoding")
+       h.Del("Etag")
+       h.Del("Last-Modified")
+       // There might be cache control headers set for some other content,
+       // but we reset it to no-cache for the error content.
+       h.Set("Cache-Control", "no-cache")
+       // There might be content type already set, but we reset it to
+       // text/plain for the error message.
+       h.Set("Content-Type", "text/plain; charset=utf-8")
+       h.Set("X-Content-Type-Options", "nosniff")
        w.WriteHeader(code)
        fmt.Fprintln(w, error)
 }