mirror of
https://github.com/photoprism/photoprism.git
synced 2026-01-23 02:24:24 +00:00
refactor(server): extract gzip exclusion helpers for better testability
- Export ShouldExcludeGzipExt for file extension checks - Extract shouldCompressGzip, clientAcceptsGzip, isConnectionUpgrade helpers - Add matchesPrefixExclusion and matchesFallbackExclusion functions - Add comprehensive unit tests for all helper functions Fixes #5384
This commit is contained in:
parent
00a6bfc858
commit
41a719255f
2 changed files with 159 additions and 55 deletions
|
|
@ -9,9 +9,9 @@ import (
|
|||
"github.com/photoprism/photoprism/internal/config"
|
||||
)
|
||||
|
||||
// gzipExcludedExtensions contains file extensions that should never be gzip-compressed.
|
||||
// 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{}{
|
||||
var GzipExcludedExtensions = map[string]struct{}{
|
||||
".png": {},
|
||||
".gif": {},
|
||||
".jpeg": {},
|
||||
|
|
@ -23,6 +23,12 @@ var gzipExcludedExtensions = map[string]struct{}{
|
|||
".gz": {},
|
||||
}
|
||||
|
||||
// ShouldExcludeGzipExt returns true if the given file extension should not be gzip-compressed.
|
||||
func ShouldExcludeGzipExt(ext string) bool {
|
||||
_, ok := GzipExcludedExtensions[strings.ToLower(ext)]
|
||||
return ok
|
||||
}
|
||||
|
||||
// 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.
|
||||
|
|
@ -61,58 +67,90 @@ func NewGzipShouldCompressFn(conf *config.Config) func(c *gin.Context) bool {
|
|||
}
|
||||
|
||||
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
|
||||
return shouldCompressGzip(c, excludedFullPaths, excludedPrefixes, clusterThemePath, photoDlPrefix, sharePrefix)
|
||||
}
|
||||
}
|
||||
|
||||
// shouldCompressGzip is the core decision logic for gzip compression.
|
||||
// It is separated from NewGzipShouldCompressFn to enable unit testing.
|
||||
func shouldCompressGzip(c *gin.Context, excludedFullPaths map[string]struct{}, excludedPrefixes []string, clusterThemePath, photoDlPrefix, sharePrefix string) bool {
|
||||
if c == nil || c.Request == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// Only compress when the client explicitly accepts gzip and the connection is not upgraded.
|
||||
if !clientAcceptsGzip(c) {
|
||||
return false
|
||||
}
|
||||
if isConnectionUpgrade(c) {
|
||||
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 ShouldExcludeGzipExt(ext) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// Exclude configured prefix groups.
|
||||
if matchesPrefixExclusion(path, excludedPrefixes) {
|
||||
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.
|
||||
if matchesFallbackExclusion(path, clusterThemePath, photoDlPrefix, sharePrefix) {
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// clientAcceptsGzip checks if the client accepts gzip encoding.
|
||||
func clientAcceptsGzip(c *gin.Context) bool {
|
||||
return strings.Contains(strings.ToLower(c.GetHeader("Accept-Encoding")), "gzip")
|
||||
}
|
||||
|
||||
// isConnectionUpgrade checks if the connection is being upgraded (e.g., WebSocket).
|
||||
func isConnectionUpgrade(c *gin.Context) bool {
|
||||
return strings.Contains(strings.ToLower(c.GetHeader("Connection")), "upgrade")
|
||||
}
|
||||
|
||||
// matchesPrefixExclusion checks if the path matches any excluded prefix.
|
||||
func matchesPrefixExclusion(path string, excludedPrefixes []string) bool {
|
||||
for _, prefix := range excludedPrefixes {
|
||||
if prefix != "" && strings.HasPrefix(path, prefix) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// matchesFallbackExclusion checks for fallback exclusions using raw path checks.
|
||||
// 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").
|
||||
func matchesFallbackExclusion(path, clusterThemePath, photoDlPrefix, sharePrefix string) bool {
|
||||
if path == clusterThemePath {
|
||||
return true
|
||||
}
|
||||
if strings.HasPrefix(path, photoDlPrefix) && strings.HasSuffix(path, "/dl") {
|
||||
return true
|
||||
}
|
||||
if strings.HasPrefix(path, sharePrefix) && strings.HasSuffix(path, "/preview") {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,72 @@ import (
|
|||
"github.com/photoprism/photoprism/internal/config"
|
||||
)
|
||||
|
||||
func TestShouldExcludeGzipExt(t *testing.T) {
|
||||
t.Run("ExcludesCompressedFormats", func(t *testing.T) {
|
||||
excludedExts := []string{".png", ".gif", ".jpeg", ".jpg", ".webp", ".mp3", ".mp4", ".zip", ".gz"}
|
||||
for _, ext := range excludedExts {
|
||||
assert.True(t, ShouldExcludeGzipExt(ext), "extension %s should be excluded", ext)
|
||||
}
|
||||
})
|
||||
t.Run("ExcludesCaseInsensitive", func(t *testing.T) {
|
||||
assert.True(t, ShouldExcludeGzipExt(".PNG"))
|
||||
assert.True(t, ShouldExcludeGzipExt(".Jpg"))
|
||||
assert.True(t, ShouldExcludeGzipExt(".GZ"))
|
||||
})
|
||||
t.Run("DoesNotExcludeTextFormats", func(t *testing.T) {
|
||||
allowedExts := []string{".html", ".css", ".js", ".json", ".xml", ".txt", ".svg"}
|
||||
for _, ext := range allowedExts {
|
||||
assert.False(t, ShouldExcludeGzipExt(ext), "extension %s should not be excluded", ext)
|
||||
}
|
||||
})
|
||||
t.Run("EmptyExtension", func(t *testing.T) {
|
||||
assert.False(t, ShouldExcludeGzipExt(""))
|
||||
})
|
||||
}
|
||||
|
||||
func TestMatchesPrefixExclusion(t *testing.T) {
|
||||
prefixes := []string{"/api/v1/dl", "/health", "/livez"}
|
||||
|
||||
t.Run("MatchesPrefix", func(t *testing.T) {
|
||||
assert.True(t, matchesPrefixExclusion("/api/v1/dl/file", prefixes))
|
||||
assert.True(t, matchesPrefixExclusion("/health", prefixes))
|
||||
assert.True(t, matchesPrefixExclusion("/healthz", prefixes))
|
||||
assert.True(t, matchesPrefixExclusion("/livez", prefixes))
|
||||
})
|
||||
t.Run("DoesNotMatchNonPrefixes", func(t *testing.T) {
|
||||
assert.False(t, matchesPrefixExclusion("/api/v1/photos", prefixes))
|
||||
assert.False(t, matchesPrefixExclusion("/other", prefixes))
|
||||
})
|
||||
t.Run("EmptyPrefixes", func(t *testing.T) {
|
||||
assert.False(t, matchesPrefixExclusion("/anything", nil))
|
||||
assert.False(t, matchesPrefixExclusion("/anything", []string{}))
|
||||
})
|
||||
t.Run("EmptyPath", func(t *testing.T) {
|
||||
assert.False(t, matchesPrefixExclusion("", prefixes))
|
||||
})
|
||||
}
|
||||
|
||||
func TestMatchesFallbackExclusion(t *testing.T) {
|
||||
clusterTheme := "/api/v1/cluster/theme"
|
||||
photoDl := "/api/v1/photos/"
|
||||
share := "/s/"
|
||||
|
||||
t.Run("MatchesClusterTheme", func(t *testing.T) {
|
||||
assert.True(t, matchesFallbackExclusion(clusterTheme, clusterTheme, photoDl, share))
|
||||
})
|
||||
t.Run("MatchesPhotoDl", func(t *testing.T) {
|
||||
assert.True(t, matchesFallbackExclusion("/api/v1/photos/abc123/dl", clusterTheme, photoDl, share))
|
||||
})
|
||||
t.Run("MatchesSharePreview", func(t *testing.T) {
|
||||
assert.True(t, matchesFallbackExclusion("/s/token/shared/preview", clusterTheme, photoDl, share))
|
||||
})
|
||||
t.Run("DoesNotMatchOtherPaths", func(t *testing.T) {
|
||||
assert.False(t, matchesFallbackExclusion("/api/v1/photos/abc123", clusterTheme, photoDl, share))
|
||||
assert.False(t, matchesFallbackExclusion("/s/token/shared/view", clusterTheme, photoDl, share))
|
||||
assert.False(t, matchesFallbackExclusion("/library/something/preview", clusterTheme, photoDl, share))
|
||||
})
|
||||
}
|
||||
|
||||
func TestGzipMiddleware(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue