test(formats): open one of every file type the handlers claim - #55
Merged
Merged
Conversation
The handlers claim about thirty-five mime types and six of them were ever opened by a test. Audio had none at all: a fixture sat in the playground that no spec touched. An svg is the one worth having. It is the only image the viewer does not hand to the element as it came: it is fetched, run through DOMPurify and given over as a data URL, and that path exists for XSS. Nothing covered it. The fixture carries a script element, an event handler and a foreign object, and the test decodes the data URL back to check they are gone and the drawing is not. Worth saying what that test does not prove. The viewer shows an svg through an img element, and no browser runs script in one of those, so a test asserting nothing executed passes just as well with the sanitiser taken out. I wrote that test, watched it pass without the sanitiser, and dropped it. What is left fails without it. The rest is breadth: png, bmp, webp, x-icon and a real two-frame apng beside the avif, each asserted by its decoded size rather than by the element existing. Audio covers what both engines decode, including all three names a WAV arrives under, which is what #45 was about and had no test. audio/aacp is left out because Chromium answers no to it, so a fixture would only record which engine ran. Navigation listed the fixtures by hand and now lists more of them. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #55 +/- ##
=======================================
Coverage 88.62% 88.62%
=======================================
Files 39 39
Lines 2743 2743
Branches 561 561
=======================================
Hits 2431 2431
Misses 305 305
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Sep 21, 2026
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.
The handlers claim about thirty-five mime types. Six were ever opened by a test, and audio had none at all:
audio.mp3sat in the playground and no spec touched it.The svg one is the reason I started. It is the only image the viewer does not hand straight to the element: it gets fetched, run through DOMPurify and handed over as a data URL, and that path exists for XSS. Nothing covered it, so a refactor that dropped the sanitiser would have gone unnoticed. The fixture carries a
<script>, anonloadand aforeignObjectwith anonerror, and the test decodes the data URL back out of the element to check they are gone while the circle and the title survive.One thing I want to flag rather than bury, because it changed what I shipped. I first wrote a second test asserting none of that script ran. It passed. Then I removed the sanitiser to check the tests were worth having, and it still passed, because the viewer shows an svg through an
<img>and no browser runs script in one of those whatever it contains. It was asserting a browser rule, not our code. I deleted it and left a note in the spec saying so. The remaining assertions do fail with the sanitiser removed.The rest is breadth, one fixture each: png, bmp, webp, x-icon and a real two-frame apng next to the avif, each checked by the decoded size rather than by the element existing. Audio covers everything both engines decode, including all three names a WAV can arrive under, which is what #45 fixed and never had a test.
audio/aacpis claimed and deliberately not covered: Chromium answersnoto it, so a fixture would only record which engine happened to run. Same reasoning would apply to a few video codecs.Still untested after this, and not coverable here:
image/heic,image/heif,image/tiff,image/emf,image/x-xbitmapandimage/jp2. All six depend on the server rendering a preview, so they need the real-server suite we talked about rather than the playground.Navigation listed the fixtures by hand and now lists more of them.
354 unit, 80 e2e across both engines.
👾 This pull request was assisted by Claude Code, commits carry an
Assisted-bytrailer.