[std-http] fix-header (#134)

Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/134

cmd/main.go

@@ -20,7 +20,6 @@ import (
 	"codeberg.org/codeberg/pages/server/certificates"
 	"codeberg.org/codeberg/pages/server/database"
 	"codeberg.org/codeberg/pages/server/gitea"
-	"codeberg.org/codeberg/pages/server/gzip"
 )
 
 // AllowedCorsDomains lists the domains for which Cross-Origin Resource Sharing is allowed.
@@ -89,11 +88,11 @@ func Serve(ctx *cli.Context) error {
 	}
 
 	// Create handler based on settings
-	httpsHandler := gzip.SetupCompression(server.Handler(mainDomainSuffix, rawDomain,
+	httpsHandler := server.Handler(mainDomainSuffix, rawDomain,
 		giteaClient,
 		giteaRoot, rawInfoPage,
 		BlacklistedPaths, allowedCorsDomains,
-		dnsLookupCache, canonicalDomainCache))
+		dnsLookupCache, canonicalDomainCache)
 
 	httpHandler := server.SetupHTTPACMEChallengeServer(challengeCache)
 

integration/get_test.go

@@ -50,7 +50,7 @@ func TestGetContent(t *testing.T) {
 	assert.EqualValues(t, http.StatusOK, resp.StatusCode)
 	assert.EqualValues(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
 	assert.True(t, getSize(resp.Body) > 1000)
-	assert.Len(t, resp.Header.Get("ETag"), 42)
+	assert.Len(t, resp.Header.Get("ETag"), 44)
 
 	// access branch name contains '/'
 	resp, err = getTestHTTPSClient().Get("https://blumia.localhost.mock.directory:4430/pages-server-integration-tests/@docs~main/")
@@ -60,7 +60,7 @@ func TestGetContent(t *testing.T) {
 	}
 	assert.EqualValues(t, "text/html; charset=utf-8", resp.Header.Get("Content-Type"))
 	assert.True(t, getSize(resp.Body) > 100)
-	assert.Len(t, resp.Header.Get("ETag"), 42)
+	assert.Len(t, resp.Header.Get("ETag"), 44)
 
 	// TODO: test get of non cachable content (content size > fileCacheSizeLimit)
 }
@@ -122,6 +122,18 @@ func TestLFSSupport(t *testing.T) {
 	assert.EqualValues(t, "actual value", body)
 }
 
+func TestGetOptions(t *testing.T) {
+	log.Println("=== TestGetOptions ===")
+	req, _ := http.NewRequest(http.MethodOptions, "https://mock-pages.codeberg-test.org:4430/README.md", nil)
+	resp, err := getTestHTTPSClient().Do(req)
+	assert.NoError(t, err)
+	if !assert.NotNil(t, resp) {
+		t.FailNow()
+	}
+	assert.EqualValues(t, http.StatusNoContent, resp.StatusCode)
+	assert.EqualValues(t, "GET, HEAD, OPTIONS", resp.Header.Get("Allow"))
+}
+
 func getTestHTTPSClient() *http.Client {
 	cookieJar, _ := cookiejar.New(nil)
 	return &http.Client{

server/context/context.go

@@ -10,12 +10,14 @@ import (
 type Context struct {
 	RespWriter http.ResponseWriter
 	Req        *http.Request
+	StatusCode int
 }
 
 func New(w http.ResponseWriter, r *http.Request) *Context {
 	return &Context{
 		RespWriter: w,
 		Req:        r,
+		StatusCode: http.StatusOK,
 	}
 }
 
@@ -27,10 +29,7 @@ func (c *Context) Context() stdContext.Context {
 }
 
 func (c *Context) Response() *http.Response {
-	if c.Req != nil {
+	if c.Req != nil && c.Req.Response != nil {
-		if c.Req.Response == nil {
-			c.Req.Response = &http.Response{Header: make(http.Header)}
-		}
 		return c.Req.Response
 	}
 	return nil

server/gitea/cache.go

@@ -57,7 +57,7 @@ func (f FileResponse) createHttpResponse(cacheKey string) (http.Header, int) {
 	}
 	header.Set(ETagHeader, f.ETag)
 	header.Set(ContentTypeHeader, f.MimeType)
-	header.Set(ContentLengthHeader, fmt.Sprint(len(f.Body)))
+	header.Set(ContentLengthHeader, fmt.Sprintf("%d", len(f.Body)))
 	header.Set(PagesCacheIndicatorHeader, "true")
 
 	log.Trace().Msgf("fileCache for '%s' used", cacheKey)

server/gitea/client.go

@@ -243,6 +243,7 @@ func (client *Client) getMimeTypeByExtension(resource string) string {
 	if client.forbiddenMimeTypes[mimeTypeSplit[0]] || mimeType == "" {
 		mimeType = client.defaultMimeType
 	}
+	log.Trace().Msgf("probe mime: %s", mimeType)
 	return mimeType
 }
 

server/gzip/gzip.go

@@ -1,37 +0,0 @@
-package gzip
-
-import (
-	"compress/gzip"
-	"net/http"
-	"strings"
-)
-
-type gzipResponseWriter struct {
-	Writer         *gzip.Writer
-	ResponseWriter http.ResponseWriter
-}
-
-func (gz gzipResponseWriter) Header() http.Header {
-	return gz.ResponseWriter.Header()
-}
-
-func (gz gzipResponseWriter) Write(b []byte) (int, error) {
-	return gz.Writer.Write(b)
-}
-
-func (gz gzipResponseWriter) WriteHeader(statusCode int) {
-	gz.ResponseWriter.WriteHeader(statusCode)
-}
-
-func SetupCompression(handler http.HandlerFunc) http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
-		if !strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") {
-			handler(w, r)
-			return
-		}
-		w.Header().Set("Content-Encoding", "gzip")
-		gz := gzip.NewWriter(w)
-		defer gz.Close()
-		handler(gzipResponseWriter{Writer: gz, ResponseWriter: w}, r)
-	}
-}

server/handler.go

@@ -80,7 +80,7 @@ func Handler(mainDomainSuffix, rawDomain string,
 
 		ctx.RespWriter.Header().Set("Allow", http.MethodGet+", "+http.MethodHead+", "+http.MethodOptions) // duplic 1
 		if ctx.IsMethod(http.MethodOptions) {
-			ctx.Response().StatusCode = http.StatusNoContent
+			ctx.RespWriter.WriteHeader(http.StatusNoContent)
 			return
 		}
 
@@ -119,8 +119,8 @@ func Handler(mainDomainSuffix, rawDomain string,
 
 			if canonicalLink != "" {
 				// Hide from search machines & add canonical link
-				ctx.Response().Header.Set("X-Robots-Tag", "noarchive, noindex")
+				ctx.RespWriter.Header().Set("X-Robots-Tag", "noarchive, noindex")
-				ctx.Response().Header.Set("Link",
+				ctx.RespWriter.Header().Set("Link",
 					strings.NewReplacer("%b", targetBranch, "%p", targetPath).Replace(canonicalLink)+
 						"; rel=\"canonical\"",
 				)

server/try.go

@@ -44,6 +44,6 @@ func tryUpstream(ctx *context.Context, giteaClient *gitea.Client,
 
 	// Try to request the file from the Gitea API
 	if !targetOptions.Upstream(ctx, giteaClient) {
-		html.ReturnErrorPage(ctx, "", ctx.Response().StatusCode)
+		html.ReturnErrorPage(ctx, "", ctx.StatusCode)
 	}
 }

server/upstream/upstream.go

@@ -75,12 +75,16 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client) (fin
 	}
 
 	// Check if the browser has a cached version
+	if ctx.Response() != nil {
 		if ifModifiedSince, err := time.Parse(time.RFC1123, string(ctx.Response().Header.Get(headerIfModifiedSince))); err == nil {
 			if !ifModifiedSince.Before(o.BranchTimestamp) {
 				ctx.RespWriter.WriteHeader(http.StatusNotModified)
+				log.Trace().Msg("check response against last modified: valid")
 				return true
 			}
 		}
+		log.Trace().Msg("check response against last modified: outdated")
+	}
 
 	log.Debug().Msg("Preparing")
 
@@ -91,8 +95,8 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client) (fin
 
 	log.Debug().Msg("Aquisting")
 
-	// Handle errors
+	// Handle not found error
-	if (err != nil && errors.Is(err, gitea.ErrorNotFound)) || (reader == nil) {
+	if err != nil && errors.Is(err, gitea.ErrorNotFound) {
 		if o.TryIndexPages {
 			// copy the o struct & try if an index page exists
 			optionsForIndexPages := *o
@@ -113,7 +117,7 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client) (fin
 			}
 		}
 
-		ctx.Response().StatusCode = http.StatusNotFound
+		ctx.StatusCode = http.StatusNotFound
 		if o.TryIndexPages {
 			// copy the o struct & try if a not found page exists
 			optionsForNotFoundPages := *o
@@ -128,9 +132,27 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client) (fin
 		}
 		return false
 	}
-	if reader != nil && (err != nil || statusCode != http.StatusOK) {
+
-		log.Printf("Couldn't fetch contents (status code %d): %v\n", statusCode, err)
+	// handle unexpected client errors
-		html.ReturnErrorPage(ctx, fmt.Sprintf("%v", err), http.StatusInternalServerError)
+	if err != nil || reader == nil || statusCode != http.StatusOK {
+		log.Debug().Msg("Handling error")
+		var msg string
+
+		if err != nil {
+			msg = "gitea client returned unexpected error"
+			log.Error().Err(err).Msg(msg)
+			msg = fmt.Sprintf("%s: %v", msg, err)
+		}
+		if reader == nil {
+			msg = "gitea client returned no reader"
+			log.Error().Msg(msg)
+		}
+		if statusCode != http.StatusOK {
+			msg = fmt.Sprintf("Couldn't fetch contents (status code %d)", statusCode)
+			log.Error().Msg(msg)
+		}
+
+		html.ReturnErrorPage(ctx, msg, http.StatusInternalServerError)
 		return true
 	}
 
@@ -149,26 +171,27 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client) (fin
 		return true
 	}
 
-	log.Debug().Msg("Handling error")
-
 	// Set ETag & MIME
-	ctx.Response().Header.Set(gitea.ETagHeader, header.Get(gitea.ETagHeader))
+	if eTag := header.Get(gitea.ETagHeader); eTag != "" {
-	ctx.Response().Header.Set(gitea.PagesCacheIndicatorHeader, header.Get(gitea.PagesCacheIndicatorHeader))
+		ctx.RespWriter.Header().Set(gitea.ETagHeader, eTag)
-	ctx.Response().Header.Set(gitea.ContentLengthHeader, header.Get(gitea.ContentLengthHeader))
+	}
-	if o.ServeRaw {
+	if cacheIndicator := header.Get(gitea.PagesCacheIndicatorHeader); cacheIndicator != "" {
-		ctx.Response().Header.Set(gitea.ContentTypeHeader, rawMime)
+		ctx.RespWriter.Header().Set(gitea.PagesCacheIndicatorHeader, cacheIndicator)
+	}
+	if length := header.Get(gitea.ContentLengthHeader); length != "" {
+		ctx.RespWriter.Header().Set(gitea.ContentLengthHeader, length)
+	}
+	if mime := header.Get(gitea.ContentTypeHeader); mime == "" || o.ServeRaw {
+		ctx.RespWriter.Header().Set(gitea.ContentTypeHeader, rawMime)
 	} else {
-		ctx.Response().Header.Set(gitea.ContentTypeHeader, header.Get(gitea.ContentTypeHeader))
+		ctx.RespWriter.Header().Set(gitea.ContentTypeHeader, mime)
 	}
-
+	ctx.RespWriter.Header().Set(headerLastModified, o.BranchTimestamp.In(time.UTC).Format(time.RFC1123))
-	if ctx.Response().StatusCode != http.StatusNotFound {
-		// Everything's okay so far
-		ctx.Response().StatusCode = http.StatusOK
-	}
-	ctx.Response().Header.Set(headerLastModified, o.BranchTimestamp.In(time.UTC).Format(time.RFC1123))
 
 	log.Debug().Msg("Prepare response")
 
+	ctx.RespWriter.WriteHeader(ctx.StatusCode)
+
 	// Write the response body to the original request
 	if reader != nil {
 		_, err := io.Copy(ctx.RespWriter, reader)