Revert "net/http: use Copy in ServeContent if CopyN not needed"

This reverts CL 446276. Reason for revert: Causing surprising performance regression. Fixes #61530 Change-Id: Ic970f2e05d875b606ce274ea621f7e4c8c337481 Reviewed-on: https://go-review.googlesource.com/c/go/+/512615 Run-TryBot: Damien Neil <[email protected]> Reviewed-by: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]>
cloudflare · Jul 24, 2023 · df0a129 · df0a129
1 parent 89a4578
commit df0a129
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 48 deletions.
diff --git a/src/net/http/fs.go b/src/net/http/fs.go
@@ -349,13 +349,8 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time,
 
  w.WriteHeader(code)
 
- if r.Method != MethodHead {
- if sendSize == size {
- // use Copy in the non-range case to make use of WriterTo if available
- io.Copy(w, sendContent)
- } else {
- io.CopyN(w, sendContent, sendSize)
- }
+ if r.Method != "HEAD" {
+ io.CopyN(w, sendContent, sendSize)
  }
 }
 

diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go
@@ -924,7 +924,6 @@ func testServeContent(t *testing.T, mode testMode) {
  wantContentType string
  wantContentRange string
  wantStatus int
- wantContent []byte
  }
  htmlModTime := mustStat(t, "testdata/index.html").ModTime()
  tests := map[string]testCase{
@@ -1140,24 +1139,6 @@ func testServeContent(t *testing.T, mode testMode) {
  wantStatus: 412,
  wantLastMod: htmlModTime.UTC().Format(TimeFormat),
  },
- "uses_writeTo_if_available_and_non-range": {
- content: &panicOnNonWriterTo{seekWriterTo: strings.NewReader("foobar")},
- serveContentType: "text/plain; charset=utf-8",
- wantContentType: "text/plain; charset=utf-8",
- wantStatus: StatusOK,
- wantContent: []byte("foobar"),
- },
- "do_not_use_writeTo_for_range_requests": {
- content: &panicOnWriterTo{ReadSeeker: strings.NewReader("foobar")},
- serveContentType: "text/plain; charset=utf-8",
- reqHeader: map[string]string{
- "Range": "bytes=0-4",
- },
- wantContentType: "text/plain; charset=utf-8",
- wantContentRange: "bytes 0-4/6",
- wantStatus: StatusPartialContent,
- wantContent: []byte("fooba"),
- },
  }
  for testName, tt := range tests {
  var content io.ReadSeeker
@@ -1171,8 +1152,7 @@ func testServeContent(t *testing.T, mode testMode) {
  } else {
  content = tt.content
  }
- contentOut := &strings.Builder{}
- for _, method := range []string{MethodGet, MethodHead} {
+ for _, method := range []string{"GET", "HEAD"} {
  //restore content in case it is consumed by previous method
  if content, ok := content.(*strings.Reader); ok {
  content.Seek(0, io.SeekStart)
@@ -1198,8 +1178,7 @@ func testServeContent(t *testing.T, mode testMode) {
  if err != nil {
  t.Fatal(err)
  }
- contentOut.Reset()
- io.Copy(contentOut, res.Body)
+ io.Copy(io.Discard, res.Body)
  res.Body.Close()
  if res.StatusCode != tt.wantStatus {
  t.Errorf("test %q using %q: got status = %d; want %d", testName, method, res.StatusCode, tt.wantStatus)
@@ -1213,28 +1192,10 @@ func testServeContent(t *testing.T, mode testMode) {
  if g, e := res.Header.Get("Last-Modified"), tt.wantLastMod; g != e {
  t.Errorf("test %q using %q: got last-modified = %q, want %q", testName, method, g, e)
  }
- if g, e := contentOut.String(), tt.wantContent; e != nil && method == MethodGet && g != string(e) {
- t.Errorf("test %q using %q: got unexpected content %q, want %q", testName, method, g, e)
- }
  }
  }
 }
 
-type seekWriterTo interface {
- io.Seeker
- io.WriterTo
-}
-
-type panicOnNonWriterTo struct {
- io.Reader
- seekWriterTo
-}
-
-type panicOnWriterTo struct {
- io.ReadSeeker
- io.WriterTo
-}
-
 // Issue 12991
 func TestServerFileStatError(t *testing.T) {
  rec := httptest.NewRecorder()