Use standard HTTP library to serve files (#24693)

`http.ServeFile/ServeContent` handles `If-xxx`, `Content-Length`,
`Range` and `Etag` correctly

After this PR, storage files (eg: avatar) could be responded with
correct Content-Length.

modules/context/context_test.go

@@ -5,16 +5,16 @@ package context
 
 import (
 	"net/http"
+	"net/http/httptest"
 	"testing"
 
-	"code.gitea.io/gitea/modules/httplib"
 	"code.gitea.io/gitea/modules/setting"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestRemoveSessionCookieHeader(t *testing.T) {
-	w := httplib.NewMockResponseWriter()
+	w := httptest.NewRecorder()
 	w.Header().Add("Set-Cookie", (&http.Cookie{Name: setting.SessionConfig.CookieName, Value: "foo"}).String())
 	w.Header().Add("Set-Cookie", (&http.Cookie{Name: "other", Value: "bar"}).String())
 	assert.Len(t, w.Header().Values("Set-Cookie"), 2)

modules/httpcache/httpcache.go

@@ -4,10 +4,8 @@
 package httpcache
 
 import (
-	"encoding/base64"
-	"fmt"
+	"io"
 	"net/http"
-	"os"
 	"strconv"
 	"strings"
 	"time"
@@ -37,38 +35,9 @@ func SetCacheControlInHeader(h http.Header, maxAge time.Duration, additionalDire
 	h.Set("Cache-Control", strings.Join(append(directives, additionalDirectives...), ", "))
 }
 
-// generateETag generates an ETag based on size, filename and file modification time
-func generateETag(fi os.FileInfo) string {
-	etag := fmt.Sprint(fi.Size()) + fi.Name() + fi.ModTime().UTC().Format(http.TimeFormat)
-	return `"` + base64.StdEncoding.EncodeToString([]byte(etag)) + `"`
-}
-
-// HandleTimeCache handles time-based caching for a HTTP request
-func HandleTimeCache(req *http.Request, w http.ResponseWriter, fi os.FileInfo) (handled bool) {
-	return HandleGenericTimeCache(req, w, fi.ModTime())
-}
-
-// HandleGenericTimeCache handles time-based caching for a HTTP request
-func HandleGenericTimeCache(req *http.Request, w http.ResponseWriter, lastModified time.Time) (handled bool) {
+func ServeContentWithCacheControl(w http.ResponseWriter, req *http.Request, name string, modTime time.Time, content io.ReadSeeker) {
 	SetCacheControlInHeader(w.Header(), setting.StaticCacheTime)
-
-	ifModifiedSince := req.Header.Get("If-Modified-Since")
-	if ifModifiedSince != "" {
-		t, err := time.Parse(http.TimeFormat, ifModifiedSince)
-		if err == nil && lastModified.Unix() <= t.Unix() {
-			w.WriteHeader(http.StatusNotModified)
-			return true
-		}
-	}
-
-	w.Header().Set("Last-Modified", lastModified.Format(http.TimeFormat))
-	return false
-}
-
-// HandleFileETagCache handles ETag-based caching for a HTTP request
-func HandleFileETagCache(req *http.Request, w http.ResponseWriter, fi os.FileInfo) (handled bool) {
-	etag := generateETag(fi)
-	return HandleGenericETagCache(req, w, etag)
+	http.ServeContent(w, req, name, modTime, content)
 }
 
 // HandleGenericETagCache handles ETag-based caching for a HTTP request.

modules/httpcache/httpcache_test.go

@@ -6,23 +6,12 @@ package httpcache
 import (
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
-type mockFileInfo struct{}
-
-func (m mockFileInfo) Name() string       { return "gitea.test" }
-func (m mockFileInfo) Size() int64        { return int64(10) }
-func (m mockFileInfo) Mode() os.FileMode  { return os.ModePerm }
-func (m mockFileInfo) ModTime() time.Time { return time.Time{} }
-func (m mockFileInfo) IsDir() bool        { return false }
-func (m mockFileInfo) Sys() interface{}   { return nil }
-
 func countFormalHeaders(h http.Header) (c int) {
 	for k := range h {
 		// ignore our headers for internal usage
@@ -34,52 +23,6 @@ func countFormalHeaders(h http.Header) (c int) {
 	return c
 }
 
-func TestHandleFileETagCache(t *testing.T) {
-	fi := mockFileInfo{}
-	etag := `"MTBnaXRlYS50ZXN0TW9uLCAwMSBKYW4gMDAwMSAwMDowMDowMCBHTVQ="`
-
-	t.Run("No_If-None-Match", func(t *testing.T) {
-		req := &http.Request{Header: make(http.Header)}
-		w := httptest.NewRecorder()
-
-		handled := HandleFileETagCache(req, w, fi)
-
-		assert.False(t, handled)
-		assert.Equal(t, 2, countFormalHeaders(w.Header()))
-		assert.Contains(t, w.Header(), "Cache-Control")
-		assert.Contains(t, w.Header(), "Etag")
-		assert.Equal(t, etag, w.Header().Get("Etag"))
-	})
-	t.Run("Wrong_If-None-Match", func(t *testing.T) {
-		req := &http.Request{Header: make(http.Header)}
-		w := httptest.NewRecorder()
-
-		req.Header.Set("If-None-Match", `"wrong etag"`)
-
-		handled := HandleFileETagCache(req, w, fi)
-
-		assert.False(t, handled)
-		assert.Equal(t, 2, countFormalHeaders(w.Header()))
-		assert.Contains(t, w.Header(), "Cache-Control")
-		assert.Contains(t, w.Header(), "Etag")
-		assert.Equal(t, etag, w.Header().Get("Etag"))
-	})
-	t.Run("Correct_If-None-Match", func(t *testing.T) {
-		req := &http.Request{Header: make(http.Header)}
-		w := httptest.NewRecorder()
-
-		req.Header.Set("If-None-Match", etag)
-
-		handled := HandleFileETagCache(req, w, fi)
-
-		assert.True(t, handled)
-		assert.Equal(t, 1, countFormalHeaders(w.Header()))
-		assert.Contains(t, w.Header(), "Etag")
-		assert.Equal(t, etag, w.Header().Get("Etag"))
-		assert.Equal(t, http.StatusNotModified, w.Code)
-	})
-}
-
 func TestHandleGenericETagCache(t *testing.T) {
 	etag := `"test"`
 

modules/httplib/serve_test.go

@@ -6,6 +6,7 @@ package httplib
 import (
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"os"
 	"strings"
@@ -25,12 +26,12 @@ func TestServeContentByReader(t *testing.T) {
 			r.Header.Set("Range", fmt.Sprintf("bytes=%s", rangeStr))
 		}
 		reader := strings.NewReader(data)
-		w := NewMockResponseWriter()
+		w := httptest.NewRecorder()
 		ServeContentByReader(r, w, "test", int64(len(data)), reader)
-		assert.Equal(t, expectedStatusCode, w.StatusCode)
+		assert.Equal(t, expectedStatusCode, w.Code)
 		if expectedStatusCode == http.StatusPartialContent || expectedStatusCode == http.StatusOK {
 			assert.Equal(t, fmt.Sprint(len(expectedContent)), w.Header().Get("Content-Length"))
-			assert.Equal(t, expectedContent, w.BodyBuffer.String())
+			assert.Equal(t, expectedContent, w.Body.String())
 		}
 	}
 
@@ -76,12 +77,12 @@ func TestServeContentByReadSeeker(t *testing.T) {
 		}
 		defer seekReader.Close()
 
-		w := NewMockResponseWriter()
+		w := httptest.NewRecorder()
 		ServeContentByReadSeeker(r, w, "test", time.Time{}, seekReader)
-		assert.Equal(t, expectedStatusCode, w.StatusCode)
+		assert.Equal(t, expectedStatusCode, w.Code)
 		if expectedStatusCode == http.StatusPartialContent || expectedStatusCode == http.StatusOK {
 			assert.Equal(t, fmt.Sprint(len(expectedContent)), w.Header().Get("Content-Length"))
-			assert.Equal(t, expectedContent, w.BodyBuffer.String())
+			assert.Equal(t, expectedContent, w.Body.String())
 		}
 	}
 

modules/public/public.go

@@ -97,10 +97,6 @@ func handleRequest(w http.ResponseWriter, req *http.Request, fs http.FileSystem,
 		return true
 	}
 
-	if httpcache.HandleFileETagCache(req, w, fi) {
-		return true
-	}
-
 	serveContent(w, req, fi, fi.ModTime(), f)
 	return true
 }
@@ -124,11 +120,11 @@ func serveContent(w http.ResponseWriter, req *http.Request, fi os.FileInfo, modt
 				w.Header().Set("Content-Type", "application/octet-stream")
 			}
 			w.Header().Set("Content-Encoding", "gzip")
-			http.ServeContent(w, req, fi.Name(), modtime, rdGzip)
+			httpcache.ServeContentWithCacheControl(w, req, fi.Name(), modtime, rdGzip)
 			return
 		}
 	}
 
-	http.ServeContent(w, req, fi.Name(), modtime, content)
+	httpcache.ServeContentWithCacheControl(w, req, fi.Name(), modtime, content)
 	return
 }

routers/web/base.go

@@ -6,7 +6,6 @@ package web
 import (
 	"errors"
 	"fmt"
-	"io"
 	"net/http"
 	"os"
 	"path"
@@ -76,12 +75,6 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
 			}
 
 			fi, err := objStore.Stat(rPath)
-			if err == nil && httpcache.HandleTimeCache(req, w, fi) {
-				return
-			}
-
-			// If we have matched and access to release or issue
-			fr, err := objStore.Open(rPath)
 			if err != nil {
 				if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
 					log.Warn("Unable to find %s %s", prefix, rPath)
@@ -92,14 +85,15 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
 				http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError)
 				return
 			}
-			defer fr.Close()
 
-			_, err = io.Copy(w, fr)
+			fr, err := objStore.Open(rPath)
 			if err != nil {
-				log.Error("Error whilst rendering %s %s. Error: %v", prefix, rPath, err)
-				http.Error(w, fmt.Sprintf("Error whilst rendering %s %s", prefix, rPath), http.StatusInternalServerError)
+				log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err)
+				http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError)
 				return
 			}
+			defer fr.Close()
+			httpcache.ServeContentWithCacheControl(w, req, path.Base(rPath), fi.ModTime(), fr)
 		})
 	}
 }

routers/web/misc/misc.go

@@ -5,13 +5,13 @@ package misc
 
 import (
 	"net/http"
-	"os"
 	"path"
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/httpcache"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
 func SSHInfo(rw http.ResponseWriter, req *http.Request) {
@@ -34,11 +34,8 @@ func DummyOK(w http.ResponseWriter, req *http.Request) {
 }
 
 func RobotsTxt(w http.ResponseWriter, req *http.Request) {
-	filePath := path.Join(setting.CustomPath, "robots.txt")
-	fi, err := os.Stat(filePath)
-	if err == nil && httpcache.HandleTimeCache(req, w, fi) {
-		return
-	}
+	filePath := util.FilePathJoinAbs(setting.CustomPath, "robots.txt")
+	httpcache.SetCacheControlInHeader(w.Header(), setting.StaticCacheTime)
 	http.ServeFile(w, req, filePath)
 }