From d4287ef81acfcdc7f47a567ee9f776dc23ef7c51 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Fri, 12 Dec 2025 12:57:58 +0100 Subject: [PATCH] Server: Refactor Gzip exclusions to use a custom func #5384 Signed-off-by: Michael Mayer --- internal/server/gzip.go | 118 ++++++++++++++++++++++++++++++++++ internal/server/gzip_test.go | 121 +++++++++++++++++++++++++++-------- internal/server/start.go | 15 +---- 3 files changed, 213 insertions(+), 41 deletions(-) create mode 100644 internal/server/gzip.go diff --git a/internal/server/gzip.go b/internal/server/gzip.go new file mode 100644 index 000000000..95b92e4da --- /dev/null +++ b/internal/server/gzip.go @@ -0,0 +1,118 @@ +package server + +import ( + "path/filepath" + "strings" + + "github.com/gin-gonic/gin" + + "github.com/photoprism/photoprism/internal/config" +) + +// gzipExcludedExtensions contains file extensions that should never be gzip-compressed. +// These formats are already compressed or typically served as large binary payloads. +var gzipExcludedExtensions = map[string]struct{}{ + ".png": {}, + ".gif": {}, + ".jpeg": {}, + ".jpg": {}, + ".webp": {}, + ".mp3": {}, + ".mp4": {}, + ".zip": {}, + ".gz": {}, +} + +// NewGzipShouldCompressFn returns a high-performance gzip decision function for PhotoPrism. +// It mirrors the legacy exclusion rules (extensions and path prefixes) and adds targeted +// route exclusions for binary/streaming endpoints that must not be compressed. +func NewGzipShouldCompressFn(conf *config.Config) func(c *gin.Context) bool { + if conf == nil { + return func(*gin.Context) bool { return false } + } + + apiBase := conf.BaseUri(config.ApiUri) + + // Raw path fallbacks for dynamic exclusions in case FullPath is unavailable. + sharePrefix := conf.BaseUri("/s/") + photoDlPrefix := apiBase + "/photos/" + clusterThemePath := apiBase + "/cluster/theme" + + // FullPath patterns (exact match) for dynamic routes that should bypass gzip. + excludedFullPaths := map[string]struct{}{ + apiBase + "/photos/:uid/dl": {}, + apiBase + "/cluster/theme": {}, + conf.BaseUri("/s/:token/:shared/preview"): {}, + } + + // Path prefixes that should bypass gzip (prefix match on raw URL path). + excludedPrefixes := []string{ + // Health endpoints are small and frequently polled; gzip would add overhead. + conf.BaseUri("/livez"), + conf.BaseUri("/health"), + conf.BaseUri("/readyz"), + conf.BaseUri(config.ApiUri + "/t"), + conf.BaseUri(config.ApiUri + "/folders/t"), + conf.BaseUri(config.ApiUri + "/dl"), + conf.BaseUri(config.ApiUri + "/zip"), + conf.BaseUri(config.ApiUri + "/albums"), + conf.BaseUri(config.ApiUri + "/labels"), + conf.BaseUri(config.ApiUri + "/videos"), + } + + return func(c *gin.Context) bool { + if c == nil || c.Request == nil { + return false + } + + // Only compress when the client explicitly accepts gzip and the connection is not upgraded. + if !strings.Contains(strings.ToLower(c.GetHeader("Accept-Encoding")), "gzip") { + return false + } + if strings.Contains(strings.ToLower(c.GetHeader("Connection")), "upgrade") { + return false + } + + path := c.Request.URL.Path + if path == "" { + return false + } + + // Exclude known already-compressed/binary extensions. + if ext := strings.ToLower(filepath.Ext(path)); ext != "" { + if _, ok := gzipExcludedExtensions[ext]; ok { + return false + } + } + + // Exclude configured prefix groups. + for _, prefix := range excludedPrefixes { + if prefix != "" && strings.HasPrefix(path, prefix) { + return false + } + } + + // Exclude matched route patterns for dynamic endpoints. + if full := c.FullPath(); full != "" { + if _, ok := excludedFullPaths[full]; ok { + return false + } + } + + // Fallback exclusions using raw path checks for robustness. + // Note: Keep the prefix guard here (not just HasSuffix), as the frontend SPA + // wildcard route may include paths ending in "/preview" (HTML) that should + // remain compressible (e.g., "/library/.../preview"). + if path == clusterThemePath { + return false + } + if strings.HasPrefix(path, photoDlPrefix) && strings.HasSuffix(path, "/dl") { + return false + } + if strings.HasPrefix(path, sharePrefix) && strings.HasSuffix(path, "/preview") { + return false + } + + return true + } +} diff --git a/internal/server/gzip_test.go b/internal/server/gzip_test.go index b0e0dbb68..9b071e82b 100644 --- a/internal/server/gzip_test.go +++ b/internal/server/gzip_test.go @@ -26,19 +26,7 @@ func TestGzipMiddleware(t *testing.T) { r := gin.New() r.Use(gzip.Gzip( gzip.DefaultCompression, - gzip.WithExcludedExtensions([]string{ - ".png", ".gif", ".jpeg", ".jpg", ".webp", ".mp3", ".mp4", ".zip", ".gz", - }), - gzip.WithExcludedPaths([]string{ - conf.BaseUri("/health"), - conf.BaseUri(config.ApiUri + "/t"), - conf.BaseUri(config.ApiUri + "/folders/t"), - conf.BaseUri(config.ApiUri + "/dl"), - conf.BaseUri(config.ApiUri + "/zip"), - conf.BaseUri(config.ApiUri + "/albums"), - conf.BaseUri(config.ApiUri + "/labels"), - conf.BaseUri(config.ApiUri + "/videos"), - }), + gzip.WithCustomShouldCompressFn(NewGzipShouldCompressFn(conf)), )) r.GET("/ok", func(c *gin.Context) { @@ -50,12 +38,53 @@ func TestGzipMiddleware(t *testing.T) { c.String(http.StatusOK, "download") }) - t.Run("CompressesSuccessfulResponse", func(t *testing.T) { - w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/ok", nil) - req.Header.Set("Accept-Encoding", "gzip") + livezPath := conf.BaseUri("/livez") + r.GET(livezPath, func(c *gin.Context) { + c.String(http.StatusOK, "ok") + }) + healthzPath := conf.BaseUri("/healthz") + r.GET(healthzPath, func(c *gin.Context) { + c.String(http.StatusOK, "ok") + }) + + readyzPath := conf.BaseUri("/readyz") + r.GET(readyzPath, func(c *gin.Context) { + c.String(http.StatusOK, "ok") + }) + + imagePath := "/file.jpg" + r.GET(imagePath, func(c *gin.Context) { + c.String(http.StatusOK, "image") + }) + + photoDlRoute := conf.BaseUri(config.ApiUri + "/photos/:uid/dl") + r.GET(photoDlRoute, func(c *gin.Context) { + c.String(http.StatusOK, "photo") + }) + + clusterThemeRoute := conf.BaseUri(config.ApiUri + "/cluster/theme") + r.GET(clusterThemeRoute, func(c *gin.Context) { + c.String(http.StatusOK, "theme") + }) + + sharePreviewRoute := conf.BaseUri("/s/:token/:shared/preview") + r.GET(sharePreviewRoute, func(c *gin.Context) { + c.String(http.StatusOK, "preview") + }) + + doRequest := func(path string, acceptGzip bool) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", path, nil) + if acceptGzip { + req.Header.Set("Accept-Encoding", "gzip") + } r.ServeHTTP(w, req) + return w + } + + t.Run("CompressesSuccessfulResponse", func(t *testing.T) { + w := doRequest("/ok", true) require.Equal(t, http.StatusOK, w.Code) assert.Equal(t, "gzip", w.Header().Get("Content-Encoding")) @@ -68,23 +97,59 @@ func TestGzipMiddleware(t *testing.T) { require.NoError(t, err) assert.Equal(t, "hello world", string(b)) }) - t.Run("DoesNotCompressExcludedPaths", func(t *testing.T) { - w := httptest.NewRecorder() - req := httptest.NewRequest("GET", excludedPath, nil) - req.Header.Set("Accept-Encoding", "gzip") - - r.ServeHTTP(w, req) + t.Run("DoesNotCompressExcludedPrefixes", func(t *testing.T) { + w := doRequest(excludedPath, true) require.Equal(t, http.StatusOK, w.Code) assert.Empty(t, w.Header().Get("Content-Encoding")) assert.Equal(t, "download", w.Body.String()) }) - t.Run("DoesNotCompressNotFound", func(t *testing.T) { - w := httptest.NewRecorder() - req := httptest.NewRequest("GET", "/missing", nil) - req.Header.Set("Accept-Encoding", "gzip") + t.Run("DoesNotCompressExcludedExtensions", func(t *testing.T) { + w := doRequest(imagePath, true) - r.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + assert.Empty(t, w.Header().Get("Content-Encoding")) + assert.Equal(t, "image", w.Body.String()) + }) + t.Run("DoesNotCompressHealthEndpoints", func(t *testing.T) { + for _, path := range []string{livezPath, healthzPath, readyzPath} { + w := doRequest(path, true) + + require.Equal(t, http.StatusOK, w.Code, path) + assert.Empty(t, w.Header().Get("Content-Encoding"), path) + assert.Equal(t, "ok", w.Body.String(), path) + } + }) + t.Run("DoesNotCompressPhotoOriginalDownload", func(t *testing.T) { + w := doRequest(conf.BaseUri(config.ApiUri+"/photos/abc/dl"), true) + + require.Equal(t, http.StatusOK, w.Code) + assert.Empty(t, w.Header().Get("Content-Encoding")) + assert.Equal(t, "photo", w.Body.String()) + }) + t.Run("DoesNotCompressClusterThemeDownload", func(t *testing.T) { + w := doRequest(clusterThemeRoute, true) + + require.Equal(t, http.StatusOK, w.Code) + assert.Empty(t, w.Header().Get("Content-Encoding")) + assert.Equal(t, "theme", w.Body.String()) + }) + t.Run("DoesNotCompressSharePreview", func(t *testing.T) { + w := doRequest(conf.BaseUri("/s/tok/shared/preview"), true) + + require.Equal(t, http.StatusOK, w.Code) + assert.Empty(t, w.Header().Get("Content-Encoding")) + assert.Equal(t, "preview", w.Body.String()) + }) + t.Run("DoesNotCompressWithoutAcceptEncoding", func(t *testing.T) { + w := doRequest("/ok", false) + + require.Equal(t, http.StatusOK, w.Code) + assert.Empty(t, w.Header().Get("Content-Encoding")) + assert.Equal(t, "hello world", w.Body.String()) + }) + t.Run("DoesNotCompressNotFound", func(t *testing.T) { + w := doRequest("/missing", true) require.Equal(t, http.StatusNotFound, w.Code) assert.Empty(t, w.Header().Get("Content-Encoding")) diff --git a/internal/server/start.go b/internal/server/start.go index eadb88ffc..4917b96f7 100644 --- a/internal/server/start.go +++ b/internal/server/start.go @@ -78,21 +78,10 @@ func Start(ctx context.Context, conf *config.Config) { case "br", "brotli": log.Infof("server: brotli compression is currently not supported") case "gzip": + // Use a custom compression predicate for fast, targeted exclusions. router.Use(gzip.Gzip( gzip.DefaultCompression, - gzip.WithExcludedExtensions([]string{ - ".png", ".gif", ".jpeg", ".jpg", ".webp", ".mp3", ".mp4", ".zip", ".gz", - }), - gzip.WithExcludedPaths([]string{ - conf.BaseUri("/health"), - conf.BaseUri(config.ApiUri + "/t"), - conf.BaseUri(config.ApiUri + "/folders/t"), - conf.BaseUri(config.ApiUri + "/dl"), - conf.BaseUri(config.ApiUri + "/zip"), - conf.BaseUri(config.ApiUri + "/albums"), - conf.BaseUri(config.ApiUri + "/labels"), - conf.BaseUri(config.ApiUri + "/videos"), - }), + gzip.WithCustomShouldCompressFn(NewGzipShouldCompressFn(conf)), )) log.Infof("server: enabled gzip compression") }