Fix nginx caching#68
Open
dfabulich wants to merge 5 commits intoiftechfoundation:mainfrom
Open
Conversation
For some twisted reason, nginx treats max-age=0 like no-store, refusing to store the data at all, even for conditional revalidation. max-age=1 expires after 1 second, basically the same, and allows nginx to perform conditional revalidation, even long after the content has expired.
* This unifies the proxy settings for root and the subdomains. * We don't need `proxy_cache_valid` because the node app will set `Cache-Control` headers for everything. * Set `Host` request header and `X-Cache` response header in all cases. * Most importantly, enable proxy_cache_revalidate, allowing nginx to perform conditional `GET` requests, passing the `Last-Modified` header.
Koa's `ctx.fresh` uses https://github.com/jshttp/fresh/blob/v0.5.2/index.js#L43-L49 which always honors `Cache-Control: no-cache` in the request header. That's unfortunate, because Chrome passes a `Cache-Control: no-cache` request header if you check "Disable cache" in Chrome Dev Tools, making it difficult/confusing to verify that nginx conditional requests are working. In this commit, we use `ctx.fresh` only if our `options` object allows nginx itself to support bypassing the cache. If we don't support clients bypassing the cache, then we'll return `304 Not Modified` if `if-modified-since` matches `last-modified`, regardless of what other headers the client sends.
2d78902 to
06dc928
Compare
If we fix a bug on the index pages, then the index page's `Last-Modified` date needs to change, or else nginx will continue to serve up the old buggy version (because the zip itself hasn't changed).
Contributor
Author
|
Wanted to ping about this PR. Is it ready to merge? |
Member
|
Sorry, I forgot about it. I'll take a look soon. |
Contributor
Author
|
Pinging again… |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #67
A long summary of `curl` test results
I used a very basic
options.json:{ "domain": "localhost", "subdomains": true }(with nosupport_bypassset).Initially, I tested in
curl, like this. (I've trimmed irrelevant lines.) Throughout, I checked the logs to see what the app server returned.Finally, I enabled
nginx.cache.support_bypassinoptions.jsonand repeated this test:Then, I tested "The Bat" in Chrome with
http://2ckvcbhjia.localhost/2ckvcbhjia/index.html.I tested with "Disable cache" checked and
support_bypassdisabled. I got:?lastmodBackground.jpg, which is referred to bystyle.css; app server returned 304This was 2.1 MB transferred for 20 requests.
Then, I refreshed with "Disable cache" unchecked. I got:
?lastmodBackground.jpg, which is referred to bystyle.css; app server returned 304This refresh was 3.4kb transferred for 20 requests.
If I enable
nginx.cache.support_bypassinoptions.jsonand "Disable cache" in Chrome, I find that nginx never passes "If-Modified-Since" headers, and always returnsX-Cache: BYPASS.