Skip to content

Match file extensions case-insensitively for Content-Type - #61

Merged
hellerve merged 1 commit into
mainfrom
claude/content-type-case-insensitive
Aug 23, 2026
Merged

hellerve merged 1 commit into
mainfrom
claude/content-type-case-insensitive

Conversation

@carpentry-agent

Copy link
Copy Markdown
Contributor

Response.content-type-for tested extensions with a cond of exact-case
web-ends-with? calls, so any path whose extension is not all lower case fell
through to application/octet-stream. IMG_1234.JPG — what every camera and
phone writes, and on the case-insensitive filesystems most authors develop on
genuinely the same file as img_1234.jpg — was handed to the browser as a
download rather than an image. The same held for .PNG, .HTML, .CSS,
.PDF and every other row of the table. nginx's types and Apache's
mime.types both fold case; so does this now.

It is reachable from the whole static-file path — Response.file,
Response.sendfile, and StaticFile.serve/mount all infer the type from
the path they are handed — so any app serving a directory of user-supplied
files hits it.

The fix

content-type-for lowers the path once with String.ascii-to-lower and
matches the (already lower-case) literals against that. The comparisons stay
on the repo's byte-indexed web-ends-with?.

The hazard worth being careful about is that a filename need not be valid
UTF-8, which is exactly why this repo carries its own web-starts-with? /
web-ends-with? instead of core's char-slicing pair (0.9.2: “a static path
whose byte length ran past its character count no longer kills the server”).
So I probed String.ascii-to-lower rather than trusting the name. It is
String.to-bytesArray.endo-map over C tolowerString.from-bytes,
i.e. byte-for-byte, and it never slices characters. Fed a string containing a
bare 0xFF — a byte no UTF-8 sequence can contain — it returns an 8-byte
string with the 0xFF intact and only the ASCII letters folded, no abort.
Bytes above 127 pass through untouched because the program starts in the C
locale and web never calls setlocale. The empty string is likewise fine.
In the same probe core's String.ends-with? answered false for a string
that does end in .jpg — the char/byte mismatch the repo's own helpers
exist to avoid.

Tests

Three assertions in test/web.carp:

  • IMG_1234.JPGimage/jpeg (uppercase)
  • /assets/Site.CsStext/css; charset=utf-8 (mixed case)
  • a name built from a raw 0xFF byte plus .JPGimage/jpeg (not valid
    UTF-8, and uppercase)

Checked for teeth: with the fix reverted and the tests kept, the suite is
315 passed, 3 failed — exactly these three and nothing else. With the fix
in place it is 318 passed, 0 failed.

Disjoint from the other open PRs

#55 rewrites web-build-response and #60 touches web-not-modified; this
touches neither — only Response.content-type-for. The CHANGELOG entry goes
at the top of the existing ## Unreleased / ### Fixed block with no
reflowing of its neighbours: #55 inserts a ### Changed section above the
### Fixed header and #60 appends below the WebSocket entry, so all three
land in different places. Verified with git merge-tree that this branch
merges cleanly against both.

docs/Response.html is regenerated for the changed doc string; the diff is
that paragraph and nothing else, and docs/index.html is untouched (still
byte-identical to docs/web_index.html).


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

Response.content-type-for tested extensions with a cond of exact-case
web-ends-with? calls, so IMG_1234.JPG -- what cameras and phones write --
fell through to application/octet-stream and a browser offered it as a
download instead of rendering it. The same held for .PNG, .HTML, .CSS,
.PDF and every other row of the table. nginx's types and Apache's
mime.types both fold case.

The path is lowered once with String.ascii-to-lower and matched against
the already-lower-case literals. That function is byte-for-byte
(to-bytes -> endo-map over C tolower -> from-bytes), so unlike core's
char-slicing starts-with?/ends-with? it cannot abort on a filename that
is not valid UTF-8; probed on a string holding a bare 0xFF, which comes
back with the byte intact and only ASCII letters folded. The comparisons
stay on the repo's byte-indexed web-ends-with? for the same reason.

Reachable from Response.file, Response.sendfile and StaticFile.serve
/mount, i.e. any app serving a directory of user-supplied files.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

carp -x test/web.carp at f56f92f: 318 passed, 0 failed, exit 0 (77s). CI green on its single test (macos-latest) job.

Teeth check, independently of the PR's: I reverted the fix by pointing all 17 web-ends-with? calls back at path, keeping the new tests. 315 passed, 3 failed — exactly the three new assertions, nothing else, and the failure messages give the before-value directly rather than by inference:

Test 'content-type-for an uppercase extension' failed:
	Expected value: 'image/jpeg', actual value: 'application/octet-stream'
Test 'content-type-for a mixed-case extension' failed:
	Expected value: 'text/css; charset=utf-8', actual value: 'application/octet-stream'
Test 'a path that is not valid UTF-8 still matches an uppercase extension' failed:
	Expected value: 'image/jpeg', actual value: 'application/octet-stream'

Findings

No bug. What I went looking for and what it does instead:

The byte-safety hazard is real and the PR handles it. I read String.ascii-to-lower in core rather than taking the probe on trust: it is String.from-bytes over Array.endo-map of Byte.tolower-, which is registered straight to C tolower on a single Byte. So it neither slices characters nor changes byte length — and byte length is what matters, because web-ends-with? (web.carp:52) takes its offsets from String.length and cuts with String.byte-slice. Lowering preserves the length exactly, so every suffix index still lines up. That is the property the whole change rests on, and it holds.

End to end, not just the pure function. Through StaticFile.mount and a real GET, against files on disk:

PHOTO.JPG   -> 200 image/jpeg
EVIL.SVG    -> 200 image/svg+xml
plain.jpg   -> 200 image/jpeg

and the edges:

empty path  -> application/octet-stream
dot only    -> application/octet-stream
just .JPG   -> image/jpeg
x.JpEg      -> image/jpeg
x.WOFF2     -> font/woff2
dir.HTML/f  -> application/octet-stream
a.css?v=2   -> application/octet-stream

A directory component that looks like an extension does not leak into the answer, and a query string still defeats the match — pre-existing, and not something this touches, since routing strips the query before the path reaches here.

One consequence worth naming out loud. This widens the set of paths served as an active type, not just a nicer one. An app serving a directory of user-supplied files now renders evil.HTML as text/html and evil.SVG as image/svg+xml (which executes script on top-level navigation) where it previously handed both over as a download. That is not a new class of exposure — the lower-case spellings already did exactly this, so the exact-case table was never a defence anyone could rely on — and matching nginx and Apache is the right behaviour. But an app that happened to be shielded by the old table for camera-written names loses that on upgrade, and the CHANGELOG entry reads purely as "gets its real content type". A clause noting that uppercase names now render rather than download would make the upgrade note honest for anyone serving uploads.

Checked and clean

  • Docs are actually fresh, not hand-edited. carp -x gendocs.carp on the branch leaves the tree clean — the committed docs/Response.html is byte-for-byte what the generator produces.
  • docs/index.html is still byte-identical to docs/web_index.html, and the change stayed off it (cmp clean), which is right since content-type-for documents under Response, not the primary module page.
  • Merge-base is 91466f7, the current origin/main, so the CHANGELOG entry is under the live ## Unreleased### Fixed and not stranded above a release.
  • Every caller of content-type-forResponse.file (web.carp:514), the three StaticFile paths (570, 577, 581) — passes a &String, and the signature is unchanged, so the widening reaches all of them and breaks none.
  • .js before .mjs in the cond is still fine: foo.mjs does not end in .js, and both rows map to the same type anyway.

Verdict: merge

Correct fix, the hazard specific to this repo (byte-indexed helpers, paths that need not be UTF-8) is the one the author actually checked, tests fail without it and only for the right reasons, and the docs regenerate clean. The only thing I would change is a clause in the CHANGELOG about uppercase names now rendering rather than downloading — not worth holding the merge for.

@hellerve
hellerve merged commit afa412c into main Aug 23, 2026
1 check passed
@hellerve
hellerve deleted the claude/content-type-case-insensitive branch August 23, 2026 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant