fix(docs): stop a wildcard Accept resolving to markdown - #105
Merged
Conversation
Every stylesheet, script and font on docs.propgate.dev came back as a markdown 404, so the site rendered with no styles. A browser asks for a stylesheet with `text/css` plus a wildcard at q=0.1. `text/css` matches nothing we offer, so the wildcard decided it — and a wildcard matched whichever kind was first in OFFERED, which is markdown. negotiateType already defaults a missing header to HTML; a wildcard says the same thing and now takes the same default. Fixing the wildcard rather than reordering OFFERED, because a reorder would only move the bug to the next caller passing a different `offered` list. Second, structural: isAssetPath sends /_next/* and any path whose last segment has an extension straight to the assets binding, so an asset cannot be negotiated even by a client asking for markdown by name. A stylesheet has no markdown twin and never should. Checked after isCatalogPath, so /openapi.json and /v1/* keep their JSON handling. 10 new tests, all of which fail without these two changes. They use the real Chrome headers for documents, stylesheets, scripts, fonts and images, plus the bare wildcard curl sends — that last one is how anybody first checks whether a deploy worked. Also: `*/*` inside a JSDoc block closes the comment. The prose says "wildcard".
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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.
docs.propgate.devrendered with no styles. Every stylesheet, script and font came back as a markdown 404.The document itself was fine, because a browser sends
text/htmlfirst for the page. Only the subresources broke — which is exactly the shape that renders as unstyled HTML.This was #100's Worker going live for the first time; my #104 fixed the deploy that had been hiding it.
Cause
OFFEREDlists markdown, then html, then json.matches()returns true for any candidate when the range is*/*, so a wildcard resolved to whichever kind happened to be listed first.A stylesheet request is
text/cssat q=1 plus a wildcard at q=0.1.text/cssmatches nothing we offer, so the wildcard decided it, and the wildcard picked markdown.negotiateTypealready defaults a missing header to HTML. A wildcard says the same thing — "any of these is fine" — and was taking a different path.Fix, in two parts
A wildcard takes the HTML default. Not a reorder of
OFFERED: that would have hidden this rather than fixed it, and the next caller passing a differentofferedlist would meet it again. The rule belongs on the wildcard.isAssetPathbypasses negotiation entirely./_next/*, and any path whose last segment carries an extension, goes straight to the assets binding. This is the part that makes the class impossible rather than merely fixed: even with the wildcard corrected, a client that genuinely sentAccept: text/markdownfor a stylesheet would have been answered with a markdown 404, because negotiation would look for…/0a-6hmdrq391z.css.md. A stylesheet has no markdown twin and never should..mdpaths are the exception — those are the twins, and asking for one by name has to keep working.Checked after
isCatalogPath, so/openapi.jsonand/v1/*keep their JSON error handling.Tests
10 new, and I verified every one fails without these changes by disabling each branch in turn:
The headers are the real ones, written out verbatim rather than abbreviated — the bug lived in the interaction between a concrete type we do not offer and a low-q wildcard, and a tidied-up header does not have that shape.
The opposite direction is covered too:
text/markdown,text/markdown,text/html;q=0.5andtext/markdown;q=0.9with a low-q wildcard all still resolve to markdown, andtext/html,text/markdown;q=0.5still resolves to HTML.One incidental thing
*/*inside a JSDoc block closes the comment. The first version of this patch would not parse; the prose now says "wildcard" and a check confirms no block comment in these three files contains the literal.Checks
pnpm lint,pnpm check,pnpm test --force— 12/12, nothing cached. Docs suite 270 tests across 28 files.verify:workerclean.After merge
Worth confirming on the deployed site, since the failure was invisible to every local signal: