From 0b7b2c0269290326112a5b69a11243c8099b5608 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 3 Aug 2026 08:08:06 +0200 Subject: [PATCH 1/5] feat(image): detect and translate svg, ico, jxl, jp2, psd, wmf and emf 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, ``, ``, `` - 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 `` 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) Claude-Session: https://claude.ai/code/session_012eZzBrhRxoXSg2zktmRy5w --- apple/include/OdrCoreObjC/ODRFile.h | 8 ++ apple/src/ODRFile.mm | 8 ++ jni/java/app/opendocument/core/FileType.java | 3 +- python/src/bind_file.cpp | 10 +- src/odr/file.hpp | 17 +++ src/odr/internal/common/image_file.cpp | 7 +- src/odr/internal/file_type_table.cpp | 81 +++++++++++++ src/odr/internal/html/image_file.cpp | 10 +- src/odr/internal/magic.cpp | 97 +++++++++++++++- test/CMakeLists.txt | 1 + test/src/internal/html/image_file_test.cpp | 114 +++++++++++++++++++ test/src/internal/magic_test.cpp | 62 ++++++++++ test/src/odr_test.cpp | 2 +- 13 files changed, 413 insertions(+), 7 deletions(-) create mode 100644 test/src/internal/html/image_file_test.cpp diff --git a/apple/include/OdrCoreObjC/ODRFile.h b/apple/include/OdrCoreObjC/ODRFile.h index e0a8430e..25dabb36 100644 --- a/apple/include/OdrCoreObjC/ODRFile.h +++ b/apple/include/OdrCoreObjC/ODRFile.h @@ -73,6 +73,14 @@ typedef NS_ENUM(NSInteger, ODRFileType) { ODRFileTypeThirdGenerationPartnershipVideo, ODRFileTypeMatroskaVideo, ODRFileTypeAudioVideoInterleave, + + ODRFileTypeScalableVectorGraphics, + ODRFileTypeWindowsIcon, + ODRFileTypeJpegXl, + ODRFileTypeJpeg2000, + ODRFileTypePhotoshopDocument, + ODRFileTypeWindowsMetafile, + ODRFileTypeEnhancedMetafile, } NS_SWIFT_NAME(FileType); typedef NS_ENUM(NSInteger, ODRFileCategory) { diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index abbddfc9..af40070a 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -81,6 +81,14 @@ ODR_SAME_ENUM(ODRFileTypeMatroskaVideo, odr::FileType::matroska_video); ODR_SAME_ENUM(ODRFileTypeAudioVideoInterleave, odr::FileType::audio_video_interleave); +ODR_SAME_ENUM(ODRFileTypeScalableVectorGraphics, + odr::FileType::scalable_vector_graphics); +ODR_SAME_ENUM(ODRFileTypeWindowsIcon, odr::FileType::windows_icon); +ODR_SAME_ENUM(ODRFileTypeJpegXl, odr::FileType::jpeg_xl); +ODR_SAME_ENUM(ODRFileTypeJpeg2000, odr::FileType::jpeg_2000); +ODR_SAME_ENUM(ODRFileTypePhotoshopDocument, odr::FileType::photoshop_document); +ODR_SAME_ENUM(ODRFileTypeWindowsMetafile, odr::FileType::windows_metafile); +ODR_SAME_ENUM(ODRFileTypeEnhancedMetafile, odr::FileType::enhanced_metafile); ODR_SAME_ENUM(ODRFileCategoryUnknown, odr::FileCategory::unknown); ODR_SAME_ENUM(ODRFileCategoryText, odr::FileCategory::text); diff --git a/jni/java/app/opendocument/core/FileType.java b/jni/java/app/opendocument/core/FileType.java index 4e1c77af..d8e76bde 100644 --- a/jni/java/app/opendocument/core/FileType.java +++ b/jni/java/app/opendocument/core/FileType.java @@ -15,7 +15,8 @@ public enum FileType { HIGH_EFFICIENCY_IMAGE_FORMAT, AV1_IMAGE_FILE_FORMAT, MPEG_AUDIO, MPEG4_AUDIO, OGG_AUDIO, WAVEFORM_AUDIO, FREE_LOSSLESS_AUDIO_CODEC, MPEG4_VIDEO, QUICKTIME_VIDEO, THIRD_GENERATION_PARTNERSHIP_VIDEO, MATROSKA_VIDEO, - AUDIO_VIDEO_INTERLEAVE; + AUDIO_VIDEO_INTERLEAVE, SCALABLE_VECTOR_GRAPHICS, WINDOWS_ICON, JPEG_XL, + JPEG_2000, PHOTOSHOP_DOCUMENT, WINDOWS_METAFILE, ENHANCED_METAFILE; static FileType fromNative(int code) { return code < 0 ? null : values()[code]; diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index 826da09c..1bb19085 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -73,7 +73,15 @@ void odr_python::bind_file(py::module_ &m) { .value("third_generation_partnership_video", odr::FileType::third_generation_partnership_video) .value("matroska_video", odr::FileType::matroska_video) - .value("audio_video_interleave", odr::FileType::audio_video_interleave); + .value("audio_video_interleave", odr::FileType::audio_video_interleave) + .value("scalable_vector_graphics", + odr::FileType::scalable_vector_graphics) + .value("windows_icon", odr::FileType::windows_icon) + .value("jpeg_xl", odr::FileType::jpeg_xl) + .value("jpeg_2000", odr::FileType::jpeg_2000) + .value("photoshop_document", odr::FileType::photoshop_document) + .value("windows_metafile", odr::FileType::windows_metafile) + .value("enhanced_metafile", odr::FileType::enhanced_metafile); py::enum_(m, "FileCategory") .value("unknown", odr::FileCategory::unknown) diff --git a/src/odr/file.hpp b/src/odr/file.hpp index 5ad85d6d..64c3770f 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -126,6 +126,23 @@ enum class FileType { matroska_video, // https://en.wikipedia.org/wiki/Audio_Video_Interleave audio_video_interleave, + + // More images that arrive alongside documents, named the same way and for + // the same reason as the block above - nothing here is decoded either. + // https://en.wikipedia.org/wiki/SVG + scalable_vector_graphics, + // https://en.wikipedia.org/wiki/ICO_(file_format) + windows_icon, + // https://en.wikipedia.org/wiki/JPEG_XL + jpeg_xl, + // https://en.wikipedia.org/wiki/JPEG_2000 + jpeg_2000, + // https://en.wikipedia.org/wiki/Adobe_Photoshop#File_format + photoshop_document, + // https://en.wikipedia.org/wiki/Windows_Metafile + windows_metafile, + // https://en.wikipedia.org/wiki/Windows_Metafile#Enhanced_Metafile + enhanced_metafile, }; /// @brief Collection of file categories. diff --git a/src/odr/internal/common/image_file.cpp b/src/odr/internal/common/image_file.cpp index 43bb9566..adbfd0e8 100644 --- a/src/odr/internal/common/image_file.cpp +++ b/src/odr/internal/common/image_file.cpp @@ -15,7 +15,12 @@ std::shared_ptr ImageFile::file() const noexcept { FileType ImageFile::file_type() const noexcept { return m_file_type; } -FileMeta ImageFile::file_meta() const noexcept { return {}; } +FileMeta ImageFile::file_meta() const noexcept { + FileMeta result; + result.type = file_type(); + result.mimetype = mimetype(); + return result; +} std::string_view ImageFile::mimetype() const noexcept { // not `mimetype_by_file_type` — that throws, and this is `noexcept` diff --git a/src/odr/internal/file_type_table.cpp b/src/odr/internal/file_type_table.cpp index c0e3cae9..36b213b4 100644 --- a/src/odr/internal/file_type_table.cpp +++ b/src/odr/internal/file_type_table.cpp @@ -176,6 +176,34 @@ constexpr std::array heif_mimetypes{"image/heic"sv, "image/heif"sv, constexpr std::array avif_extensions{"avif"sv, "avifs"sv}; constexpr std::array avif_mimetypes{"image/avif"sv, "image/avif-sequence"sv}; +constexpr std::array svg_extensions{"svg"sv}; +constexpr std::array svg_mimetypes{"image/svg+xml"sv}; + +// `.cur` is the same container with a different resource type, so it rides +// along here rather than becoming a type of its own +constexpr std::array ico_extensions{"ico"sv, "cur"sv}; +constexpr std::array ico_mimetypes{"image/vnd.microsoft.icon"sv, + "image/x-icon"sv}; + +constexpr std::array jxl_extensions{"jxl"sv}; +constexpr std::array jxl_mimetypes{"image/jxl"sv}; + +constexpr std::array jp2_extensions{"jp2"sv, "jpx"sv, "jpf"sv, + "j2k"sv, "jpc"sv, "j2c"sv}; +constexpr std::array jp2_mimetypes{"image/jp2"sv, "image/jpx"sv}; + +// `.psb` is the large document variant and carries the same signature +constexpr std::array psd_extensions{"psd"sv, "psb"sv}; +constexpr std::array psd_mimetypes{"image/vnd.adobe.photoshop"sv, + "application/x-photoshop"sv}; + +constexpr std::array wmf_extensions{"wmf"sv}; +constexpr std::array wmf_mimetypes{"image/wmf"sv, "image/x-wmf"sv, + "application/x-msmetafile"sv}; + +constexpr std::array emf_extensions{"emf"sv}; +constexpr std::array emf_mimetypes{"image/emf"sv, "image/x-emf"sv}; + constexpr std::array mp3_extensions{"mp3"sv}; constexpr std::array mp3_mimetypes{"audio/mpeg"sv, "audio/mp3"sv, "audio/x-mpeg"sv}; @@ -579,6 +607,59 @@ constexpr std::array table{ FileCategory::video, DocumentType::unknown, {.detect_by_content = true, .open = true, .translate_html = true}}, + + // Named but not decoded, like the images above. `translate_html` says the + // image page is written and the data url is labelled with the type below, + // not that every browser paints it - that is already true of tiff and heif. + Row{FileType::scalable_vector_graphics, + "svg"sv, + svg_extensions, + svg_mimetypes, + FileCategory::image, + DocumentType::unknown, + {.detect_by_content = true, .open = true, .translate_html = true}}, + Row{FileType::windows_icon, + "ico"sv, + ico_extensions, + ico_mimetypes, + FileCategory::image, + DocumentType::unknown, + {.detect_by_content = true, .open = true, .translate_html = true}}, + Row{FileType::jpeg_xl, + "jxl"sv, + jxl_extensions, + jxl_mimetypes, + FileCategory::image, + DocumentType::unknown, + {.detect_by_content = true, .open = true, .translate_html = true}}, + Row{FileType::jpeg_2000, + "jp2"sv, + jp2_extensions, + jp2_mimetypes, + FileCategory::image, + DocumentType::unknown, + {.detect_by_content = true, .open = true, .translate_html = true}}, + Row{FileType::photoshop_document, + "psd"sv, + psd_extensions, + psd_mimetypes, + FileCategory::image, + DocumentType::unknown, + {.detect_by_content = true, .open = true, .translate_html = true}}, + Row{FileType::windows_metafile, + "wmf"sv, + wmf_extensions, + wmf_mimetypes, + FileCategory::image, + DocumentType::unknown, + {.detect_by_content = true, .open = true, .translate_html = true}}, + Row{FileType::enhanced_metafile, + "emf"sv, + emf_extensions, + emf_mimetypes, + FileCategory::image, + DocumentType::unknown, + {.detect_by_content = true, .open = true, .translate_html = true}}, }; /// Finds the row whose list, selected by @p list, contains @p needle. diff --git a/src/odr/internal/html/image_file.cpp b/src/odr/internal/html/image_file.cpp index f8f4475f..4431cef4 100644 --- a/src/odr/internal/html/image_file.cpp +++ b/src/odr/internal/html/image_file.cpp @@ -158,8 +158,14 @@ void html::translate_image_src(const ImageFile &image_file, std::ostream &out, // TODO hacky - `image/jpg` works for all common image types in chrome. // An image inside a document keeps it: browsers sniff ``, and naming // the real type here would rewrite every reference output we have. The - // standalone image page does name it - see `image_mime_type`. - write_image_src(image_file, out, "image/jpg"); + // standalone image page does name it - see `image_mime_type`. Svg is the one + // exception: it is markup, not an image a browser sniffs, so a data url + // labelled `image/jpg` renders nothing at all. + const std::string mime_type = + image_file.file_type() == FileType::scalable_vector_graphics + ? "image/svg+xml" + : "image/jpg"; + write_image_src(image_file, out, mime_type); } HtmlService html::create_image_service(const ImageFile &image_file, diff --git a/src/odr/internal/magic.cpp b/src/odr/internal/magic.cpp index 4a288939..02a71d3f 100644 --- a/src/odr/internal/magic.cpp +++ b/src/odr/internal/magic.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include namespace odr::internal { @@ -83,6 +84,64 @@ FileType iso_base_media_file_type(const std::string &head) { return FileType::mpeg4_video; } +/// Whether @p head opens an svg document: an xml prologue - byte order mark, +/// whitespace, ``, ``, `` - and then a root +/// element named `svg`, with or without a namespace prefix. +/// +/// The root element is what makes this safe: a flat opendocument and an html +/// page carrying an inline `` both open with a different one. +bool is_svg(std::string_view head) { + static constexpr std::string_view byte_order_mark = "\xEF\xBB\xBF"; + static constexpr std::string_view whitespace = " \t\r\n"; + + if (head.starts_with(byte_order_mark)) { + head.remove_prefix(byte_order_mark.size()); + } + + // skips the prologue; a part of it that does not end inside the head means + // we cannot tell, which is not an svg + while (true) { + const std::size_t begin = head.find_first_not_of(whitespace); + if (begin == std::string_view::npos) { + return false; + } + head.remove_prefix(begin); + + std::string_view terminator; + if (head.starts_with(""; + } else if (head.starts_with(""; + } else if (head.starts_with(""; + } else { + break; + } + + const std::size_t end = head.find(terminator); + if (end == std::string_view::npos) { + return false; + } + head.remove_prefix(end + terminator.size()); + } + + if (!head.starts_with("<")) { + return false; + } + head.remove_prefix(1); + + const std::size_t name_end = head.find_first_of(" \t\r\n/>"); + if (name_end == std::string_view::npos) { // the name runs past the head + return false; + } + std::string_view name = head.substr(0, name_end); + if (const std::size_t colon = name.find(':'); + colon != std::string_view::npos) { + name.remove_prefix(colon + 1); + } + return name == "svg"; +} + } // namespace FileType magic::file_type(const std::string &magic) { @@ -161,11 +220,47 @@ FileType magic::file_type(const std::string &magic) { return FileType::mpeg_audio; } + if (match_magic(magic, "00 00 01 00") || // icon + match_magic(magic, "00 00 02 00")) { // cursor + return FileType::windows_icon; + } + if (match_magic(magic, "00 00 00 0C 6A 50 20 20 0D 0A 87 0A") || // 'jP ' box + match_magic(magic, "FF 4F FF 51")) { // bare codestream + return FileType::jpeg_2000; + } + if (match_magic(magic, "00 00 00 0C 4A 58 4C 20 0D 0A 87 0A")) { // 'JXL ' box + return FileType::jpeg_xl; + } + if (match_magic(magic, "38 42 50 53")) { // '8BPS' + return FileType::photoshop_document; + } + if (match_magic(magic, "D7 CD C6 9A") || // aldus placeable header + match_magic(magic, "01 00 09 00 00 03") || // memory metafile header + match_magic(magic, "02 00 09 00 00 03")) { // disk metafile header + return FileType::windows_metafile; + } + // the leading record type alone is far too weak, so the header signature at + // offset 40 has to agree + if (match_magic(magic, "01 00 00 00") && tag_at(magic, 40) == " EMF") { + return FileType::enhanced_metafile; + } + // two bytes and nothing more to check, so it goes after everything else + if (match_magic(magic, "FF 0A")) { // bare codestream + return FileType::jpeg_xl; + } + + if (is_svg(magic)) { + return FileType::scalable_vector_graphics; + } + return FileType::unknown; } FileType magic::file_type(std::istream &in) { - static constexpr std::size_t max_head_size = 12; + // most signatures are a prefix, but two are not: an enhanced metafile names + // itself at offset 40, and an svg root element sits behind a prologue of no + // fixed length + static constexpr std::size_t max_head_size = 1024; // value initialized, and cut back to what was actually read: a file shorter // than the longest signature would otherwise be matched against whatever the diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6667c62a..8907510f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -37,6 +37,7 @@ add_executable(odr_test "src/table_position_test.cpp" "src/internal/html/common_test.cpp" + "src/internal/html/image_file_test.cpp" "src/internal/html/media_file_test.cpp" "src/internal/magic_test.cpp" diff --git a/test/src/internal/html/image_file_test.cpp b/test/src/internal/html/image_file_test.cpp new file mode 100644 index 00000000..81c25159 --- /dev/null +++ b/test/src/internal/html/image_file_test.cpp @@ -0,0 +1,114 @@ +#include +#include +#include + +#include +#include + +#include + +#include +#include +#include + +using namespace odr; + +namespace { + +/// Signature plus payload. Nothing past the signature is ever read - these +/// formats are named, not decoded. +File image_file(const std::string &content) { + return File(std::make_shared(content)); +} + +File svg_file() { + return image_file(R"()" + "\n" + R"()"); +} + +File ico_file() { return image_file(std::string("\x00\x00\x01\x00", 4) + "d"); } + +File png_file() { + return image_file(std::string("\x89PNG\r\n\x1a\n", 8) + "payload"); +} + +std::string cache_path(const std::string &name) { + return (std::filesystem::current_path() / name).string(); +} + +std::string write_path(const HtmlService &service, const std::string &path) { + std::ostringstream out; + service.write(path, out); + return out.str(); +} + +std::string image_src(const File &file) { + std::ostringstream out; + internal::html::translate_image_src(DecodedFile(file).as_image_file(), out, + HtmlConfig()); + return out.str(); +} + +} // namespace + +TEST(image_file, svg_is_detected_and_opens_as_an_image) { + const DecodedFile file{svg_file()}; + + EXPECT_EQ(file.file_type(), FileType::scalable_vector_graphics); + EXPECT_EQ(file.file_category(), FileCategory::image); + EXPECT_EQ(file.file_meta().mimetype, "image/svg+xml"); + EXPECT_TRUE(file.capabilities().translate_html); + + EXPECT_TRUE(file.is_image_file()); + EXPECT_FALSE(file.is_decodable()); + EXPECT_FALSE(file.is_text_file()); +} + +TEST(image_file, svg_translates_to_an_image_page) { + const DecodedFile file{svg_file()}; + const HtmlService service = + html::translate(file, cache_path("image_svg"), HtmlConfig()); + + ASSERT_EQ(service.list_views().size(), 1); + EXPECT_EQ(service.list_views().front().name(), "image"); + + const std::string html = write_path(service, "image.html"); + EXPECT_NE(html.find(""s), + FileType::scalable_vector_graphics); + EXPECT_EQ(detect(""s), FileType::scalable_vector_graphics); + EXPECT_EQ(detect("\n"s), + FileType::scalable_vector_graphics); + EXPECT_EQ(detect("\xef\xbb\xbf\n" + "\n" + "\n" + ""s), + FileType::scalable_vector_graphics); +} + +TEST(magic, not_svg) { + using namespace std::string_literals; + + // an inline `` does not make an html page an image + EXPECT_EQ(detect("\n"s), + FileType::unknown); + // a flat opendocument declares the svg namespace and is still not an svg + EXPECT_EQ(detect("\n"s), + FileType::unknown); + // a prologue or a root element name that runs past the head tells us nothing + EXPECT_EQ(detect(" every_file_type() { std::vector result; for (auto i = static_cast(FileType::unknown); - i <= static_cast(FileType::audio_video_interleave); ++i) { + i <= static_cast(FileType::enhanced_metafile); ++i) { result.push_back(static_cast(i)); } return result; From 875c81197fb34a6d2d7d5b6292ee365bdcb4eb1d Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 3 Aug 2026 08:36:45 +0200 Subject: [PATCH 2/5] fix(magic): keep reading until an svg's root element is reached 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) Claude-Session: https://claude.ai/code/session_012eZzBrhRxoXSg2zktmRy5w --- src/odr/internal/magic.cpp | 84 ++++++++++++++++++++++---------- test/src/internal/magic_test.cpp | 19 ++++++++ 2 files changed, 76 insertions(+), 27 deletions(-) diff --git a/src/odr/internal/magic.cpp b/src/odr/internal/magic.cpp index 02a71d3f..d32a6538 100644 --- a/src/odr/internal/magic.cpp +++ b/src/odr/internal/magic.cpp @@ -9,7 +9,6 @@ #include #include -#include #include #include #include @@ -19,6 +18,15 @@ namespace odr::internal { namespace { +/// At most @p size bytes, cut back to what was actually read - a file shorter +/// than the longest signature must never be matched against what sat behind it. +std::string read_head(std::istream &in, const std::size_t size) { + std::string result(size, '\0'); + in.read(result.data(), static_cast(size)); + result.resize(static_cast(in.gcount())); + return result; +} + bool match_magic(const std::string &head, const std::string &pattern) { const auto bytes = util::string::split(pattern, " "); if (bytes.size() > head.size()) { @@ -84,26 +92,33 @@ FileType iso_base_media_file_type(const std::string &head) { return FileType::mpeg4_video; } +enum class SvgProbe { + no, ///< the root element is there and is not `svg` + yes, ///< the root element is `svg` + incomplete, ///< the head ends inside the prologue or the root element name +}; + /// Whether @p head opens an svg document: an xml prologue - byte order mark, /// whitespace, ``, ``, `` - and then a root /// element named `svg`, with or without a namespace prefix. /// /// The root element is what makes this safe: a flat opendocument and an html -/// page carrying an inline `` both open with a different one. -bool is_svg(std::string_view head) { +/// page carrying an inline `` both open with a different one. A prologue +/// has no length limit, so a head that ends inside one answers `incomplete` +/// rather than `no` - the caller decides whether to read further. +SvgProbe svg_probe(std::string_view head) { static constexpr std::string_view byte_order_mark = "\xEF\xBB\xBF"; static constexpr std::string_view whitespace = " \t\r\n"; + static constexpr std::string_view name_end = " \t\r\n/>"; if (head.starts_with(byte_order_mark)) { head.remove_prefix(byte_order_mark.size()); } - // skips the prologue; a part of it that does not end inside the head means - // we cannot tell, which is not an svg while (true) { const std::size_t begin = head.find_first_not_of(whitespace); if (begin == std::string_view::npos) { - return false; + return SvgProbe::incomplete; } head.remove_prefix(begin); @@ -114,32 +129,35 @@ bool is_svg(std::string_view head) { terminator = "-->"; } else if (head.starts_with(""; + } else if (head.size() < 4 && head.starts_with("<")) { + // too short to tell which of the three it is, or whether it is one + return SvgProbe::incomplete; } else { break; } const std::size_t end = head.find(terminator); if (end == std::string_view::npos) { - return false; + return SvgProbe::incomplete; } head.remove_prefix(end + terminator.size()); } if (!head.starts_with("<")) { - return false; + return SvgProbe::no; } head.remove_prefix(1); - const std::size_t name_end = head.find_first_of(" \t\r\n/>"); - if (name_end == std::string_view::npos) { // the name runs past the head - return false; + const std::size_t end = head.find_first_of(name_end); + if (end == std::string_view::npos) { + return SvgProbe::incomplete; } - std::string_view name = head.substr(0, name_end); + std::string_view name = head.substr(0, end); if (const std::size_t colon = name.find(':'); colon != std::string_view::npos) { name.remove_prefix(colon + 1); } - return name == "svg"; + return name == "svg" ? SvgProbe::yes : SvgProbe::no; } } // namespace @@ -249,7 +267,7 @@ FileType magic::file_type(const std::string &magic) { return FileType::jpeg_xl; } - if (is_svg(magic)) { + if (svg_probe(magic) == SvgProbe::yes) { return FileType::scalable_vector_graphics; } @@ -257,19 +275,31 @@ FileType magic::file_type(const std::string &magic) { } FileType magic::file_type(std::istream &in) { - // most signatures are a prefix, but two are not: an enhanced metafile names - // itself at offset 40, and an svg root element sits behind a prologue of no - // fixed length - static constexpr std::size_t max_head_size = 1024; - - // value initialized, and cut back to what was actually read: a file shorter - // than the longest signature would otherwise be matched against whatever the - // stack held behind it - std::array head{}; - in.read(head.data(), head.size()); - - return file_type( - std::string(head.data(), static_cast(in.gcount()))); + // enough for every signature - the longest is the enhanced metafile's, which + // ends at offset 44 - and for the prologue of a normal svg + static constexpr std::size_t head_size = 1024; + // a prologue has no length limit, so reading up to the root element needs a + // bound of its own; a licence comment or a doctype with an internal subset + // fits many times over + static constexpr std::size_t max_svg_head_size = 64 * head_size; + + std::string head = read_head(in, head_size); + if (const FileType file_type = magic::file_type(head); + file_type != FileType::unknown) { + return file_type; + } + + while (svg_probe(head) == SvgProbe::incomplete && + head.size() < max_svg_head_size) { + const std::string next = read_head(in, head_size); + if (next.empty()) { + break; + } + head += next; + } + + return svg_probe(head) == SvgProbe::yes ? FileType::scalable_vector_graphics + : FileType::unknown; } FileType magic::file_type(const abstract::File &file) { diff --git a/test/src/internal/magic_test.cpp b/test/src/internal/magic_test.cpp index 515c3232..10c7d4de 100644 --- a/test/src/internal/magic_test.cpp +++ b/test/src/internal/magic_test.cpp @@ -173,6 +173,25 @@ TEST(magic, svg) { FileType::scalable_vector_graphics); } +/// A prologue has no length limit - a generated file can carry a licence +/// comment far longer than the signature head - so detection reads on until it +/// reaches the root element. +TEST(magic, svg_behind_a_long_prologue) { + using namespace std::string_literals; + + const std::string comment = "\n"; + EXPECT_EQ(detect(R"()" + "\n" + + comment + ""), + FileType::scalable_vector_graphics); + + // the same length of prologue in front of something that is not an svg + EXPECT_EQ(detect(comment + ""), FileType::unknown); + + // reading on is bounded: a comment that never ends is not an svg + EXPECT_EQ(detect("`, `` - and then a root -/// element named `svg`, with or without a namespace prefix. -/// -/// The root element is what makes this safe: a flat opendocument and an html -/// page carrying an inline `` both open with a different one. A prologue -/// has no length limit, so a head that ends inside one answers `incomplete` -/// rather than `no` - the caller decides whether to read further. -SvgProbe svg_probe(std::string_view head) { - static constexpr std::string_view byte_order_mark = "\xEF\xBB\xBF"; - static constexpr std::string_view whitespace = " \t\r\n"; - static constexpr std::string_view name_end = " \t\r\n/>"; - - if (head.starts_with(byte_order_mark)) { - head.remove_prefix(byte_order_mark.size()); - } - - while (true) { - const std::size_t begin = head.find_first_not_of(whitespace); - if (begin == std::string_view::npos) { - return SvgProbe::incomplete; - } - head.remove_prefix(begin); - - std::string_view terminator; - if (head.starts_with(""; - } else if (head.starts_with(""; - } else if (head.starts_with(""; - } else if (head.size() < 4 && head.starts_with("<")) { - // too short to tell which of the three it is, or whether it is one - return SvgProbe::incomplete; - } else { - break; - } - - const std::size_t end = head.find(terminator); - if (end == std::string_view::npos) { - return SvgProbe::incomplete; - } - head.remove_prefix(end + terminator.size()); - } - - if (!head.starts_with("<")) { - return SvgProbe::no; - } - head.remove_prefix(1); - - const std::size_t end = head.find_first_of(name_end); - if (end == std::string_view::npos) { - return SvgProbe::incomplete; - } - std::string_view name = head.substr(0, end); - if (const std::size_t colon = name.find(':'); - colon != std::string_view::npos) { - name.remove_prefix(colon + 1); - } - return name == "svg" ? SvgProbe::yes : SvgProbe::no; -} - } // namespace FileType magic::file_type(const std::string &magic) { @@ -267,39 +199,15 @@ FileType magic::file_type(const std::string &magic) { return FileType::jpeg_xl; } - if (svg_probe(magic) == SvgProbe::yes) { - return FileType::scalable_vector_graphics; - } - return FileType::unknown; } FileType magic::file_type(std::istream &in) { - // enough for every signature - the longest is the enhanced metafile's, which - // ends at offset 44 - and for the prologue of a normal svg - static constexpr std::size_t head_size = 1024; - // a prologue has no length limit, so reading up to the root element needs a - // bound of its own; a licence comment or a doctype with an internal subset - // fits many times over - static constexpr std::size_t max_svg_head_size = 64 * head_size; - - std::string head = read_head(in, head_size); - if (const FileType file_type = magic::file_type(head); - file_type != FileType::unknown) { - return file_type; - } - - while (svg_probe(head) == SvgProbe::incomplete && - head.size() < max_svg_head_size) { - const std::string next = read_head(in, head_size); - if (next.empty()) { - break; - } - head += next; - } + // every signature is a prefix but one: an enhanced metafile names itself at + // offset 40, so the head has to reach 44 + static constexpr std::size_t head_size = 64; - return svg_probe(head) == SvgProbe::yes ? FileType::scalable_vector_graphics - : FileType::unknown; + return file_type(read_head(in, head_size)); } FileType magic::file_type(const abstract::File &file) { diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index 4feb8f4d..5b4e4c06 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -18,7 +18,9 @@ #include #include #include +#include #include +#include #include #include @@ -282,6 +284,24 @@ open_strategy::list_file_types(const std::shared_ptr &file, } catch (...) { ODR_VERBOSE(logger, "failed to open as json"); } + + // an svg has no signature to find it by - it is xml, and only the root + // element tells the two apart, so both are reported + try { + ODR_VERBOSE(logger, "try open as xml"); + util::xml::check_xml_file(*file->stream()); + result.push_back(FileType::xml); + + try { + ODR_VERBOSE(logger, "try open as svg"); + svg::check_svg_file(*file->stream()); + result.push_back(FileType::scalable_vector_graphics); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as svg"); + } + } catch (...) { + ODR_VERBOSE(logger, "failed to open as xml"); + } } catch (...) { ODR_VERBOSE(logger, "failed to open as text"); } @@ -393,6 +413,17 @@ open_strategy::open_file(const std::shared_ptr &file, ODR_VERBOSE(logger, "failed to open as json"); } + // see `list_file_types` - an svg is only recognised by parsing it, and + // a plain xml file has no decoder of its own, so it stays text + try { + ODR_VERBOSE(logger, "try open as svg"); + svg::check_svg_file(*file->stream()); + return std::make_unique(file, + FileType::scalable_vector_graphics); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as svg"); + } + ODR_VERBOSE(logger, "open as text file"); // TODO looks dirty return std::make_unique(file); diff --git a/src/odr/internal/svg/svg_util.cpp b/src/odr/internal/svg/svg_util.cpp new file mode 100644 index 00000000..8823ec0a --- /dev/null +++ b/src/odr/internal/svg/svg_util.cpp @@ -0,0 +1,28 @@ +#include + +#include + +#include + +#include +#include + +namespace odr::internal { + +void svg::check_svg_file(std::istream &in) { + const pugi::xml_document document = util::xml::parse(in); + + // pugixml does not process namespaces, so the root element carries whatever + // prefix the document bound to the svg namespace + std::string_view name = document.document_element().name(); + if (const std::size_t colon = name.find(':'); + colon != std::string_view::npos) { + name.remove_prefix(colon + 1); + } + + if (name != "svg") { + throw std::runtime_error("no svg file"); + } +} + +} // namespace odr::internal diff --git a/src/odr/internal/svg/svg_util.hpp b/src/odr/internal/svg/svg_util.hpp new file mode 100644 index 00000000..61cac5e9 --- /dev/null +++ b/src/odr/internal/svg/svg_util.hpp @@ -0,0 +1,10 @@ +#pragma once + +#include + +namespace odr::internal::svg { + +/// Throws unless @p in holds an xml document whose root element is `svg`. +void check_svg_file(std::istream &in); + +} // namespace odr::internal::svg diff --git a/src/odr/internal/util/xml_util.cpp b/src/odr/internal/util/xml_util.cpp index 69d3d9e8..59e224bb 100644 --- a/src/odr/internal/util/xml_util.cpp +++ b/src/odr/internal/util/xml_util.cpp @@ -8,6 +8,8 @@ #include +#include + namespace odr::internal::util { pugi::xml_document xml::parse(const std::string &in) { @@ -26,6 +28,8 @@ pugi::xml_document xml::parse(std::istream &in) { return result; } +void xml::check_xml_file(std::istream &in) { std::ignore = parse(in); } + pugi::xml_document xml::parse(const abstract::ReadableFilesystem &filesystem, const AbsPath &path) { pugi::xml_document result; diff --git a/src/odr/internal/util/xml_util.hpp b/src/odr/internal/util/xml_util.hpp index a4dc51ca..490a7727 100644 --- a/src/odr/internal/util/xml_util.hpp +++ b/src/odr/internal/util/xml_util.hpp @@ -22,6 +22,9 @@ pugi::xml_document parse(const std::string &); pugi::xml_document parse(std::istream &); pugi::xml_document parse(const abstract::ReadableFilesystem &, const AbsPath &); +/// Throws unless @p in holds a well formed xml document. +void check_xml_file(std::istream &in); + struct StringToken { enum class Type { none, diff --git a/test/src/internal/html/image_file_test.cpp b/test/src/internal/html/image_file_test.cpp index 81c25159..37c87217 100644 --- a/test/src/internal/html/image_file_test.cpp +++ b/test/src/internal/html/image_file_test.cpp @@ -1,15 +1,18 @@ #include #include +#include #include #include #include +#include #include #include #include #include +#include using namespace odr; @@ -52,6 +55,49 @@ std::string image_src(const File &file) { } // namespace +namespace { + +std::vector detect(const std::string &content) { + return internal::open_strategy::list_file_types( + std::make_shared(content), Logger::null()); +} + +} // namespace + +/// An svg has no signature - it is xml, and only the root element separates it +/// from any other xml. So `magic` does not guess it; the open strategy parses +/// the file and reports every layer it verified, least specific first. +TEST(image_file, svg_is_detected_by_parsing_it) { + EXPECT_EQ(detect(R"()"), + (std::vector{FileType::text_file, FileType::xml, + FileType::scalable_vector_graphics})); + + // a namespace prefix binds the root element just the same + EXPECT_EQ(detect(R"()"), + (std::vector{FileType::text_file, FileType::xml, + FileType::scalable_vector_graphics})); + + // a prologue of any length is the parser's problem, not a scanner's + EXPECT_EQ(detect("\n"), + (std::vector{FileType::text_file, FileType::xml, + FileType::scalable_vector_graphics})); +} + +/// The root element is what separates the two, so xml that is not an svg stops +/// at xml - including the two that a head scanner would most easily confuse. +TEST(image_file, xml_that_is_not_an_svg_stops_at_xml) { + for (const std::string &content : + {R"()", + R"()"}) { + EXPECT_EQ(detect(content), + (std::vector{FileType::text_file, FileType::xml})) + << content; + } + + // not xml at all + EXPECT_EQ(detect("just some text"), (std::vector{FileType::text_file})); +} + TEST(image_file, svg_is_detected_and_opens_as_an_image) { const DecodedFile file{svg_file()}; diff --git a/test/src/internal/magic_test.cpp b/test/src/internal/magic_test.cpp index 10c7d4de..79aeb58e 100644 --- a/test/src/internal/magic_test.cpp +++ b/test/src/internal/magic_test.cpp @@ -158,57 +158,6 @@ TEST(magic, image_signatures) { EXPECT_EQ(detect("\xff\xfb\x90\x00"s), FileType::mpeg_audio); } -TEST(magic, svg) { - using namespace std::string_literals; - - EXPECT_EQ(detect(""s), - FileType::scalable_vector_graphics); - EXPECT_EQ(detect(""s), FileType::scalable_vector_graphics); - EXPECT_EQ(detect("\n"s), - FileType::scalable_vector_graphics); - EXPECT_EQ(detect("\xef\xbb\xbf\n" - "\n" - "\n" - ""s), - FileType::scalable_vector_graphics); -} - -/// A prologue has no length limit - a generated file can carry a licence -/// comment far longer than the signature head - so detection reads on until it -/// reaches the root element. -TEST(magic, svg_behind_a_long_prologue) { - using namespace std::string_literals; - - const std::string comment = "\n"; - EXPECT_EQ(detect(R"()" - "\n" + - comment + ""), - FileType::scalable_vector_graphics); - - // the same length of prologue in front of something that is not an svg - EXPECT_EQ(detect(comment + ""), FileType::unknown); - - // reading on is bounded: a comment that never ends is not an svg - EXPECT_EQ(detect("