From b529c116c028fc77244b45e2ec8511da590b1038 Mon Sep 17 00:00:00 2001 From: Will Norris Date: Thu, 1 May 2025 02:26:20 -0700 Subject: [PATCH] add -forceCache flag to override no-store and private directives The httpcache package is intended only to be used in private caches, so it will cache responses marked `private` like normal. However, imageproxy is a shared cache, so these response should not be cached under normal circumstances. This change introduces a potentially breaking change to start respecting the `private` cache directive in responses. This also adds a new `-forceCache` flag to ignore the `private` and `no-store` directives, and cache all responses regardless. --- README.md | 18 ++++++---- cmd/imageproxy/main.go | 2 ++ imageproxy.go | 41 +++++++++++++++++++---- imageproxy_test.go | 53 +++++++++++++++++++++++++++++- third_party/httpcache/httpcache.go | 6 ++-- 5 files changed, 104 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index b241917a3..1718fb503 100644 --- a/README.md +++ b/README.md @@ -184,15 +184,19 @@ first check an in-memory cache for an image, followed by a gcs bucket: [tiered fashion]: https://pkg.go.dev/github.com/die-net/lrucache/twotier -#### Cache Duration +#### Override Cache Directives -By default, images are cached for the duration specified in response headers. -If an image has no cache directives, or an explicit `Cache-Control: no-cache` header, -then the response is not cached. +By default, imageproxy will respect the caching directives in response headers, +including the cache duration and explicit instructions **not** to cache the response, +such as `no-store` and `private` cache-control directives. -To override the response cache directives, set a minimum time that response should be cached for. -This will ignore `no-cache` and `no-store` directives, and will set `max-age` -to the specified value if it is greater than the original `max-age` value. +You can force imageproxy to cache responses, even if they explicitly say not to, +by passing the `-forceCache` flag. Note that this is generally not recommended. + +A minimum cache duration can be set using the `-minCacheDuration` flag. This +will extend the cache duration if the response header indicates a shorter value. +If called without the `-forceCache` flag, this will have no effect on responses +with the `no-store` or `private` directives. imageproxy -cache /tmp/imageproxy -minCacheDuration 5m diff --git a/cmd/imageproxy/main.go b/cmd/imageproxy/main.go index a4cc04ddd..7738b3f85 100644 --- a/cmd/imageproxy/main.go +++ b/cmd/imageproxy/main.go @@ -47,6 +47,7 @@ var _ = flag.Bool("version", false, "Deprecated: this flag does nothing") var contentTypes = flag.String("contentTypes", "image/*", "comma separated list of allowed content types") var userAgent = flag.String("userAgent", "willnorris/imageproxy", "specify the user-agent used by imageproxy when fetching images from origin website") var minCacheDuration = flag.Duration("minCacheDuration", 0, "minimum duration to cache remote images") +var forceCache = flag.Bool("forceCache", false, "Ignore no-store and private directives in responses") func init() { flag.Var(&cache, "cache", "location to cache images (see https://github.com/willnorris/imageproxy#cache)") @@ -89,6 +90,7 @@ func main() { p.Verbose = *verbose p.UserAgent = *userAgent p.MinimumCacheDuration = *minCacheDuration + p.ForceCache = *forceCache server := &http.Server{ Addr: *addr, diff --git a/imageproxy.go b/imageproxy.go index b7f3e688e..8afe6cd6e 100644 --- a/imageproxy.go +++ b/imageproxy.go @@ -94,8 +94,13 @@ type Proxy struct { PassRequestHeaders []string // MinimumCacheDuration is the minimum duration to cache remote images. - // This will override cache-control instructions from the remote server. + // This will override cache duration from the remote server. MinimumCacheDuration time.Duration + + // ForceCache, when true, forces caching of all images, even if the + // remote server specifies 'private' or 'no-store' in the cache-control + // header. + ForceCache bool } // NewProxy constructs a new proxy. The provided http RoundTripper will be @@ -135,14 +140,40 @@ func NewProxy(transport http.RoundTripper, cache Cache) *Proxy { } // updateCacheHeaders updates the cache-control headers in the provided headers. -// It sets the cache-control max-age value to the maximum of the minimum cache -// duration, the expires header, and the max-age header. It also removes the +// +// If the cache-control header includes the 'private' directive, +// then 'no-store' is added to the header to prevent caching. +// If p.ForceCache is set, then 'private' and 'no-store' are both ignored and removed. +// +// This method also sets the cache-control max-age value to the maximum of the minimum cache +// duration, the expires header, and the max-age header. It also removes the // expires header. func (p *Proxy) updateCacheHeaders(hdr http.Header) { + cc := tphc.ParseCacheControl(hdr) + + // respect 'private' and 'no-store' directives unless ForceCache is set. + // The httpcache package ignores the 'private' directive, + // since it's not intended to be used as a shared cache. + // imageproxy IS a shared cache, so we enforce the 'private' directive ourself + // by setting 'no-store', which httpcache does respect. + if p.ForceCache { + delete(cc, "private") + delete(cc, "no-store") + hdr.Set("Cache-Control", cc.String()) + } else { + if _, ok := cc["private"]; ok { + cc["no-store"] = "" + hdr.Set("Cache-Control", cc.String()) + return + } + if _, ok := cc["no-store"]; ok { + return + } + } + if p.MinimumCacheDuration == 0 { return } - cc := tphc.ParseCacheControl(hdr) var expiresDuration time.Duration var maxAgeDuration time.Duration @@ -160,8 +191,6 @@ func (p *Proxy) updateCacheHeaders(hdr http.Header) { maxAge := max(p.MinimumCacheDuration, expiresDuration, maxAgeDuration) cc["max-age"] = fmt.Sprintf("%d", int(maxAge.Seconds())) - delete(cc, "no-cache") - delete(cc, "no-store") hdr.Set("Cache-Control", cc.String()) hdr.Del("Expires") diff --git a/imageproxy_test.go b/imageproxy_test.go index f39d00044..6e05ddd9b 100644 --- a/imageproxy_test.go +++ b/imageproxy_test.go @@ -377,6 +377,7 @@ func TestProxy_UpdateCacheHeaders(t *testing.T) { tests := []struct { name string minDuration time.Duration + forceCache bool headers http.Header want http.Header }{ @@ -398,6 +399,14 @@ func TestProxy_UpdateCacheHeaders(t *testing.T) { "Cache-Control": {"max-age=600"}, }, }, + { + name: "min duration, no header", + minDuration: 30 * time.Second, + headers: http.Header{}, + want: http.Header{ + "Cache-Control": {"max-age=30"}, + }, + }, { name: "cache control exceeds min duration", minDuration: 30 * time.Second, @@ -457,11 +466,53 @@ func TestProxy_UpdateCacheHeaders(t *testing.T) { "Cache-Control": {"max-age=3600"}, }, }, + { + name: "respect no-store", + headers: http.Header{ + "Cache-Control": {"max-age=600, no-store"}, + }, + want: http.Header{ + "Cache-Control": {"max-age=600, no-store"}, + }, + }, + { + name: "respect private", + headers: http.Header{ + "Cache-Control": {"max-age=600, private"}, + }, + want: http.Header{ + "Cache-Control": {"max-age=600, no-store, private"}, + }, + }, + { + name: "force cache, normalize directives", + forceCache: true, + headers: http.Header{ + "Cache-Control": {"MAX-AGE=600, no-store, private"}, + }, + want: http.Header{ + "Cache-Control": {"max-age=600"}, + }, + }, + { + name: "force cache with min duration", + minDuration: 1 * time.Hour, + forceCache: true, + headers: http.Header{ + "Cache-Control": {"max-age=600, private, no-store"}, + }, + want: http.Header{ + "Cache-Control": {"max-age=3600"}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - p := &Proxy{MinimumCacheDuration: tt.minDuration} + p := &Proxy{ + MinimumCacheDuration: tt.minDuration, + ForceCache: tt.forceCache, + } hdr := maps.Clone(tt.headers) p.updateCacheHeaders(hdr) diff --git a/third_party/httpcache/httpcache.go b/third_party/httpcache/httpcache.go index 654cc9cb6..34c0a99ef 100644 --- a/third_party/httpcache/httpcache.go +++ b/third_party/httpcache/httpcache.go @@ -2,6 +2,7 @@ package httpcache import ( "net/http" + "sort" "strings" ) @@ -17,9 +18,9 @@ func ParseCacheControl(headers http.Header) CacheControl { } if strings.ContainsRune(part, '=') { keyval := strings.Split(part, "=") - cc[strings.Trim(keyval[0], " ")] = strings.Trim(keyval[1], ",") + cc[strings.ToLower(strings.Trim(keyval[0], " "))] = strings.Trim(keyval[1], ",") } else { - cc[part] = "" + cc[strings.ToLower(part)] = "" } } return cc @@ -34,5 +35,6 @@ func (cc CacheControl) String() string { parts = append(parts, k+"="+v) } } + sort.StringSlice(parts).Sort() return strings.Join(parts, ", ") }