rack-cache
rack-cache copied to clipboard
rtomayko
X-Content-Digest header causes conflicts with downstream middleware/apps
I'm setting up a sinatra app and a client library for accessing the app, both using Rack::Cache for caching (the client using faraday_middleware: https://github.com/pengwynn/faraday_middleware/wiki/Caching).
The sinatra app sets the "X-Content-Digest" header and sends it with the rest of the request. When the client library gets the request and starts handling it via Rack::Cache, it notices that the X-Content-Digest header is set (https://github.com/rtomayko/rack-cache/blob/master/lib/rack/cache/metastore.rb#L59), so it doesn't store the entity on the client side, but still stores the meta information. Future requests from the client for cached information find the meta entry, try to open the entity, fail to open it (raising an error), which then causes Rack::Cache on the client side to pass on to the server.
Is this intended functionality to avoid caching an entity multiple times, or a bug?
I can imagine possibly wanting to be able to stack multiple Rack::Cache instances not only for this use case, but also on the server side (maybe, for instance, one with a memcached entity store and a secondary file based entity store).
Was looking into submitting a possible fix, and discovered I could just to add 'X-Content-Digest' to the list of ignored headers when setting up Rack::Cache on the client side. Maybe there just needs to be a documentation update to mention the ignore_headers option - but I'm not sure if that's the ideal solution or not.
I consider this a bug. It should be possible to use Rack::Cache on the server (e.g. as you would use Varnish) and on the client as well (e.g. as web browsers perform caching). I would vote to change Rack::Cache to work around this issue.
A potential fix - this option adds a configuration setting to strip out the offending header from Rack::Cache's output:
https://github.com/therabidbanana/rack-cache/commit/6956f97c146e43bef39f9d277bfa57b0784358c3
Not sure I like the fix just being a configuration option though - it doesn't handle the issue that some other middleware could inadvertently set the X-Content-Digest header for their own purpose, causing Rack::Cache to not be able to cache anything.
I don't think an option is the way to go. It puts the responsibility in the hands of the user, and the user doesn't care. He/she just wants it to cache stuff automatically.
