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.

Author: willnorris

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
 

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,

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")

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)
 

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, ", ")
 }