Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix nginx caching #68

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix nginx caching #68

wants to merge 5 commits into from

Conversation

dfabulich
Copy link
Contributor

@dfabulich dfabulich commented Dec 4, 2024 •
edited
Loading

Fixes #67

A long summary of `curl` test results

I used a very basic options.json: { "domain": "localhost", "subdomains": true } (with no support_bypass set).

Initially, I tested in curl, like this. (I've trimmed irrelevant lines.) Throughout, I checked the logs to see what the app server returned.

# Home page; has no last-modified header, so nginx returns 200 / Expired
% curl -o /dev/null -v http://localhost
< HTTP/1.1 200 OK
< Cache-Control: max-age=1
< X-Cache: EXPIRED

# Index page; app server returns 304, nginx returns 200 / Revalidated
% curl -o /dev/null -v "http://localhost/?url=https%3A%2F%2Fifarchive.org%2Fif-archive%2Fgames%2Fcompetition2024%2FGames%2FTHE_BAT.zip"
< HTTP/1.1 200 OK
< Cache-Control: max-age=1
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: REVALIDATED

# Index page conditional get; app server returns 304, nginx returns 304
% curl -o /dev/null -v --header "If-Modified-Since: Thu, 17 Oct 2024 03:22:28 GMT" "http://localhost/?url=https%3A%2F%2Fifarchive.org%2Fif-archive%2Fgames%2Fcompetition2024%2FGames%2FTHE_BAT.zip
< HTTP/1.1 304 Not Modified
< Cache-Control: max-age=1
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: REVALIDATED

# HTML file on subdomain; app server returns 304, nginx returns 200 / Revalidated
% curl -o /dev/null -v "http://2ckvcbhjia.localhost/2ckvcbhjia/index.html"
< HTTP/1.1 200 OK
< Cache-Control: max-age=1
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: REVALIDATED

# Conditional get of HTML file on subdomain; app server returns 304, nginx returns 304
% curl -o /dev/null -v --header "If-Modified-Since: Thu, 17 Oct 2024 03:22:28 GMT" "http://2ckvcbhjia.localhost/2ckvcbhjia/index.html"
< HTTP/1.1 304 Not Modified
< Cache-Control: max-age=1
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: REVALIDATED

# "Safe" CSS file on subdomain, which we want to redirect to main domain
# App server returns 302 with no "Last-Modified" date; nginx returns 302 / Expired
% curl -o /dev/null -v "http://2ckvcbhjia.localhost/2ckvcbhjia/interpreter/style.css"
< HTTP/1.1 302 Found
< Cache-Control: max-age=1
< Location: //localhost/2ckvcbhjia/interpreter/style.css?lastmod=1729135348000
< X-Cache: EXPIRED

# Safe CSS file on main domain with `?lastmod` parameter
# App server initially returns 200 OK with max-age=604800 (one week)
# From then on, nginx skips querying app server, returns 200 / Hit
% curl -o /dev/null -v "http://localhost/2ckvcbhjia/interpreter/style.css?lastmod=1729135348000"
< HTTP/1.1 200 OK
< Cache-Control: max-age=604800
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: HIT

# Conditional get of CSS file on main domain with `?lastmod` parameter; nginx returns 304 / Hit
% curl -o /dev/null -v --header "If-Modified-Since: Thu, 17 Oct 2024 03:22:28 GMT" "http://localhost/2ckvcbhjia/interpreter/style.css?lastmod=1729135348000"
< HTTP/1.1 304 Not Modified
< Cache-Control: max-age=604800
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: HIT

# 404 page; nginx always queries app server and returns 404 with max-age=0, no Last-Modified or X-Cache header
% curl -o /dev/null -v "http://localhost/404"
< HTTP/1.1 404 Not Found
< Cache-Control: max-age=0

Finally, I enabled nginx.cache.support_bypass in options.json and repeated this test:

# HTML file on subdomain with client headers to bypass cache
# app server returns 200 (nginx doesn't pass If-Modified-Since), nginx returns 200 / Bypass
% curl -o /dev/null -v --header "Pragma: no-cache" --header "Cache-Control: no-cache" "http://2ckvcbhjia.localhost/2ckvcbhjia/index.html"
< HTTP/1.1 200 OK
< Cache-Control: max-age=1
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: BYPASS

# Conditional get of HTML file on subdomain with client headers to bypass cache
# app server returns 200 (nginx doesn't pass If-Modified-Since), nginx returns 304 / Bypass
% curl -o /dev/null -v --header "Pragma: no-cache" --header "Cache-Control: no-cache" --header "If-Modified-Since: Thu, 17 Oct 2024 03:22:28 GMT" "http://2ckvcbhjia.localhost/2ckvcbhjia/index.html"
< HTTP/1.1 304 Not Modified
< Cache-Control: max-age=1
< Last-Modified: Thu, 17 Oct 2024 03:22:28 GMT
< X-Cache: BYPASS

Then, I tested "The Bat" in Chrome with http://2ckvcbhjia.localhost/2ckvcbhjia/index.html.

I tested with "Disable cache" checked and support_bypass disabled. I got:

  • 200 OK / X-Cache REVALIDATED for the HTML; app server returned 304
  • 302 Found / X-Cache EXPIRED for the subdomain subresources
  • 200 OK / X-Cache HIT for the main domain subresources with ?lastmod
    • I confirmed that nginx did not contact the app server for these requests
  • 200 OK / X-Cache REVALIDATED for Background.jpg, which is referred to by style.css; app server returned 304

This was 2.1 MB transferred for 20 requests.

Then, I refreshed with "Disable cache" unchecked. I got:

  • 304 Not Modified / X-Cache REVALIDATED for the HTML; app server returned 304
  • 302 Found / X-Cache EXPIRED for the subdomain subresources
  • 200 OK (from disk cache) for the main domain subresources with ?lastmod
    • I confirmed that the browser did not contact nginx for these requests
  • 304 Not Modified / X-Cache REVALIDATED for Background.jpg, which is referred to by style.css; app server returned 304

This refresh was 3.4kb transferred for 20 requests.

If I enable nginx.cache.support_bypass in options.json and "Disable cache" in Chrome, I find that nginx never passes "If-Modified-Since" headers, and always returns X-Cache: BYPASS.

@dfabulich
Log requests to the node server, to repro iftechfoundation#67
433b911
@dfabulich
Use max-age=1, to allow nginx to perform conditional revalidation
241c79f 
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.
@dfabulich
Enable proxy_cache_revalidate for both root and subdomains
b5ad027 
* 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.
@dfabulich
Only allow client to force unconditional request if options permit it
06dc928 
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.
@dfabulich
Set Last-Modified for index pages no earlier than app startup date
a0ea745 
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Caching isn't working
1 participant
@dfabulich