From 433b911c7d6ccf34fc5f4383977b6c306ed00ec7 Mon Sep 17 00:00:00 2001 From: Dan Fabulich Date: Tue, 3 Dec 2024 20:15:08 -0800 Subject: [PATCH 1/5] Log requests to the node server, to repro #67 --- app/src/app.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/src/app.js b/app/src/app.js index ebbb3db..3b71a27 100644 --- a/app/src/app.js +++ b/app/src/app.js @@ -89,6 +89,14 @@ export default class UnboxApp { this.app.use(koaBody()) + this.app.use(async (ctx, next) => { + try { + await next() + } finally { + console.log(`${ctx.method} ${ctx.url} ${ctx.status} "${ctx.headers['if-modified-since']}"`) + } + }) + // And the main handler this.app.use(this.handler.bind(this)) } From 241c79f65739b574ef2ee963d61a7c8d51e69724 Mon Sep 17 00:00:00 2001 From: Dan Fabulich Date: Tue, 3 Dec 2024 20:19:38 -0800 Subject: [PATCH 2/5] Use max-age=1, to allow nginx to perform conditional revalidation 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. --- app/src/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/app.js b/app/src/app.js index 3b71a27..0989821 100644 --- a/app/src/app.js +++ b/app/src/app.js @@ -113,7 +113,7 @@ export default class UnboxApp { // Solve CORS issues ctx.set('Access-Control-Allow-Origin', '*') - ctx.set('Cache-Control', `max-age=0`) + ctx.set('Cache-Control', `max-age=1`) // Front page if (request_path === '/') { From b5ad027e900f665b56504bffe5852d639708b408 Mon Sep 17 00:00:00 2001 From: Dan Fabulich Date: Tue, 3 Dec 2024 20:23:21 -0800 Subject: [PATCH 3/5] Enable `proxy_cache_revalidate` for both root and subdomains * 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. --- nginx/nginx.sh | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/nginx/nginx.sh b/nginx/nginx.sh index 6fec877..c294678 100755 --- a/nginx/nginx.sh +++ b/nginx/nginx.sh @@ -23,6 +23,18 @@ GZIP="gzip on; LISTEN="listen 80; listen [::]:80;" +if [ "$SUPPORT_BYPASS" = "true" ]; then + BYPASS="proxy_cache_bypass \$http_pragma;" +fi + +# Common proxy settings +PROXY="proxy_pass http://app:8080; + proxy_set_header Host \$host; + proxy_cache cache; + proxy_cache_revalidate on; + $BYPASS + add_header X-Cache \$upstream_cache_status;" + # Erase the conf file > $CONF_FILE @@ -53,8 +65,13 @@ EOF fi fi +CACHE_DIR="$DATA_DIR/nginx-cache" +mkdir -p $CACHE_DIR + # Top domain server cat >> $CONF_FILE <> $CONF_FILE < Date: Tue, 3 Dec 2024 21:02:57 -0800 Subject: [PATCH 4/5] Only allow client to force unconditional request if options permit it 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. --- app/src/app.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/app/src/app.js b/app/src/app.js index 0989821..2a64f2c 100644 --- a/app/src/app.js +++ b/app/src/app.js @@ -93,7 +93,7 @@ export default class UnboxApp { try { await next() } finally { - console.log(`${ctx.method} ${ctx.url} ${ctx.status} "${ctx.headers['if-modified-since']}"`) + console.log(`${ctx.method} ${ctx.url} ${ctx.status} "${ctx.headers['if-modified-since']}" "${ctx.headers['cache-control']}"`) } }) @@ -115,6 +115,17 @@ export default class UnboxApp { ctx.set('Cache-Control', `max-age=1`) + const ctxFresh = () => { + if (this.options.nginx?.cache?.support_bypass) { + // 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 + return ctx.fresh + } else { + const modifiedSince = ctx.request.headers['if-modified-since'] + return !!modifiedSince && modifiedSince === ctx.response.headers['last-modified'] + } + } + // Front page if (request_path === '/') { // Allow the IF Archive admins to direct us to update the index as soon as it changes @@ -214,7 +225,7 @@ export default class UnboxApp { // Send and check the Last-Modified/If-Modified-Since headers ctx.status = 200 ctx.lastModified = new Date(details.date) - if (ctx.fresh) { + if (ctxFresh()) { ctx.status = 304 return } @@ -375,7 +386,7 @@ export default class UnboxApp { // Send and check the Last-Modified/If-Modified-Since headers ctx.status = 200 ctx.lastModified = new Date(details.date) - if (ctx.fresh) { + if (ctxFresh()) { ctx.status = 304 return } From a0ea745fbdb2d2b917cc9ee866a2c6275d0a4125 Mon Sep 17 00:00:00 2001 From: Dan Fabulich Date: Fri, 6 Dec 2024 12:07:09 -0800 Subject: [PATCH 5/5] Set `Last-Modified` for index pages no earlier than app startup date 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). --- app/src/app.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/src/app.js b/app/src/app.js index 2a64f2c..59147fd 100644 --- a/app/src/app.js +++ b/app/src/app.js @@ -43,6 +43,8 @@ export default class UnboxApp { this.app = new Koa() + this.startupDate = Date.now() + // Add the layers // Catch errors @@ -224,7 +226,11 @@ export default class UnboxApp { // Send and check the Last-Modified/If-Modified-Since headers ctx.status = 200 - ctx.lastModified = new Date(details.date) + // 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). + // TODO: Use a "deployDate" instead, so restarting Node doesn't flush the index cache? + ctx.lastModified = new Date(Math.max(details.date, this.startupDate)) if (ctxFresh()) { ctx.status = 304 return