From: Mitar <mitar.git@tnode.com>
Date: Tue, 5 Mar 2024 23:45:51 +0000 (+0000)
Subject: net/http: remove misleading response headers on error
X-Git-Tag: go1.23rc1~987
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d463de61b38cc037808970a8aeed8255082a1df5;p=gostls13.git

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 70a4b8982f..4891c28768 100644
--- 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 b0a2a1d888..fa953d842e 100644
--- 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)
 }