]> Cypherpunks repositories - gostls13.git/commitdiff
net/http: use Copy in ServeContent if CopyN not needed
authorLeo Antunes <leo@costela.net>
Sun, 30 Oct 2022 10:15:27 +0000 (10:15 +0000)
committerGopher Robot <gobot@golang.org>
Thu, 9 Mar 2023 17:49:45 +0000 (17:49 +0000)
This small PR allows optimizations made in io.Copy (like the use of
io.WriterTo) to be used in one possible path of http.ServeContent
(in case of a non-Range request).
This, in turn, allows us to skip the buffer allocation in io.Copy.

Change-Id: Ifa2ece206ecd4556aaaed15d663b65e95e00bb0a
GitHub-Last-Rev: 94fc0318145ba1bd48502564f6488aade871c301
GitHub-Pull-Request: golang/go#56480
Reviewed-on: https://go-review.googlesource.com/c/go/+/446276
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>

src/net/http/fs.go
src/net/http/fs_test.go

index 83459046bf76eab7e1c487b4e6c59de36902d6ed..7f302491aba0d591aadaf0f50ca01770d0b9bca3 100644 (file)
@@ -349,8 +349,13 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time,
 
        w.WriteHeader(code)
 
-       if r.Method != "HEAD" {
-               io.CopyN(w, sendContent, sendSize)
+       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)
+               }
        }
 }
 
index 74f7a80e279e53def134a268d405613ba3a9a3c3..ce42920123def2d398c21b1cf8718c138ff47391 100644 (file)
@@ -937,6 +937,7 @@ 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{
@@ -1152,6 +1153,24 @@ 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
@@ -1165,7 +1184,8 @@ func testServeContent(t *testing.T, mode testMode) {
                } else {
                        content = tt.content
                }
-               for _, method := range []string{"GET", "HEAD"} {
+               contentOut := &strings.Builder{}
+               for _, method := range []string{MethodGet, MethodHead} {
                        //restore content in case it is consumed by previous method
                        if content, ok := content.(*strings.Reader); ok {
                                content.Seek(0, io.SeekStart)
@@ -1191,7 +1211,8 @@ func testServeContent(t *testing.T, mode testMode) {
                        if err != nil {
                                t.Fatal(err)
                        }
-                       io.Copy(io.Discard, res.Body)
+                       contentOut.Reset()
+                       io.Copy(contentOut, 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)
@@ -1205,10 +1226,28 @@ 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()