fix: preserve XML input lengths and parse declaration attributes

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.

src/odr/internal/xml/xml_util.cpp

@@ -5,13 +5,16 @@
 #include <odr/internal/abstract/file.hpp>
 #include <odr/internal/abstract/filesystem.hpp>
 #include <odr/internal/common/path.hpp>
+#include <odr/internal/common/text_cursor.hpp>
+#include <odr/internal/util/stream_util.hpp>
 
 #include <pugixml.hpp>
 
 #include <algorithm>
 #include <cstddef>
 #include <cstdint>
 #include <cstdlib>
+#include <limits>
 #include <memory>
 #include <new>
 #include <string_view>
@@ -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<std::streamsize>(probe.size()));
-  probe.resize(static_cast<std::size_t>(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("<?xml")) {
-    return {};
-  }
-  const std::size_t declaration_end = head.find("?>");
-  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("<?xml") || head.size() <= 5 ||
+      !util::string::is_ascii_whitespace(head[5])) {
     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] != '=') {
+  const std::size_t end = head.find("?>");
+  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::size_t>(
+                              std::numeric_limits<std::streamsize>::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<std::streamsize>(size));
-  if (stream->gcount() != static_cast<std::streamsize>(size)) {
+  if (stream->bad() || stream->gcount() != static_cast<std::streamsize>(size)) {
     throw NoXmlFile();
   }
 
@@ -208,8 +209,8 @@ std::vector<xml::StringToken> xml::tokenize_text(const std::string &text) {
   std::vector<StringToken> 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::StringToken> 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] == ' ' &&

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<const std::string_view> order);
 
-/// Throws unless @p in holds a well formed xml document.
-void check_xml_file(std::istream &in);
-
 /// The `encoding` pseudo-attribute of an `<?xml …?>` 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.

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"(<?xml version="1.0" encoding="ISO-8859-1"?>)"
+               R"(<office:document office:mimetype=")"
+               R"(application/vnd.oasis.opendocument.text"><office:body>)"
+               "<office:text><text:p>\xe4</text:p></office:text>"
+               "</office:body></office:document>"))
+          .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("<table:table/>")))
                                 .as_document_file()

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"(<?xml version="1.0"?><a/>)"), "");
   EXPECT_EQ(declared_encoding("<a/>"), "");
+  EXPECT_EQ(declared_encoding(R"(<?xml-stylesheet encoding="latin1"?><a/>)"),
+            "");
+  EXPECT_EQ(declared_encoding(R"(<?xml version='encoding="latin1"'?><a/>)"),
+            "");
+  EXPECT_EQ(declared_encoding(R"(<?xml fakeencoding="latin1"?><a/>)"), "");
+  EXPECT_EQ(declared_encoding(R"(<?xml version="1.0" encoding = "latin1"?>)"),
+            "latin1");
   EXPECT_EQ(declared_encoding(""), "");
   // no `?>` in the probe: an unterminated declaration names nothing
   EXPECT_EQ(declared_encoding(R"(<?xml encoding="UTF-8")"), "");

test/src/internal/xml/xml_util_test.cpp

@@ -3,6 +3,8 @@
 #include <gtest/gtest.h>
 
 #include <memory>
+#include <pugixml.hpp>
+#include <sstream>
 #include <string>
 
 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("<?xml version=\"1.0\" encoding=\"ISO-8859-1\"?><a>\xc3\xa4</a>");
+  EXPECT_STREQ(document.document_element().child_value(), "\xc3\xa4");
+}
+
+TEST(xml_util, rejects_failed_input_streams) {
+  std::istringstream in("<a/>");
+  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);
+}