feat(image): detect and translate svg, ico, jxl, jp2, psd, wmf and emf - #656
Open
andiwand wants to merge 4 commits into
Open
feat(image): detect and translate svg, ico, jxl, jp2, psd, wmf and emf#656andiwand wants to merge 4 commits into
andiwand wants to merge 4 commits into
Conversation
Seven image formats a viewer is regularly handed alongside documents had no name, so magic left them `unknown` and the open strategy's text probe reported them as plain text. Svg was missing entirely - the only svg in the tree was the svm converter's output. Naming them is nearly all it takes: `open_strategy` builds an `ImageFile` for any type whose table row says `FileCategory::image`, and the image page reads its mime type straight out of that row. Detection needed the work. The sniff head grows from 12 bytes to 1024, because two of these are not a prefix: an enhanced metafile names itself at offset 40, and an svg root element sits behind an xml prologue of no fixed length. `is_svg` walks that prologue - byte order mark, whitespace, `<?...?>`, `<!--...-->`, `<!DOCTYPE ...>` - and requires the first start tag to be `svg`. Insisting on the root element is what keeps it off an html page carrying an inline `<svg>` and off a flat opendocument, which declares the svg namespace and is not an image. An image inside a document still goes out as `image/jpg`; naming the real type would rewrite every reference output. Svg is the exception, because markup in a data url labelled `image/jpg` renders nothing at all. Also fills in `ImageFile::file_meta`, which returned an empty struct while the media, font and svm wrappers all filled theirs in - so every image, the eight that already existed included, reported no type and no mimetype. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012eZzBrhRxoXSg2zktmRy5w
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b7b2c0269
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
An svg prologue has no length limit - a generated file can carry a licence comment or a doctype with an internal subset far longer than the signature head - so a fixed head cut the scan off before the root element and the file fell through to the text probe. `svg_probe` now separates "the root element is not svg" from "the head ends inside the prologue", and the stream overload reads on while the answer is the latter, bounded at 64 KiB. Nothing else needs the extra bytes: the longest signature ends at offset 44. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012eZzBrhRxoXSg2zktmRy5w
`magic` had grown a hand written xml tokenizer - byte order mark, processing instructions, comments, doctype, namespace prefixed root - plus a 64 KiB read on loop, so that a guess about bytes could be conclusive about markup. That is the wrong layer. An svg has no signature; it is xml, and only the root element separates it from any other xml. The codebase already had the answer: `list_file_types` returns ranked candidates and `open_file` verifies them in order, which is how csv and json work - `magic` never guesses either, their constructors parse and throw. Svg now joins them. `check_svg_file` parses with pugixml and checks the root element, so an arbitrary prologue, utf-16 and malformed xml are the parser's problem rather than a scanner's. `FileType::xml` is named too, and detection reports every layer it verified: an svg comes back as `[text_file, xml, scalable_vector_graphics]`, a plain xml file as `[text_file, xml]`. Nothing opens `xml` yet - it still decodes as text - so its row claims detection only. The signature head shrinks from 1024 back to 64 bytes, which is all the enhanced metafile's offset 40 needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012eZzBrhRxoXSg2zktmRy5w
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qwdxdwz6MXBMxNyMpuDV11
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
1 participant
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.
🤖 Generated with Claude Code
Seven image formats a viewer is regularly handed alongside documents had no name, so
magicleft themunknownand the open strategy's text probe reported them as plain text. SVG was missing entirely — the only SVG in the tree was the SVM converter's output.What is added
FileTypescalable_vector_graphicssvgimage/svg+xmlwindows_iconico,curimage/vnd.microsoft.iconjpeg_xljxlimage/jxljpeg_2000jp2,jpx,jpf,j2k,jpc,j2cimage/jp2photoshop_documentpsd,psbimage/vnd.adobe.photoshopwindows_metafilewmfimage/wmfenhanced_metafileemfimage/emfNaming them is nearly all it takes:
open_strategybuilds anImageFilefor any type whose table row saysFileCategory::image, and the image page reads its MIME type straight out of that row. No new branch in the open or render path.Detection
The sniff head grows from 12 bytes to 1024, because two of these are not a prefix — an enhanced metafile names itself at offset 40, and an SVG root element sits behind an XML prologue of no fixed length. Every existing signature is unaffected:
match_magiconly ever compares the pattern's own length.is_svgwalks that prologue — byte order mark, whitespace,<?…?>,<!--…-->,<!DOCTYPE …>— and requires the first start tag to besvg, with or without a namespace prefix. Insisting on the root element is what keeps it off an HTML page carrying an inline<svg>and off a flat OpenDocument, which declares thesvgnamespace and is not an image. Both are covered byTEST(magic, not_svg).The two-byte
FF 0Abare JPEG XL codestream is the weakest signature here, so it is matched after everything else.Embedded images
An image inside a document still goes out as
image/jpg— naming the real type would rewrite every reference output we have. SVG is the exception, because markup in a data URL labelledimage/jpgrenders nothing at all.Drive-by fix
ImageFile::file_metareturned an empty struct while the media, font and SVM wrappers all filled theirs in, so every image — the eight that already existed included — reported no type and no mimetype. It now mirrors the other three.Notes
ODR_SAME_ENUMasserts catch a misordering at compile time; the Java enum has no such check..svgzis deliberately not detected — it is a gzip stream, and an extension alias detection cannot back would be misleading. UTF-16 encoded SVG is likewise not handled.psd/wmf/emfdeclaretranslate_htmlbecause the image page is written and the data URL correctly labelled, not because a browser paints them — already the situationtiffandheifare in.Test plan
test/src/internal/html/image_file_test.cpp; newmagic.image_signatures,magic.svg,magic.not_svg, including the collision guards that00 01 00 00is stilltruetype_fontandFF D8 FF DBis stilljpeg.