Cache the anonymous page, so a bot costs a file read instead of a render (#233) - #242
Merged
Merged
Conversation
…der (#233) On 2026-09-20 the front page took 62,588 requests and every one was cache=- : not one served by nginx, every one a Rails render, for 1,787 distinct clients. Assets were at 89% hit, snapshots and the Open Wall were cached, and the largest class of traffic on the site was the only large one that was not. 25,525 of those arrived from openipc.kz, openipc.ru and openipc.eu, which proxy without caching. So caching at the origin is what stops a mirror visitor costing a render, without touching three configs this repository does not own. Rails had been asking for this for days. Since #155 a GET on / answers `public, max-age=300, stale-while-revalidate=3600` and a catalogue page `max-age=3600, stale-while-revalidate=86400`, and nothing was listening. The change is a proxy_cache on `location /`, the way the three locations above it already have one. ## What this location needs that the others do not It is the one that also serves /admin and Devise, so two of the guards that are right for a gallery page are wrong here. No `proxy_hide_header Set-Cookie`. Every other cached location strips it, because those pages never set one; this location serves the sign-in form, and stripping the header there means nobody can sign in. Not *storing* such a response is what is wanted, and nginx does that by itself. No `proxy_cache_valid`. Rails is the only thing that knows about the flash, the signed-in admin and the CSRF token, so it is the only thing that can decide what may be cached; proxy_cache_valid applies to responses that say nothing, which here are exactly the ones that must not be stored. ## Two things that would have gone unnoticed `add_header` in a location replaces every inherited one, so adding X-Cache-Status would have removed the vhost's HSTS from every page on the site -- invisibly, because Rails sends a stronger one of its own. Repeated in the location, and asserted. The key keeps the query string. Only 444 of 63,032 front-page requests carry one, so there is nothing to gain by dropping it, and dropping it is what produced the bug openipc-microcache.conf documents at length. ## Verified on production / and /ru HIT; /donate, /privacy and /get-started MISS then HIT. Two HSTS headers, as before. Accept-Language still selects the language, so Vary is being honoured -- / with Accept-Language: ru answers lang="ru", which is why that header is load-bearing rather than decoration. /admin/sign_in is MISS, max-age=0 private, and still sets its session cookie. 733 tests. Six guards, each confirmed by reverting it -- after the first round of reverts turned out to be editing the assets location instead of this one, and the first version of the tests matched the comments explaining why a directive is absent rather than the directives themselves.
PR Summary by QodoCache anonymous pages at the nginx origin
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
The test grepped the whole vhost for `add_header Strict-Transport-Security`, and since this change the catch-all contains one. So removing the server-level directive left the assertion looking at the location's own copy, passed, and would have let every other location -- assets, fonts, the snapshot pages, the Open Wall -- lose the header it inherits, silently. Confirmed before fixing: with the server-level line deleted, the old test was still green. It now reads directives that belong to a `server` block rather than to any location inside it, tracked by brace depth rather than by indentation so it does not quietly stop working when someone reformats the file. Both directions fail as they should: deleting the inherited declaration, and dropping the location's repeat of it. 733 tests, no rubocop offences.
Same behaviour, three small methods instead of one that tripped four Metrics cops at once. Both revert directions still fail.
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
1 participant
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.
Closes #233.
On 2026-09-20 the front page took 62,588 requests and every one was
cache=-— not one served by nginx, every one a Rails render, for 1,787 distinct clients. Assets were at 89% hit, snapshots and the Open Wall were cached, and the largest class of traffic on the site was the only large one that was not.25,525 of those arrived from
openipc.kz,openipc.ruandopenipc.eu, which proxy without caching. So caching at the origin is what stops a mirror visitor costing a render, without touching three configs this repository does not own.Rails had been asking for this for days: since #155 a GET on
/answerspublic, max-age=300, stale-while-revalidate=3600and a catalogue pagemax-age=3600, stale-while-revalidate=86400, and nothing was listening. The change is aproxy_cacheonlocation /, the way the three locations above it already have one.Measured on production
A 125× drop on the site's largest request class.
Also verified live, being the things that could have gone badly:
/and/ruHIT;/donate,/privacy,/get-startedMISS then HIT./admin/sign_inis MISS,max-age=0, private, and still sets its session cookie — signing in works and is never cached.Accept-Language: ruon/still answerslang="ru", soVaryis being honoured. That header is load-bearing here rather than decoration: the unprefixed/really does negotiate.What this location needs that the others do not
It is the one that also serves
/adminand Devise, so two guards that are right for a gallery page are wrong here, and both would have been natural to copy.No
proxy_hide_header Set-Cookie. Every other cached location strips it, because those pages never set one. This location serves the sign-in form, and stripping the header there means nobody can sign in. Not storing such a response is what is wanted, and nginx does that by itself.No
proxy_cache_valid. Rails is the only thing that knows about the flash, the signed-in admin and the CSRF token, so it is the only thing that can decide what may be cached.proxy_cache_validapplies to responses that say nothing — which here are exactly the ones that must not be stored.Two things that would have gone unnoticed
add_headerin a location replaces every inherited one. AddingX-Cache-Statuswould have removed the vhost's HSTS from every page on the site — invisibly, because Rails sends a stronger one of its own and a browser would still have been protected. Repeated in the location, and asserted.The key keeps the query string. Only 444 of 63,032 front-page requests carry one, so there is nothing to gain by dropping it — and dropping it is what produced the bug
openipc-microcache.confdocuments at length, where a key that ignored?locale=served Russian to English readers.Verification
Six guards, each confirmed by reverting it. That took three attempts to do honestly:
proxy_cache_validandproxy_hide_header Set-Cookieare absent, and so reported the opposite of the truth. They read directives now, comments stripped.catch_allhelper matched the port-80 redirect block rather than the proxying one; it now asserts there is exactly one proxyinglocation /.microcache_language_testalready reads every cached Rails location and holds the properties that keep languages apart and admins isolated; the new location is covered by it automatically and passes.733 tests.
Already on production
The nginx half is applied — that is how #144's
push-nginx.shworks, with the repository as the source of truth — so the numbers above are real rather than projected. Backups are at*.bak.20260921-091423and it reverts in one reload.Noticed, not mine
25% of requests to
/snapshots(463 of 1,854 yesterday) end in a 60-second timeout and HTTP 408. The rate is unchanged across this deploy — 5 of 18 before the reload, 4 of 17 after — so it predates this and wants its own issue.