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..d7ad3c17c 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,24 @@ 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(), pugi::parse_default, pugi::encoding_utf8); + !success) { throw NoXmlFile(); } return result; } pugi::xml_document xml::parse(std::istream &in) { + const std::string bytes = util::stream::read(in); pugi::xml_document result; - if (const auto success = result.load(in); !success) { + if (const auto success = result.load_buffer(bytes.data(), bytes.size()); + !success) { throw NoXmlFile(); } return result; } -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 +123,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) { + if (!head.starts_with(""); + if (end == std::string_view::npos) { return {}; } - at = head.find_first_not_of(space, at + 1); - if (at == std::string_view::npos || (head[at] != '"' && head[at] != '\'')) { - 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 +180,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 +209,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 +223,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..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 &); @@ -42,9 +44,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/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 257c8885f..6f7c862d5 100644 --- a/test/src/internal/xml/xml_file_test.cpp +++ b/test/src/internal/xml/xml_file_test.cpp @@ -104,6 +104,13 @@ 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(R"()"), + "latin1"); 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,22 @@ 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_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); + EXPECT_THROW((void)parse(in), std::ios_base::failure); + EXPECT_THROW((void)read_declared_encoding(in), std::ios_base::failure); +}