Skip to content

test(formats): open one of every file type the handlers claim - #55

Merged
skjnldsv merged 1 commit into
mainfrom
test/svg-sanitiser
Sep 21, 2026
Merged

skjnldsv merged 1 commit into
mainfrom
test/svg-sanitiser

Conversation

@skjnldsv

Copy link
Copy Markdown
Contributor

The handlers claim about thirty-five mime types. Six were ever opened by a test, and audio had none at all: audio.mp3 sat 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>, an onload and a foreignObject with an onerror, 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/aacp is claimed and deliberately not covered: Chromium answers no to 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-xbitmap and image/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-by trailer.

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>
@skjnldsv skjnldsv added status: review Waiting for reviews type: tests 🧪 Tests related issue or changes AI assisted labels Sep 21, 2026
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.62%. Comparing base (345aaa1) to head (b2f155a).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@skjnldsv
skjnldsv merged commit 33cb410 into main Sep 21, 2026
18 checks passed
@skjnldsv
skjnldsv deleted the test/svg-sanitiser branch September 21, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI assisted status: review Waiting for reviews type: tests 🧪 Tests related issue or changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant