From 38d5006b1681afdbafeac47bf0cb66042adc179f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 4 Oct 2026 22:56:15 +0200 Subject: [PATCH 1/2] fix: preserve XML input lengths and parse declaration attributes --- CHANGELOG.md | 3 + src/odr/internal/xml/xml_util.cpp | 83 +++++++++++-------------- src/odr/internal/xml/xml_util.hpp | 3 - test/src/internal/xml/xml_file_test.cpp | 5 ++ test/src/internal/xml/xml_util_test.cpp | 16 +++++ 5 files changed, 62 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1a265742..24c00d0ee 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 +- XML helpers preserve embedded zero bytes when detecting UTF-16/32, report + stream failures, and read encoding names only from declaration attributes. + - Table spans use storage proportional to merged cells, survive skipped rows, and reject overflowing dimensions. Recalculation bounds formula spans before multiplying or accumulating their size. diff --git a/src/odr/internal/xml/xml_util.cpp b/src/odr/internal/xml/xml_util.cpp index 603250b1e..1096dc2fc 100644 --- a/src/odr/internal/xml/xml_util.cpp +++ b/src/odr/internal/xml/xml_util.cpp @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include @@ -12,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -74,22 +77,16 @@ std::string xml::escape_attribute(const std::string_view value) { pugi::xml_document xml::parse(const std::string &in) { pugi::xml_document result; - if (const auto success = result.load_string(in.c_str()); !success) { + if (const auto success = result.load_buffer(in.data(), in.size()); !success) { throw NoXmlFile(); } return result; } pugi::xml_document xml::parse(std::istream &in) { - pugi::xml_document result; - if (const auto success = result.load(in); !success) { - throw NoXmlFile(); - } - return result; + return parse(util::stream::read(in)); } -void xml::check_xml_file(std::istream &in) { std::ignore = parse(in); } - void xml::set_attribute(pugi::xml_node node, const char *name, const char *value) { pugi::xml_attribute attribute = node.attribute(name); @@ -118,54 +115,50 @@ xml::insert_in_sequence(pugi::xml_node parent, const char *name, } std::string xml::read_declared_encoding(std::istream &in) { - static constexpr std::size_t probe_size = 1024; - static constexpr std::string_view space = " \t\r\n"; - - std::string probe(probe_size, '\0'); - in.read(probe.data(), static_cast(probe.size())); - probe.resize(static_cast(in.gcount())); - + const std::string probe = util::stream::read(in, 1024); std::string_view head(probe); if (head.starts_with("\xef\xbb\xbf")) { head.remove_prefix(3); } - if (!head.starts_with(""); - if (declaration_end == std::string_view::npos) { - return {}; - } - head = head.substr(0, declaration_end); - - const std::size_t name = head.find("encoding"); - if (name == std::string_view::npos) { - return {}; - } - head.remove_prefix(name + std::string_view("encoding").size()); - - std::size_t at = head.find_first_not_of(space); - if (at == std::string_view::npos || head[at] != '=') { + if (!head.starts_with(""); + if (end == std::string_view::npos) { return {}; } - const char quote = head[at]; - ++at; - const std::size_t value_end = head.find(quote, at); - if (value_end == std::string_view::npos) { - return {}; + TextCursor cursor(head.substr(5, end - 5)); + while (!cursor.empty()) { + cursor.skip_whitespace(); + const auto name = cursor.take_while(util::string::is_ascii_letter); + if (name.empty() || !cursor.consume('=')) { + return {}; + } + cursor.skip_whitespace(); + const char quote = cursor.take(); + if (quote != '\'' && quote != '"') { + return {}; + } + const auto value = cursor.rest(); + const auto close = value.find(quote); + if (close == std::string_view::npos) { + return {}; + } + if (name == "encoding") { + return std::string(value.substr(0, close)); + } + cursor.advance(close + 1); } - return std::string(head.substr(at, value_end - at)); + return {}; } /// Reads @p file once; pugixml's stream loader buffers it twice. The buffer is /// `malloc`ed because pugixml takes it over and frees it, parse or no parse. pugi::xml_document xml::parse(const abstract::File &file) { const std::size_t size = file.size(); - if (size == 0) { + if (size == 0 || size > static_cast( + std::numeric_limits::max())) { throw NoXmlFile(); } // before the buffer: opening an entry that is encrypted or compressed by a @@ -179,7 +172,7 @@ pugi::xml_document xml::parse(const abstract::File &file) { } stream->read(buffer.get(), static_cast(size)); - if (stream->gcount() != static_cast(size)) { + if (stream->bad() || stream->gcount() != static_cast(size)) { throw NoXmlFile(); } @@ -208,8 +201,8 @@ std::vector xml::tokenize_text(const std::string &text) { std::vector result; auto token_type{StringToken::Type::none}; - std::uint32_t token_start{0}; - auto close_token = [&](const std::uint32_t token_end, + std::size_t token_start{0}; + auto close_token = [&](const std::size_t token_end, const StringToken::Type new_token_type) { if (token_type == new_token_type) { return; @@ -222,7 +215,7 @@ std::vector xml::tokenize_text(const std::string &text) { token_type = new_token_type; }; - for (std::uint32_t i = 0; i < text.size(); ++i) { + for (std::size_t i = 0; i < text.size(); ++i) { if (text[i] == '\t') { close_token(i, StringToken::Type::tabs); } else if (text[i] == ' ' && diff --git a/src/odr/internal/xml/xml_util.hpp b/src/odr/internal/xml/xml_util.hpp index cd1459502..9ab05416b 100644 --- a/src/odr/internal/xml/xml_util.hpp +++ b/src/odr/internal/xml/xml_util.hpp @@ -42,9 +42,6 @@ void set_attribute(pugi::xml_node node, const char *name, const char *value); pugi::xml_node insert_in_sequence(pugi::xml_node parent, const char *name, std::span order); -/// Throws unless @p in holds a well formed xml document. -void check_xml_file(std::istream &in); - /// The `encoding` pseudo-attribute of an `` declaration at the head of /// @p in, empty if there is none. Ascii only - utf-16 and utf-32 are named by /// their byte order mark. diff --git a/test/src/internal/xml/xml_file_test.cpp b/test/src/internal/xml/xml_file_test.cpp index 257c8885f..f61dd7b02 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -104,6 +104,11 @@ TEST(XmlDeclaration, the_encoding_pseudo_attribute_is_read_off_the_bytes) { EXPECT_EQ(declared_encoding(R"()"), ""); EXPECT_EQ(declared_encoding(""), ""); + EXPECT_EQ(declared_encoding(R"()"), + ""); + EXPECT_EQ(declared_encoding(R"()"), + ""); + EXPECT_EQ(declared_encoding(R"()"), ""); EXPECT_EQ(declared_encoding(""), ""); // no `?>` in the probe: an unterminated declaration names nothing EXPECT_EQ(declared_encoding(R"( #include +#include +#include #include using namespace odr::internal::xml; @@ -74,3 +76,17 @@ TEST(xml_util, tokenize_text_single_space) { EXPECT_EQ(StringToken::Type::string, tokens[0].type); EXPECT_EQ("a b", tokens[0].string); } + +TEST(xml_util, parses_utf16_buffers_without_c_string_truncation) { + const std::string bytes("\xff\xfe<\0a\0/\0>\0", 10); + EXPECT_STREQ(parse(bytes).document_element().name(), "a"); + std::istringstream in(bytes); + EXPECT_STREQ(parse(in).document_element().name(), "a"); +} + +TEST(xml_util, rejects_failed_input_streams) { + std::istringstream in(""); + in.setstate(std::ios::badbit); + EXPECT_THROW((void)parse(in), std::ios_base::failure); + EXPECT_THROW((void)read_declared_encoding(in), std::ios_base::failure); +} From 8421f8e16d38c86c90b96e6d9df9111d8bd69498 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 08:46:23 +0200 Subject: [PATCH 2/2] fix(xml): parse a string as UTF-8, and only detect the encoding of a stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit load_buffer with encoding_auto reads a latin1 declaration. The flat ODF passes TextFile::text(), which is already UTF-8 but keeps its declaration, so a latin1 document decoded its text twice and read "ä" as "ä". The string overload now states encoding_utf8, as load_string did, and keeps the explicit length. The stream overload reads raw bytes, so it keeps the detection. Tests cover the string overload, a latin1 flat ODF document, and whitespace before the `=` of a declaration attribute. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/internal/xml/xml_util.cpp | 12 ++++++++++-- src/odr/internal/xml/xml_util.hpp | 6 ++++-- test/src/internal/odf/odf_flat_file_test.cpp | 16 ++++++++++++++++ test/src/internal/xml/xml_file_test.cpp | 2 ++ test/src/internal/xml/xml_util_test.cpp | 13 +++++++++---- 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/odr/internal/xml/xml_util.cpp b/src/odr/internal/xml/xml_util.cpp index 1096dc2fc..d7ad3c17c 100644 --- a/src/odr/internal/xml/xml_util.cpp +++ b/src/odr/internal/xml/xml_util.cpp @@ -77,14 +77,22 @@ std::string xml::escape_attribute(const std::string_view value) { pugi::xml_document xml::parse(const std::string &in) { pugi::xml_document result; - if (const auto success = result.load_buffer(in.data(), in.size()); !success) { + if (const auto success = result.load_buffer( + in.data(), in.size(), pugi::parse_default, pugi::encoding_utf8); + !success) { throw NoXmlFile(); } return result; } pugi::xml_document xml::parse(std::istream &in) { - return parse(util::stream::read(in)); + const std::string bytes = util::stream::read(in); + pugi::xml_document result; + if (const auto success = result.load_buffer(bytes.data(), bytes.size()); + !success) { + throw NoXmlFile(); + } + return result; } void xml::set_attribute(pugi::xml_node node, const char *name, diff --git a/src/odr/internal/xml/xml_util.hpp b/src/odr/internal/xml/xml_util.hpp index 9ab05416b..be2833d48 100644 --- a/src/odr/internal/xml/xml_util.hpp +++ b/src/odr/internal/xml/xml_util.hpp @@ -28,9 +28,11 @@ namespace odr::internal::xml { /// As @ref escape_text, plus the `"` that would end an attribute value. [[nodiscard]] std::string escape_attribute(std::string_view value); +/// Parses UTF-8 text, whatever encoding its declaration still names. pugi::xml_document parse(const std::string &); -/// Buffers @p in twice on the way in; prefer the @ref abstract::File overload, -/// which reads once against the size the file knows. +/// Detects the encoding of the bytes. Buffers @p in twice on the way in; prefer +/// the @ref abstract::File overload, which reads once against the size the file +/// knows. pugi::xml_document parse(std::istream &); pugi::xml_document parse(const abstract::File &); pugi::xml_document parse(const abstract::ReadableFilesystem &, const AbsPath &); diff --git a/test/src/internal/odf/odf_flat_file_test.cpp b/test/src/internal/odf/odf_flat_file_test.cpp index 3fbf087e2..8a05c6f95 100644 --- a/test/src/internal/odf/odf_flat_file_test.cpp +++ b/test/src/internal/odf/odf_flat_file_test.cpp @@ -82,6 +82,22 @@ std::string packaged_text(const std::string &name, const std::string &body) { } // namespace +TEST(FlatOdf, a_latin1_document_reads_its_text_once) { + const Document document = + open(File::from_memory( + R"()" + R"()" + "\xe4" + "")) + .as_document_file() + .document(); + const Element paragraph = + first_of_type(document.root_element(), ElementType::paragraph); + ASSERT_TRUE(paragraph); + EXPECT_EQ(paragraph.first_child().as_text().content(), "\xc3\xa4"); +} + TEST(FlatOdf, missing_related_elements_return_empty_handles) { const Document document = open(File::from_memory(flat_text(""))) .as_document_file() diff --git a/test/src/internal/xml/xml_file_test.cpp b/test/src/internal/xml/xml_file_test.cpp index f61dd7b02..6f7c862d5 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -109,6 +109,8 @@ TEST(XmlDeclaration, the_encoding_pseudo_attribute_is_read_off_the_bytes) { EXPECT_EQ(declared_encoding(R"()"), ""); EXPECT_EQ(declared_encoding(R"()"), ""); + EXPECT_EQ(declared_encoding(R"()"), + "latin1"); EXPECT_EQ(declared_encoding(""), ""); // no `?>` in the probe: an unterminated declaration names nothing EXPECT_EQ(declared_encoding(R"(\0", 10); - EXPECT_STREQ(parse(bytes).document_element().name(), "a"); - std::istringstream in(bytes); +TEST(xml_util, parses_utf16_streams_without_c_string_truncation) { + std::istringstream in(std::string("\xff\xfe<\0a\0/\0>\0", 10)); EXPECT_STREQ(parse(in).document_element().name(), "a"); } +// A flat ODF's text is already UTF-8, whatever its declaration still names. +TEST(xml_util, parses_a_string_as_utf8) { + const pugi::xml_document document = + parse("\xc3\xa4"); + EXPECT_STREQ(document.document_element().child_value(), "\xc3\xa4"); +} + TEST(xml_util, rejects_failed_input_streams) { std::istringstream in(""); in.setstate(std::ios::badbit);