From 32c182197f3d27d26302ca9faa8c388a23386b97 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 02:36:51 +0200 Subject: [PATCH 1/2] fix: reserve archive viewer resources before entries --- CHANGELOG.md | 3 + src/odr/internal/html/filesystem.cpp | 40 ++++++++---- src/odr/internal/html/frontend.cpp | 15 +++++ src/odr/internal/html/frontend.hpp | 1 + src/odr/internal/html/html_writer.cpp | 2 +- test/src/html_test.cpp | 90 ++++++++++++++++++--------- 6 files changed, 106 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7a2b43de..6ee3ae852 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Archive listings reserve shipped script and stylesheet paths before exposing + entries, and reject unsafe entry links. Archives cannot replace viewer scripts. + - HTML and SVG rendering validates document length units before writing styles or coordinates, preventing malformed units from injecting output markup. diff --git a/src/odr/internal/html/filesystem.cpp b/src/odr/internal/html/filesystem.cpp index fcba75bc9..ec4cf9ec2 100644 --- a/src/odr/internal/html/filesystem.cpp +++ b/src/odr/internal/html/filesystem.cpp @@ -12,8 +12,11 @@ #include #include #include +#include #include +#include + #include #include @@ -162,10 +165,14 @@ class HtmlServiceImpl final : public HtmlService { entry_path->string(), file, false, false, true); HtmlResourceLocation location = config().resource_locator(resource, config()); - // Two resources at one location write over each other; the stylesheet is - // registered first, so an entry landing on it yields. + // Shipped resources reserve their locations before archive entries. if (location.has_value() && - resource_at(state.resources(), *location) != nullptr) { + (uri_kind(*location) == UriKind::refused || + util::string::equals_ignore_case(*location, listing_path) || + std::ranges::any_of(state.resources(), [&](const auto &entry) { + return entry.second && + util::string::equals_ignore_case(*entry.second, *location); + }))) { return std::nullopt; } @@ -174,7 +181,7 @@ class HtmlServiceImpl final : public HtmlService { } HtmlResources write_filesystem(HtmlWriter &out) const { - HtmlResources resources; + HtmlResources resources = locate_filesystem_resources(config()); const WritingState state(out, config(), resources, logger()); const FileWalker file_walker = m_filesystem.file_walker("/"); @@ -223,10 +230,13 @@ class HtmlServiceImpl final : public HtmlService { HtmlElementOptions().set_inline(true).set_class("odr-files-name")); if (location.has_value()) { out.write_element_begin( - "a", - HtmlElementOptions().set_inline(true).set_attributes( - HtmlAttributesVector{{"href", xml::escape_attribute(*location)}, - {"title", xml::escape_attribute(name)}})); + "a", HtmlElementOptions() + .set_inline(true) + .set_extra(std::string( + link_target_attributes(uri_kind(*location)))) + .set_attributes(HtmlAttributesVector{ + {"href", xml::escape_attribute(*location)}, + {"title", xml::escape_attribute(name)}})); out.write_raw(escape_text(file_path.string())); out.write_element_end("a"); } else { @@ -251,11 +261,15 @@ class HtmlServiceImpl final : public HtmlService { : entry_data_url(file, mime_type_of(file_path)); href.has_value()) { out.write_element_begin( - "a", - HtmlElementOptions().set_inline(true).set_attributes( - HtmlAttributesVector{{"href", *href}, - {"download", xml::escape_attribute(name)}, - {"title", xml::escape_attribute(name)}})); + "a", HtmlElementOptions() + .set_inline(true) + .set_extra(std::string( + location ? link_target_attributes(uri_kind(*location)) + : std::string_view())) + .set_attributes(HtmlAttributesVector{ + {"href", *href}, + {"download", xml::escape_attribute(name)}, + {"title", xml::escape_attribute(name)}})); out.write_raw("\u2193"); out.write_element_end("a"); } diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index cf740feea..05dda9a33 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -135,6 +135,11 @@ constexpr Asset host_bridge_js_asset{HtmlResourceType::js, "text/javascript", /// Appends @p asset to @p resources; `nullopt` to embed it. HtmlResourceLocation locate(const Asset &asset, const HtmlConfig &config, HtmlResources &resources) { + for (const auto &[resource, location] : resources) { + if (resource.is_shipped() && resource.name() == asset.name) { + return location; + } + } const odr::HtmlResource resource = HtmlResource::create( asset.type, std::string(asset.mime_type), std::string(asset.name), std::string(asset.name), odr::File::from_memory(written(asset)), true, @@ -362,6 +367,16 @@ void html::write_host_bridge_script(const WritingState &state) { write_script(host_bridge_js_asset, state); } +HtmlResources html::locate_filesystem_resources(const HtmlConfig &config) { + static constexpr std::array assets{filesystem_css_asset, search_css_asset, + search_js_asset, viewport_js_asset}; + static constexpr std::array dark{filesystem_dark_css_asset, + search_dark_css_asset}; + HtmlResources resources = locate_all(assets, dark, config); + locate_host_bridge(config, resources); + return resources; +} + HtmlResources html::locate_text_resources(const HtmlConfig &config) { static constexpr std::array assets{text_css_asset, search_css_asset, search_js_asset, editing_js_asset, diff --git a/src/odr/internal/html/frontend.hpp b/src/odr/internal/html/frontend.hpp index 8a35686fb..b30ebc1c0 100644 --- a/src/odr/internal/html/frontend.hpp +++ b/src/odr/internal/html/frontend.hpp @@ -68,6 +68,7 @@ void write_host_bridge_script(const WritingState &state); /// What the corresponding `write_*` calls would link, without writing anything: /// a service has to answer for these paths as well as for its views. Every /// entry is located `nullopt` when the config embeds them. +HtmlResources locate_filesystem_resources(const HtmlConfig &config); HtmlResources locate_text_resources(const HtmlConfig &config); HtmlResources locate_xml_resources(const HtmlConfig &config); HtmlResources locate_media_resources(const HtmlConfig &config); diff --git a/src/odr/internal/html/html_writer.cpp b/src/odr/internal/html/html_writer.cpp index 860802deb..d02378c0e 100644 --- a/src/odr/internal/html/html_writer.cpp +++ b/src/odr/internal/html/html_writer.cpp @@ -87,7 +87,7 @@ void write_element_options(std::ostream &out, if (options.attributes) { write_attributes(out, *options.attributes); } - if (options.extra) { + if (options.extra && !is_empty(*options.extra)) { out << " "; write_writable(out, *options.extra); } diff --git a/test/src/html_test.cpp b/test/src/html_test.cpp index 773c63e69..66fd9834c 100644 --- a/test/src/html_test.cpp +++ b/test/src/html_test.cpp @@ -121,42 +121,70 @@ TEST(html, linked_images_are_served) { check("odr-public/xlsx/sample.xlsx"); } -// The one archive the reference-output suite renders has no directory in it. -// An archive may hold a file named like the stylesheet. Forcing the collision -// through the locator saves needing such an archive in the test data. -TEST(html, archive_entry_yields_to_a_shipped_resource) { - const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); - +TEST(html, archive_entry_yields_to_each_shipped_resource) { const DecodedFile file = open(TestData::test_file_path("odr-public/odt/about.odt"), - DecodeOptions::as(FileType::zip), logger); - - HtmlConfig config((std::filesystem::current_path() / "collision").string()); - config.embed_shipped_resources = false; - config.resource_locator = [](const HtmlResource &resource, - const HtmlConfig &) -> HtmlResourceLocation { - return resource.is_shipped() ? "content.xml" : resource.path(); - }; - - std::ostringstream out; - const HtmlResources resources = - html::translate(file, config, logger).list_views().at(0).write_html(out); - - // The locator puts every shipped resource on that one location, so what the - // count says is that the entry of that name is not among them. - std::size_t shipped = 0; - std::size_t claimants = 0; - for (const auto &[resource, location] : resources) { - if (resource.is_shipped()) { - ++shipped; + DecodeOptions::as(FileType::zip)); + for (const std::string asset : {"filesystem.css", "search.js", "viewport.js", + "host-bridge.js", "SEARCH.JS"}) { + HtmlConfig config; + config.embed_shipped_resources = false; + config.host_message_handler = "host"; + config.resource_locator = + [asset](const HtmlResource &resource, + const HtmlConfig &) -> HtmlResourceLocation { + return resource.is_shipped() && + resource.name() == + (asset == "SEARCH.JS" ? "search.js" : asset) + ? (asset == "SEARCH.JS" ? "CONTENT.XML" : "content.xml") + : resource.path(); + }; + const HtmlService service = html::translate(file, config); + std::ostringstream out; + const HtmlResources resources = service.list_views().at(0).write_html(out); + std::size_t claimants = 0; + for (const auto &[resource, location] : resources) { + if (location == "content.xml" || location == "CONTENT.XML") { + ++claimants; + EXPECT_TRUE(resource.is_shipped()); + EXPECT_EQ(resource.name(), asset == "SEARCH.JS" ? "search.js" : asset); + } } - if (location.has_value() && *location == "content.xml") { - ++claimants; - EXPECT_TRUE(resource.is_shipped()); + EXPECT_EQ(claimants, 1u); + std::ostringstream served; + const std::string path = + asset == "SEARCH.JS" ? "CONTENT.XML" : "content.xml"; + service.write(path, served); + EXPECT_FALSE(served.str().starts_with(" HtmlResourceLocation { + return resource.is_shipped() ? std::nullopt : std::optional(location); + }; + std::ostringstream out; + html::translate(file, config).list_views().at(0).write_html(out); + const std::string page = out.str(); + if (location != "https://example.com/file") { + EXPECT_EQ(page.find(location), std::string::npos); + } else { + EXPECT_NE(page.find(R"(href="https://example.com/file")"), + std::string::npos); + EXPECT_NE(page.find(R"(target="_blank" rel="noopener noreferrer")"), + std::string::npos); } } - EXPECT_GT(shipped, 0); - EXPECT_EQ(claimants, shipped); } TEST(html, archive_listing) { From 47877f15692b4a142e93350d7d836e6ac67d8d58 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 21:37:51 +0200 Subject: [PATCH 2/2] style: group the standard includes of the archive listing Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/internal/html/filesystem.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/odr/internal/html/filesystem.cpp b/src/odr/internal/html/filesystem.cpp index ec4cf9ec2..ab9507a60 100644 --- a/src/odr/internal/html/filesystem.cpp +++ b/src/odr/internal/html/filesystem.cpp @@ -16,7 +16,6 @@ #include #include - #include #include