validate: harden the static staleness probe - #1284
Open
MDA2AV wants to merge 1 commit into
Open
Conversation
Three gaps, each demonstrated against a synthetic entry that cheats in
exactly that way, and each of which the old probe passed clean.
1. Same-length replacement. The probe used to change the content *and* the
length, which made the change as easy to detect as possible. A body cache
validated on size alone therefore passed. The replacement is now
byte-for-byte the same length, so only a cache keyed on mtime or content
sees it -- which is what a real framework cache uses.
2. The compressed path was never tested. The target was hero.webp, which has
no .br/.gz twin, and the request forced identity. After the pre-compressed
rule change that is the minority path: 15 of the 20 files are served
encoded, and the compressible text is 743 KB of the 842 KB total. An entry
could cache .br bodies forever and pass. There is now a second probe on
app.js that replaces the original and both twins together and asks with
the q-valued Accept-Encoding the load generator actually sends.
3. The window was six times longer than a measured run. DURATION is 5s, so a
cache with any TTL under 30s was never revalidated while the numbers were
taken -- 30s, 300s and never were indistinguishable. Measured: a pure 10s
TTL cache with no disk check at all passes at 30s ("served in 10s") and
fails at 2s. The framework window is now 2s, which is a tolerance for a
slow first request or an inotify debounce rather than a staleness budget.
Every compliant entry measured so far flips on the very next request --
Node, Rust, Elixir, Clojure, JVM, PHP, Lua, Perl, C++, all at 0s -- so
nothing legitimate needs the room.
Infrastructure keeps 30s. Its rule explicitly allows open_file_cache and
mmap and says nothing about following the disk, so the tier was being held to
a requirement it does not carry. None of the four configure open_file_cache
today, so they passed by luck; the first one to add it would have failed.
The comparison is now against what the server served a moment earlier rather
than against the file on disk, so it holds whether the entry answers with the
original, a pre-compressed variant, or something it compressed itself.
validate: bun 70/0, caddy 53/0.
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.
Follow-up to the rule change in #1267 / #1277, prompted by #1279: if entries are going to serve bodies from a framework cache, the probe that certifies "follows the disk" has to actually mean something.
Three gaps. Each was found by building a synthetic entry that cheats in exactly that way — all three passed the old probe clean.
1. The replacement changed length, so a size-only cache passed
The probe changed content and length, which made the change as easy to detect as possible. A body cache validated on size alone therefore never noticed.
It now replaces with byte-for-byte the same length, so only a cache keyed on mtime or content sees it — which is what a real framework cache uses. fulmine's, for instance, still passes: it compares the stat the request already paid for.
2. The compressed path was never tested at all
The target was
hero.webp— no.br/.gztwin — and the request forcedidentity.After the pre-compressed rule change that is the minority path. Validate itself reports 15 of 20 files compressed, and the compressible text is 743 KB of the 842 KB total. An entry could cache
.brbodies forever and pass.There is now a second probe on
app.jsthat replaces the original and both twins together, asking with the q-valuedAccept-Encodingthe load generator actually sends.3. The window was 6× longer than a measured run
DURATION=5s(common.sh:22). At 30s, a cache with any TTL under that was never revalidated while the numbers were taken — 30s, 300s and "never" were indistinguishable.Measured, not argued. A pure 10s TTL cache with no disk check at all:
So the framework window is now 2s: a tolerance for a slow first request or an inotify debounce, not a staleness budget. Nothing legitimate needs the room — every compliant entry measured so far flips on the very next request. Node, Rust, Elixir, Clojure, JVM, PHP, Lua, Perl, C++: all
0s, across 20 observations.It stays
HTTPARENA_STATIC_STALE_WINDOW-overridable.Infrastructure keeps 30s
Its rule explicitly allows
open_file_cacheand mmap — "serving files fast from a tuned cache is the job" — and says nothing about following the disk. The tier was being held to a requirement it does not carry. None of nginx/caddy/h2o/varnish configureopen_file_cachetoday, so they passed by luck; the first one to add it would have been failed for doing what its rule invites.Synthetic results
.brforever, reads original per requestAlso
The comparison is now against what the server served a moment earlier rather than against the file on disk, so it holds whether the entry answers with the original, a pre-compressed variant, or something it compressed itself.
validate: bun 70/0, caddy 53/0.One thing to decide
If you would rather allow 30s staleness for framework entries too, that is a one-line change — but the rule text needs to move with it. It currently says "replace a file and the next response must carry the new bytes", which 30s does not enforce.
🤖 Generated with Claude Code