Open willnorris opened 9 years ago
Changes in https://github.com/willnorris/imageproxy/commit/781938ac22a3bcabeb7c7e1b5f36d857afd93084 (+https://github.com/shepik/imageproxy/commit/ab1257122f45fb203a46588ed30b2a6cca0c9331#diff-0fecf8d8a1c3b52ef05e62d6a1bf6eff) could be considered for upstream. Basically, i hard-coded "always cache image from remote_url for 14 days" (so that 2 different resizes of that image will use 1 request to the remote server, regardless of its cache policy)
was this a remote URL that wasn't setting its own cache headers?
it was.
hmm :confused: Not sure what I'd do in that case, since it hasn't actually been a problem for me personally yet. It's one thing if upstream specifically says "Cache-Control: no-cache", but if they don't have anything, then is it really a problem? And either way, you don't want to spin the CPU unnecessarily. However, it's unfortunate that it requires such deep changes into the httpcache library... I'll try and see if there's a less invasive way of achieving the same thing.
Other Cache-Control headers to consider... how should imageproxy handle these?
no-store - The cache should not store anything about the client request or server response.
no-transform - No transformations or conversions should be made to the resource. The Content-Encoding, Content-Range, Content-Type headers must not be modified by a proxy. A non- transparent proxy might, for example, convert between image formats in order to save cache space or to reduce the amount of traffic on a slow link. The no-transform directive disallows this.
(From https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#Other)
@willnorris I would like to overwrite cache-control in case none is existent - through the config:
["Cache-Control", "max-age:3600"] => adds the cache-control only if missing
["Overwrite-Cache-Control","max-age:3600"] => overwrites or adds existing cache-control
notably, we don't do anything directly with the
Cache-Control
. The httpcache library handles respecting upstream directives in terms of what gets cached, but we don't set any Cache-Control directives on downstream responses.@shepik, I see that you rewrote part of the httpcache library in 781938ac22a3bcabeb7c7e1b5f36d857afd93084, but I haven't yet looked closely at what you changed. Were these custom changes for something you're doing, or are these things that should be considered for inclusion upstream?
also /cc @jelinden who added Cache-Control support in e41beef55bf8fbfc1aeb35d296df06d2b4cb6a71 for cloudfront. I'm not sure that we want to hard-code a max-age, as opposed to respecting the max-age from upstream, but I'd be curious to hear what cloudfront needs in this regard.