From 20405f88d6ba8d3f988fd343edc63bc04d4c7d77 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 22:49:11 +0200 Subject: [PATCH 1/4] feat(svg): read an svg as the xml it is, and draw it in the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `FileType::scalable_vector_graphics` was a label the generic image wrapper would put on any bytes at all, and an svg rendered as a base64 data url in an `` — fixed size, no selectable text, nothing to inspect. It now opens as `svg::SvgFile`, an image over an `xml::XmlFile`: the xml module parses it, resolves its encoding from the declaration and records the root element name, and svg answers the one question left. So `open` as an svg rejects what is not one, and detection costs one parse rather than two — `XmlFile::root_name()` is what the second probe reads. The markup goes into the page, so the drawing fits the screen and its text can be selected. That makes it live markup, which behind an `` it was not, so it arrives scrubbed — script and embedding elements, event handlers, references that leave the document, animation aimed at any of those, and css that reaches outside — under a page that declares `script-src 'none'`. An svg *inside* a document keeps the data url: there it is one image in a layout, and `` renders svg in secure static mode. Reasoning in `src/odr/internal/svg/AGENTS.md`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V1PApAFAK7q2yN7Rd2UUpr --- AGENTS.md | 1 + CHANGELOG.md | 2 + CMakeLists.txt | 2 + src/odr/exceptions.cpp | 2 + src/odr/exceptions.hpp | 5 + src/odr/file.cpp | 4 + src/odr/file.hpp | 5 +- src/odr/html.cpp | 8 + src/odr/internal/file_type_table.cpp | 6 +- src/odr/internal/html/frontend.cpp | 18 +++ src/odr/internal/html/frontend.hpp | 2 + src/odr/internal/html/html_writer.cpp | 11 ++ src/odr/internal/html/html_writer.hpp | 3 + src/odr/internal/html/svg_file.cpp | 136 ++++++++++++++++ src/odr/internal/html/svg_file.hpp | 22 +++ src/odr/internal/open_strategy.cpp | 48 ++++-- src/odr/internal/svg/AGENTS.md | 62 +++++++ src/odr/internal/svg/svg_file.cpp | 43 +++++ src/odr/internal/svg/svg_file.hpp | 36 +++++ src/odr/internal/svg/svg_util.cpp | 178 +++++++++++++++++++-- src/odr/internal/svg/svg_util.hpp | 20 ++- src/odr/internal/xml/xml_file.cpp | 13 +- src/odr/internal/xml/xml_file.hpp | 15 +- test/CMakeLists.txt | 1 + test/src/internal/html/image_file_test.cpp | 6 +- test/src/internal/svg/svg_file_test.cpp | 168 +++++++++++++++++++ 26 files changed, 776 insertions(+), 41 deletions(-) create mode 100644 src/odr/internal/html/svg_file.cpp create mode 100644 src/odr/internal/html/svg_file.hpp create mode 100644 src/odr/internal/svg/AGENTS.md create mode 100644 src/odr/internal/svg/svg_file.cpp create mode 100644 src/odr/internal/svg/svg_file.hpp create mode 100644 test/src/internal/svg/svg_file_test.cpp diff --git a/AGENTS.md b/AGENTS.md index a8270b69f..b4960980a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,6 +63,7 @@ bytes ─▶ magic/open_strategy ─▶ DecodedFile ─▶ Document ─▶ Eleme | `src/odr/internal/oldms/` | **Legacy MS binary** (.doc/.ppt/.xls). | | `src/odr/internal/pdf/` | PDF (own parser). | | `src/odr/internal/xml/` | XML, rendered as a source view; see [`xml/AGENTS.md`](src/odr/internal/xml/AGENTS.md). | +| `src/odr/internal/svg/` | SVG, read as xml and drawn in the page; see [`svg/AGENTS.md`](src/odr/internal/svg/AGENTS.md). | | `src/odr/internal/{csv,json,text,svm}/` | Smaller formats. | | `cli/src/` | CLI tools: `translate`, `back_translate`, `meta`, `server`. | | `python/` | Python bindings (`pyodr`, pybind11); see [`python/AGENTS.md`](python/AGENTS.md). | diff --git a/CHANGELOG.md b/CHANGELOG.md index c0f152038..52d9411cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ The release run heads these entries with the version and opens a fresh - An xml file opens as xml and reads as a foldable, highlighted source view rather than as one very long line, in the encoding its declaration names. +- An svg is recognised by reading it rather than by what it is called, and + renders as a drawing that fits the screen instead of a fixed-size image. ## v6.4.0 - 2026-08-09 diff --git a/CMakeLists.txt b/CMakeLists.txt index 6d1b30001..230d35abb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -147,6 +147,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/html/image_file.cpp" "src/odr/internal/html/media_file.cpp" "src/odr/internal/html/pdf_file.cpp" + "src/odr/internal/html/svg_file.cpp" "src/odr/internal/html/text_file.cpp" "src/odr/internal/html/xml_file.cpp" @@ -235,6 +236,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/font/sfnt_transform.cpp" "src/odr/internal/font/font_file.cpp" + "src/odr/internal/svg/svg_file.cpp" "src/odr/internal/svg/svg_util.cpp" "src/odr/internal/svm/svm_file.cpp" diff --git a/src/odr/exceptions.cpp b/src/odr/exceptions.cpp index 065ca8e05..aaa8d361c 100644 --- a/src/odr/exceptions.cpp +++ b/src/odr/exceptions.cpp @@ -70,6 +70,8 @@ NoLegacyMicrosoftFile::NoLegacyMicrosoftFile() NoXmlFile::NoXmlFile() : Exception("not an xml file") {} +NoSvgFile::NoSvgFile() : Exception("not an svg file") {} + UnsupportedCryptoAlgorithm::UnsupportedCryptoAlgorithm() : Exception("unsupported crypto algorithm") {} diff --git a/src/odr/exceptions.hpp b/src/odr/exceptions.hpp index dac37e653..c9d55fc17 100644 --- a/src/odr/exceptions.hpp +++ b/src/odr/exceptions.hpp @@ -147,6 +147,11 @@ struct NoXmlFile final : Exception { NoXmlFile(); }; +/// @brief No SVG file exception +struct NoSvgFile final : Exception { + NoSvgFile(); +}; + /// @brief Unsupported crypto algorithm exception struct UnsupportedCryptoAlgorithm final : Exception { UnsupportedCryptoAlgorithm(); diff --git a/src/odr/file.cpp b/src/odr/file.cpp index e64fc15e1..aa95091ca 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -338,6 +338,10 @@ std::unique_ptr ImageFile::stream() const { return m_impl->file()->stream(); } +std::shared_ptr ImageFile::impl() const { + return m_impl; +} + ArchiveFile::ArchiveFile(std::shared_ptr impl) : DecodedFile(impl), m_impl{std::move(impl)} {} diff --git a/src/odr/file.hpp b/src/odr/file.hpp index 8343a95e7..c730c12f2 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -128,7 +128,8 @@ enum class FileType { 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. + // the same reason as the block above - nothing here is decoded either, + // except svg, which is xml. // https://en.wikipedia.org/wiki/SVG scalable_vector_graphics, // https://en.wikipedia.org/wiki/ICO_(file_format) @@ -461,6 +462,8 @@ class ImageFile final : public DecodedFile { [[nodiscard]] std::unique_ptr stream() const; + [[nodiscard]] std::shared_ptr impl() const; + private: std::shared_ptr m_impl; }; diff --git a/src/odr/html.cpp b/src/odr/html.cpp index 8fea45503..2cd476623 100644 --- a/src/odr/html.cpp +++ b/src/odr/html.cpp @@ -15,8 +15,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -286,6 +288,12 @@ HtmlService html::translate(const TextFile &text_file, const HtmlConfig &config, HtmlService html::translate(const ImageFile &image_file, const HtmlConfig &config, const Logger &logger) { + // an svg is markup, and goes into the page as markup + if (const auto svg_file = + std::dynamic_pointer_cast(image_file.impl()); + svg_file != nullptr) { + return internal::html::create_svg_service(svg_file, config, logger); + } return internal::html::create_image_service(image_file, config, logger); } diff --git a/src/odr/internal/file_type_table.cpp b/src/odr/internal/file_type_table.cpp index c5063b54c..093ecd948 100644 --- a/src/odr/internal/file_type_table.cpp +++ b/src/odr/internal/file_type_table.cpp @@ -606,9 +606,9 @@ constexpr std::array table{ DocumentType::unknown, {.detect_by_content = true, .open = true, .translate_html = true}}, - // Named but not decoded, like the images above. `translate_html` means the - // image page is written and the data url labelled, not that every browser - // paints it. + // Named but not decoded, like the images above — except svg, which is xml + // and is parsed. For the rest `translate_html` means the image page is + // written and the data url labelled, not that every browser paints it. Row{FileType::scalable_vector_graphics, "svg"sv, svg_extensions, diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index a6ab74007..f18291af6 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -123,6 +123,13 @@ body{background:#fff} .odr-xml-decl,.odr-xml-doctype,.odr-xml-pi{color:var(--odr-xml-meta)} )css"; +/// The svg keeps whatever intrinsic size it declares; these rules only cap it. +constexpr std::string_view svg_css = R"css( +body{margin:0;background:#fff} +.odr-svg{display:flex;align-items:center;justify-content:center;min-height:100vh} +.odr-svg>svg{max-width:100%;max-height:100vh} +)css"; + constexpr std::string_view filesystem_css = R"css( :root{ --odr-files-line:#e3e5e8; @@ -929,6 +936,8 @@ constexpr Asset text_css_asset{HtmlResourceType::css, "text/css", "text.css", text_css}; constexpr Asset xml_css_asset{HtmlResourceType::css, "text/css", "xml.css", xml_css}; +constexpr Asset svg_css_asset{HtmlResourceType::css, "text/css", "svg.css", + svg_css}; constexpr Asset filesystem_css_asset{HtmlResourceType::css, "text/css", "filesystem.css", filesystem_css}; constexpr Asset media_css_asset{HtmlResourceType::css, "text/css", "media.css", @@ -1009,6 +1018,10 @@ void html::write_xml_style(const WritingState &state) { write_style(xml_css_asset, state); } +void html::write_svg_style(const WritingState &state) { + write_style(svg_css_asset, state); +} + void html::write_filesystem_style(const WritingState &state) { write_style(filesystem_css_asset, state); } @@ -1039,6 +1052,11 @@ HtmlResources html::locate_xml_resources(const HtmlConfig &config) { return locate_all(assets, config); } +HtmlResources html::locate_svg_resources(const HtmlConfig &config) { + static constexpr std::array assets{svg_css_asset}; + return locate_all(assets, config); +} + HtmlResources html::locate_media_resources(const HtmlConfig &config) { static constexpr std::array assets{media_css_asset}; return locate_all(assets, config); diff --git a/src/odr/internal/html/frontend.hpp b/src/odr/internal/html/frontend.hpp index 4b433bb07..57bcf1f3e 100644 --- a/src/odr/internal/html/frontend.hpp +++ b/src/odr/internal/html/frontend.hpp @@ -14,6 +14,7 @@ void write_document_style(const WritingState &state); void write_spreadsheet_style(const WritingState &state); void write_text_style(const WritingState &state); void write_xml_style(const WritingState &state); +void write_svg_style(const WritingState &state); void write_filesystem_style(const WritingState &state); void write_media_style(const WritingState &state); @@ -29,6 +30,7 @@ void write_text_script(const WritingState &state); /// entry is located `nullopt` when the config embeds them. HtmlResources locate_text_resources(const HtmlConfig &config); HtmlResources locate_xml_resources(const HtmlConfig &config); +HtmlResources locate_svg_resources(const HtmlConfig &config); HtmlResources locate_media_resources(const HtmlConfig &config); } // namespace odr::internal::html diff --git a/src/odr/internal/html/html_writer.cpp b/src/odr/internal/html/html_writer.cpp index bcb68fb24..a872a44a6 100644 --- a/src/odr/internal/html/html_writer.cpp +++ b/src/odr/internal/html/html_writer.cpp @@ -172,6 +172,17 @@ void HtmlWriter::write_header_meta(const std::string &name, out() << R"("/>)"; } +void HtmlWriter::write_header_meta_equiv(const std::string &equiv, + const std::string &content) { + write_new_line(); + + out() << R"()"; +} + void HtmlWriter::write_header_viewport(const std::string &viewport) { write_header_meta("viewport", viewport); } diff --git a/src/odr/internal/html/html_writer.hpp b/src/odr/internal/html/html_writer.hpp index b26713d02..0636a1990 100644 --- a/src/odr/internal/html/html_writer.hpp +++ b/src/odr/internal/html/html_writer.hpp @@ -58,6 +58,9 @@ class HtmlWriter { void write_header_end(); void write_header_title(const std::string &title); void write_header_meta(const std::string &name, const std::string &content); + /// An `http-equiv` meta, e.g. a content security policy. + void write_header_meta_equiv(const std::string &equiv, + const std::string &content); void write_header_viewport(const std::string &viewport); void write_header_target(const std::string &target); void write_header_charset(const std::string &charset); diff --git a/src/odr/internal/html/svg_file.cpp b/src/odr/internal/html/svg_file.cpp new file mode 100644 index 000000000..8f8257b2b --- /dev/null +++ b/src/odr/internal/html/svg_file.cpp @@ -0,0 +1,136 @@ +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include +#include + +namespace odr::internal::html { +namespace { + +class HtmlServiceImpl final : public HtmlService { +public: + HtmlServiceImpl(std::shared_ptr svg_file, HtmlConfig config, + const Logger &logger) + : HtmlService(std::move(config), logger), m_svg_file{std::move(svg_file)}, + m_resources{locate_svg_resources(this->config())} { + m_views.emplace_back( + std::make_shared(*this, "image", 0, "image.html")); + } + + void warmup() const override {} + + [[nodiscard]] const HtmlViews &list_views() const override { return m_views; } + + [[nodiscard]] bool exists(const std::string &path) const override { + return path == "image.html" || resource_at(m_resources, path) != nullptr; + } + + [[nodiscard]] std::string mimetype(const std::string &path) const override { + if (path == "image.html") { + return "text/html"; + } + if (const odr::HtmlResource *resource = resource_at(m_resources, path); + resource != nullptr) { + return resource->mime_type(); + } + + throw FileNotFound("Unknown path: " + path); + } + + void write(const std::string &path, std::ostream &out) const override { + if (path == "image.html") { + HtmlWriter writer(out, config()); + write_svg(writer); + return; + } + if (const odr::HtmlResource *resource = resource_at(m_resources, path); + resource != nullptr) { + resource->write_resource(out); + return; + } + + throw FileNotFound("Unknown path: " + path); + } + + HtmlResources write_html(const std::string &path, + HtmlWriter &out) const override { + if (path == "image.html") { + return write_svg(out); + } + + throw FileNotFound("Unknown path: " + path); + } + + HtmlResources write_svg(HtmlWriter &out) const { + HtmlResources resources; + const WritingState state(out, config(), resources); + + const std::unique_ptr document = + xml::parse_source(m_svg_file->text()); + svg::sanitize(document->document_element()); + + out.write_begin(); + + out.write_header_begin(); + + out.write_header_charset("UTF-8"); + // second line of defence after the scrub above, and free: no view of an + // svg has a script of its own + out.write_header_meta_equiv("Content-Security-Policy", "script-src 'none'"); + out.write_header_target("_blank"); + out.write_header_title("odr"); + write_viewport_meta(out, config(), true); + + write_svg_style(state); + + out.write_header_end(); + + out.write_body_begin(); + + out.write_element_begin("div", HtmlElementOptions().set_class("odr-svg")); + document->document_element().print(out.out(), "", pugi::format_raw); + out.write_element_end("div"); + + out.write_body_end(); + + out.write_end(); + + return resources; + } + +protected: + std::shared_ptr m_svg_file; + /// The css this view links; empty of locations when the config embeds it. + HtmlResources m_resources; + + HtmlViews m_views; +}; + +} // namespace +} // namespace odr::internal::html + +namespace odr::internal { + +HtmlService +html::create_svg_service(const std::shared_ptr &svg_file, + HtmlConfig config, const Logger &logger) { + return odr::HtmlService( + std::make_unique(svg_file, std::move(config), logger)); +} + +} // namespace odr::internal diff --git a/src/odr/internal/html/svg_file.hpp b/src/odr/internal/html/svg_file.hpp new file mode 100644 index 000000000..e2e5dbefe --- /dev/null +++ b/src/odr/internal/html/svg_file.hpp @@ -0,0 +1,22 @@ +#pragma once + +#include + +namespace odr { +struct HtmlConfig; +class HtmlService; +class Logger; +} // namespace odr + +namespace odr::internal::svg { +class SvgFile; +} // namespace odr::internal::svg + +namespace odr::internal::html { + +/// Renders @p svg_file by writing its markup into the page rather than as a +/// data url. +HtmlService create_svg_service(const std::shared_ptr &svg_file, + HtmlConfig config, const Logger &logger); + +} // namespace odr::internal::html diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index ccf821428..070719603 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -18,12 +18,14 @@ #include #include #include +#include #include #include #include #include #include +#include namespace odr::internal { @@ -121,6 +123,18 @@ open_file_as(const std::shared_ptr &file, const FileType as, throw NoSvmFile(); } + if (as == FileType::scalable_vector_graphics) { + ODR_VERBOSE(logger, "open as svg"); + try { + auto text = std::make_shared(file); + return std::make_unique( + std::make_shared(text)); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as svg"); + } + throw NoSvgFile(); + } + // no decoder below: the bytes go to the browser as they are, so only the // category has to be right const FileCategory category = file_category_by_file_type(as); @@ -305,12 +319,12 @@ open_strategy::list_file_types(const std::shared_ptr &file, // xml, so both are reported try { ODR_VERBOSE(logger, "try open as xml"); - result.push_back(xml::XmlFile(text).file_type()); + auto xml_file = std::make_shared(text); + result.push_back(xml_file->file_type()); try { ODR_VERBOSE(logger, "try open as svg"); - svg::check_svg_file(*file->stream()); - result.push_back(FileType::scalable_vector_graphics); + result.push_back(svg::SvgFile(xml_file).file_type()); } catch (...) { ODR_VERBOSE(logger, "failed to open as svg"); } @@ -427,20 +441,24 @@ open_strategy::open_file(const std::shared_ptr &file, ODR_VERBOSE(logger, "failed to open as json"); } - // svg first - it is the more specific reading of the same bytes - and - // xml last, before the line list - 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"); - } - + // svg is read off the parse xml already did: it is the more specific + // reading of the same bytes, and xml is the last resort before the line + // list try { ODR_VERBOSE(logger, "try open as xml"); - return std::make_unique(text); + auto xml_file = std::make_unique(text); + + try { + ODR_VERBOSE(logger, "try open as svg"); + svg::check_svg_file(*xml_file); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as svg"); + // handed on as it is, so the parse is not repeated + return xml_file; + } + + return std::make_unique( + std::shared_ptr(std::move(xml_file))); } catch (...) { ODR_VERBOSE(logger, "failed to open as xml"); } diff --git a/src/odr/internal/svg/AGENTS.md b/src/odr/internal/svg/AGENTS.md new file mode 100644 index 000000000..a0b284207 --- /dev/null +++ b/src/odr/internal/svg/AGENTS.md @@ -0,0 +1,62 @@ +# AGENTS.md — `internal/svg` + +Read the root [`AGENTS.md`](../../../../AGENTS.md) first. This file covers what +svg does differently, and why. + +## An svg is xml + +`SvgFile` is an `abstract::ImageFile` over a `std::shared_ptr`. +The [xml module](../xml/AGENTS.md) parses, rejects what is not well formed, and +resolves the encoding from the declaration. What is left is one question — is +the root element `svg`? — answered against `XmlFile::root_name()`, which the +xml parse already recorded, so detection costs one parse and not two. + +pugixml does not process namespaces, so the root name arrives with whatever +prefix the document bound (``) and the prefix comes off by hand. Same +for attributes, matched on their local name. + +`FileType::scalable_vector_graphics` is therefore no longer a label the generic +`common::ImageFile` will put on any bytes: `open` as an svg throws `NoSvgFile` +unless it is one. + +## It renders as markup + +Every other image goes into the page as ``. An svg is written +into the page as the markup it is, so it scales to the viewport and its text is +selectable. + +An svg **inside** a document keeps the data url — `translate_image_src` — where +it is one image in a layout and `` renders svg in secure static mode. A +starview metafile converted to svg (`html/image_file.cpp`) takes that path too. + +## Which is why there is a sanitiser + +Inside an `` an svg is inert. In the page it is live markup, and the file +came from wherever the user got it. Two lines of defence: + +1. **`svg::sanitize` scrubs the tree** — script, `foreignObject` and the other + embedding elements, every `on*` attribute, every `href`/`src` that is not a + same-document fragment or a `data:image/` url, SMIL animation aimed at any + of those, and css that reaches outside the document (`@import`, or a `url()` + that is neither a fragment nor a `data:image/`). +2. **The page declares `script-src 'none'`** — free, since no view of an svg + has a script of its own, and it catches whatever the scrub missed. + +Two rules need their reasons on record: + +- **Everything is matched case-insensitively.** In html *foreign content* the + parser lowercases names, so `"); + + EXPECT_THAT(html, Not(HasSubstr("evil"))); + EXPECT_THAT(html, HasSubstr("")); +} + +/// The html parser lowercases element names in foreign content, so `"), Not(HasSubstr("evil"))); + EXPECT_THAT(svg_html("x"), + Not(HasSubstr("foreignObject"))); +} + +TEST(SvgHtml, event_handlers_are_stripped) { + EXPECT_THAT(svg_html(R"()"), + Not(HasSubstr("evil"))); + EXPECT_THAT(svg_html(R"()"), Not(HasSubstr("evil"))); + // and the element itself survives + EXPECT_THAT(svg_html(R"()"), + HasSubstr(R"()")); +} + +/// A reference is kept only where it goes nowhere — same document, or an image +/// carried in the url itself. +TEST(SvgHtml, references_that_leave_the_document_are_stripped) { + EXPECT_THAT(svg_html(R"()"), + HasSubstr(R"()")); + EXPECT_THAT(svg_html(R"()"), + HasSubstr("data:image/png;base64,AA==")); + + EXPECT_THAT(svg_html(R"()"), + Not(HasSubstr("javascript"))); + EXPECT_THAT(svg_html(""), + Not(HasSubstr("evil"))); + EXPECT_THAT(svg_html(R"()"), + Not(HasSubstr("example.com"))); +} + +/// SMIL animates whatever attribute it is pointed at, including the ones the +/// attribute rules just removed. +TEST(SvgHtml, animating_a_stripped_attribute_is_stripped_too) { + EXPECT_THAT(svg_html(R"()"), + Not(HasSubstr("evil"))); + EXPECT_THAT(svg_html(R"()"), + Not(HasSubstr("javascript"))); + // an ordinary animation is left alone + EXPECT_THAT(svg_html(R"()"), + HasSubstr(R"(attributeName="x")")); +} + +TEST(SvgHtml, css_that_reaches_outside_the_document_is_stripped) { + EXPECT_THAT( + svg_html(""), + Not(HasSubstr("example.com"))); + EXPECT_THAT( + svg_html(R"()"), + Not(HasSubstr("example.com"))); + EXPECT_THAT(svg_html(""), + Not(HasSubstr("example.com"))); + + // a fragment url is how a gradient is referenced, and stays + EXPECT_THAT(svg_html(""), + HasSubstr("url(#grad)")); +} + +/// An svg inside a document is still written as a data url — an `` is +/// what the renderer has there, and it renders svg in secure static mode. +TEST(SvgHtml, an_svg_referenced_by_a_document_stays_a_data_url) { + std::ostringstream out; + internal::html::translate_image_src( + File::from_memory(std::string(svg_open) + ""), out, HtmlConfig()); + + EXPECT_THAT(out.str(), HasSubstr("data:image/svg+xml;base64,")); +} From 7db8d401381e16dab32fc69ca73a3cff3aec22fa Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 10 Aug 2026 07:37:38 +0200 Subject: [PATCH 2/4] fix(svg): match the animated attribute like every other name, and unprefix what is written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways the inline rendering read a document differently from the browser that gets it. `attributeName` was looked up through pugixml's exact-match `attribute()`, so a `` was left in place — and the html parser lowercases attribute names in foreign content and maps that one straight back, so the animation still aimed at the href the scrub had just taken off. The lookup now goes by local name, case-insensitively, like every other name here. A document that binds the svg namespace to a prefix was written into the page as ``, and the html parser enters foreign content on `svg` alone: a file that drew fine as an `` data url drew nothing at all. The prefixes bound to the svg namespace come off before the markup is written — only those, because a bare `span` or `div` in foreign content is what ends the svg early. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014ER7HcXMJZ1Q8azxx4wJRS --- src/odr/internal/html/svg_file.cpp | 1 + src/odr/internal/svg/AGENTS.md | 15 ++++-- src/odr/internal/svg/svg_util.cpp | 72 ++++++++++++++++++++++++- src/odr/internal/svg/svg_util.hpp | 6 +++ test/src/internal/svg/svg_file_test.cpp | 30 +++++++++++ 5 files changed, 120 insertions(+), 4 deletions(-) diff --git a/src/odr/internal/html/svg_file.cpp b/src/odr/internal/html/svg_file.cpp index 8f8257b2b..cc3c6dc13 100644 --- a/src/odr/internal/html/svg_file.cpp +++ b/src/odr/internal/html/svg_file.cpp @@ -82,6 +82,7 @@ class HtmlServiceImpl final : public HtmlService { const std::unique_ptr document = xml::parse_source(m_svg_file->text()); + svg::drop_namespace_prefixes(document->document_element()); svg::sanitize(document->document_element()); out.write_begin(); diff --git a/src/odr/internal/svg/AGENTS.md b/src/odr/internal/svg/AGENTS.md index a0b284207..f522c2c12 100644 --- a/src/odr/internal/svg/AGENTS.md +++ b/src/odr/internal/svg/AGENTS.md @@ -25,6 +25,12 @@ Every other image goes into the page as ``. An svg is writte into the page as the markup it is, so it scales to the viewport and its text is selectable. +Which is why the prefix a document bound to the svg namespace comes off the +element names first: the html parser enters foreign content on `svg`, not on +`s:svg`, and a file that used to draw as an `` has to keep drawing. Only +that prefix — an element of some other namespace keeps its own, because a bare +`span` or `div` in foreign content is what ends the svg early. + An svg **inside** a document keeps the data url — `translate_image_src` — where it is one image in a layout and `` renders svg in secure static mode. A starview metafile converted to svg (`html/image_file.cpp`) takes that path too. @@ -44,9 +50,12 @@ came from wherever the user got it. Two lines of defence: Two rules need their reasons on record: -- **Everything is matched case-insensitively.** In html *foreign content* the - parser lowercases names, so `"); - - EXPECT_THAT(html, Not(HasSubstr("evil"))); - EXPECT_THAT(html, HasSubstr("")); -} - -/// The html parser lowercases element names in foreign content, so `"), Not(HasSubstr("evil"))); - EXPECT_THAT(svg_html("x"), - Not(HasSubstr("foreignObject"))); -} +/// Every layer the file was read through stays reachable, so what the parse +/// already did is not done again downstream. +TEST(SvgFile, the_layers_it_was_read_through_are_reachable) { + const std::string content = std::string(svg_open) + ""; + const std::shared_ptr file = svg_file(content); -TEST(SvgHtml, event_handlers_are_stripped) { - EXPECT_THAT(svg_html(R"()"), - Not(HasSubstr("evil"))); - EXPECT_THAT(svg_html(R"()"), Not(HasSubstr("evil"))); - // and the element itself survives - EXPECT_THAT(svg_html(R"()"), - HasSubstr(R"()")); + EXPECT_EQ(file->text(), content); + EXPECT_EQ(file->text_file()->text(), content); + EXPECT_EQ(file->xml_file()->root_name(), "svg"); + EXPECT_STREQ(file->document().document_element().name(), "svg"); + EXPECT_EQ(file->file()->size(), content.size()); } -/// A reference is kept only where it goes nowhere — same document, or an image -/// carried in the url itself. -TEST(SvgHtml, references_that_leave_the_document_are_stripped) { - EXPECT_THAT(svg_html(R"()"), - HasSubstr(R"()")); - EXPECT_THAT(svg_html(R"()"), - HasSubstr("data:image/png;base64,AA==")); - - EXPECT_THAT(svg_html(R"()"), - Not(HasSubstr("javascript"))); - EXPECT_THAT(svg_html(""), - Not(HasSubstr("evil"))); - EXPECT_THAT(svg_html(R"()"), - Not(HasSubstr("example.com"))); -} +/// The encoding comes off the declaration, which is what the xml layer is for. +TEST(SvgFile, the_declared_encoding_is_what_it_is_decoded_with) { + const std::shared_ptr file = + svg_file(R"()" + + std::string(svg_open) + "\xe4"); -/// SMIL animates whatever attribute it is pointed at, including the ones the -/// attribute rules just removed. -TEST(SvgHtml, animating_a_stripped_attribute_is_stripped_too) { - EXPECT_THAT(svg_html(R"()"), - Not(HasSubstr("evil"))); - EXPECT_THAT(svg_html(R"()"), - Not(HasSubstr("javascript"))); - // an ordinary animation is left alone - EXPECT_THAT(svg_html(R"()"), - HasSubstr(R"(attributeName="x")")); + EXPECT_EQ(file->xml_file()->encoding(), TextEncoding::iso_8859_1); + EXPECT_THAT(file->text(), HasSubstr("ä")); } -/// The html parser lowercases the attribute name too and maps it back, so -/// `attributename` aims at what `attributeName` aims at. -TEST(SvgHtml, the_animated_attribute_is_named_case_insensitively) { - EXPECT_THAT(svg_html(R"()"), - Not(HasSubstr("evil"))); - EXPECT_THAT(svg_html(R"()"), - Not(HasSubstr("javascript"))); -} - -/// The html parser enters foreign content on `svg`, not on `s:svg` — a prefixed -/// document drew as an `` data url and has to keep drawing in the page. -TEST(SvgHtml, a_prefixed_document_is_written_unprefixed) { +/// An svg renders as the `` data url every other image does - inside one +/// a browser renders svg in secure static mode, and nothing here has to scrub +/// the markup to earn that. +TEST(SvgFile, it_renders_as_an_image) { const HtmlService service = odr::html::translate( - DecodedFile(svg_file(R"()" - R"(x)" - R"()")), + DecodedFile(svg_file(std::string(svg_open) + "")), HtmlConfig(), Logger::null()); - std::ostringstream out; - service.write("image.html", out); - - EXPECT_THAT(out.str(), HasSubstr(R"(
)")); - // a foreign prefix stays: a bare `span` in foreign content ends the svg - EXPECT_THAT(out.str(), HasSubstr("")); -} + ASSERT_EQ(service.list_views().size(), 1); + EXPECT_EQ(service.list_views().front().name(), "image"); -TEST(SvgHtml, css_that_reaches_outside_the_document_is_stripped) { - EXPECT_THAT( - svg_html(""), - Not(HasSubstr("example.com"))); - EXPECT_THAT( - svg_html(R"()"), - Not(HasSubstr("example.com"))); - EXPECT_THAT(svg_html(""), - Not(HasSubstr("example.com"))); - - // a fragment url is how a gradient is referenced, and stays - EXPECT_THAT(svg_html(""), - HasSubstr("url(#grad)")); -} - -/// An svg inside a document is still written as a data url — an `` is -/// what the renderer has there, and it renders svg in secure static mode. -TEST(SvgHtml, an_svg_referenced_by_a_document_stays_a_data_url) { std::ostringstream out; - internal::html::translate_image_src( - File::from_memory(std::string(svg_open) + ""), out, HtmlConfig()); + service.write("image.html", out); + EXPECT_THAT(out.str(), HasSubstr("