From 46a95cbbe450f87517195884db400bf4aacaedd4 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 00:07:31 +0200 Subject: [PATCH 1/2] fix: validate BIFF record and worksheet boundaries --- CHANGELOG.md | 4 + src/odr/internal/oldms/spreadsheet/AGENTS.md | 9 +- src/odr/internal/oldms/spreadsheet/xls_io.cpp | 21 +++-- src/odr/internal/oldms/spreadsheet/xls_io.hpp | 2 +- .../internal/oldms/spreadsheet/xls_parser.cpp | 38 +++++++-- test/src/internal/oldms/xls_test.cpp | 84 ++++++++++++++++++- 6 files changed, 138 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6cb68083..fd48e266d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Legacy Excel rejects truncated BIFF records, missing substream terminators, + invalid worksheet ranges, and inconsistent multi-cell records. Shared-string + counts no longer trigger allocation before reading; numbers ignore the host locale. + - Legacy Word bounds font and piece tables by their declared lengths, checks piece coverage and offsets, and reads PLC entries without unaligned access. diff --git a/src/odr/internal/oldms/spreadsheet/AGENTS.md b/src/odr/internal/oldms/spreadsheet/AGENTS.md index 75276f0d6..4a628c43b 100644 --- a/src/odr/internal/oldms/spreadsheet/AGENTS.md +++ b/src/odr/internal/oldms/spreadsheet/AGENTS.md @@ -49,8 +49,10 @@ the remainder (§2.5.293). Formatting runs (`cRun`·4 B) and phonetic data **Fail early.** Throw on: a missing or non-BIFF8 `BOF` (`vers != 0x0600`); a non-`CONTINUE` record where a continuation is required; an out-of-range SST -index; a malformed `MulRk` body; an unknown `FormulaValue` type; a truncated -stream. Skip records that are not modelled. +index; a malformed `MulRk` body or column range; an unknown `FormulaValue` type; +invalid worksheet dimensions; a truncated header or skipped body; a missing +substream EOF or pending formula string result. SST storage grows only as +strings are read. Skip records that are not modelled. **Cell formatting is resolved at parse time, per XF, in the `StyleRegistry`.** The parser fills both registries, because BIFF keeps styles and content in @@ -82,7 +84,8 @@ returns the fill. - RK numbers (§2.5.217): the low 2 bits are flags, bit 0 `fX100` (divide by 100), bit 1 `fInt` (a 30-bit signed int, else the high 30 bits of an IEEE double). -- Numbers use `%.15g`, close to Excel's "General". Booleans are `TRUE` and +- Numbers use locale-independent `fmt` with 15 significant digits, close to + Excel's "General". Booleans are `TRUE` and `FALSE`. Errors (§2.5.10) are `#DIV/0!`, `#VALUE!`, `#REF!`, `#NAME?`, `#NUM!`, `#N/A` and `#NULL!`. - A date cell shows its raw serial number (open work §1). diff --git a/src/odr/internal/oldms/spreadsheet/xls_io.cpp b/src/odr/internal/oldms/spreadsheet/xls_io.cpp index 2778eaeda..13bc26247 100644 --- a/src/odr/internal/oldms/spreadsheet/xls_io.cpp +++ b/src/odr/internal/oldms/spreadsheet/xls_io.cpp @@ -5,9 +5,8 @@ #include #include -#include #include -#include +#include #include #include @@ -17,12 +16,14 @@ BiffReader::BiffReader(std::istream &in) : m_in{&in} {} bool BiffReader::next_record() { if (m_remaining > 0) { - m_in->ignore(static_cast(m_remaining)); - m_remaining = 0; + skip_bytes(m_remaining); } const std::optional header = util::byte_stream::try_read(*m_in); if (!header) { + if (m_in->bad() || !m_in->eof() || m_in->gcount() != 0) { + throw std::runtime_error("xls: truncated or unreadable record header"); + } return false; } @@ -89,6 +90,9 @@ void BiffReader::skip_bytes(const std::size_t count) { } const std::size_t take = std::min(left, m_remaining); m_in->ignore(static_cast(take)); + if (!*m_in || m_in->gcount() != static_cast(take)) { + throw std::runtime_error("xls: truncated record body"); + } left -= take; m_remaining -= take; } @@ -156,11 +160,14 @@ std::string BiffReader::read_xl_unicode_rich_extended_string() { return result; } -void BiffReader::expect_bof() { +void BiffReader::expect_bof(const std::uint16_t substream_type) { if (!next_record() || record_type() != biff_bof) { throw std::runtime_error("xls: expected BOF record"); } const auto bof = read(); + if (bof.dt != substream_type) { + throw std::runtime_error("xls: unexpected BIFF substream type"); + } if (bof.vers != bof_vers_biff8) { throw std::runtime_error("xls: unsupported BIFF version " + std::to_string(bof.vers)); @@ -176,9 +183,7 @@ std::string spreadsheet::format_number(const double value) { return "NaN"; } - std::array buffer{}; - std::snprintf(buffer.data(), buffer.size(), "%.15g", value); - return buffer.data(); + return fmt::format("{:.15g}", value); } std::string spreadsheet::error_code_string(const std::uint8_t error) { diff --git a/src/odr/internal/oldms/spreadsheet/xls_io.hpp b/src/odr/internal/oldms/spreadsheet/xls_io.hpp index 1476c7cba..8b753133f 100644 --- a/src/odr/internal/oldms/spreadsheet/xls_io.hpp +++ b/src/odr/internal/oldms/spreadsheet/xls_io.hpp @@ -61,7 +61,7 @@ class BiffReader final { /// formatting runs and phonetic data. Used for the SST string array. std::string read_xl_unicode_rich_extended_string(); - void expect_bof(); + void expect_bof(std::uint16_t substream_type); private: std::istream *m_in{nullptr}; diff --git a/src/odr/internal/oldms/spreadsheet/xls_parser.cpp b/src/odr/internal/oldms/spreadsheet/xls_parser.cpp index 61f97dff8..4545d7c9a 100644 --- a/src/odr/internal/oldms/spreadsheet/xls_parser.cpp +++ b/src/odr/internal/oldms/spreadsheet/xls_parser.cpp @@ -34,6 +34,9 @@ struct GlobalStyles { void add_cell(ElementRegistry ®istry, const ElementIdentifier sheet_id, const std::uint32_t column, const std::uint32_t row, const std::uint16_t ixfe, std::string text) { + if (column >= 256 || row >= 65536) { + throw std::runtime_error("xls: cell outside BIFF8 worksheet bounds"); + } auto [cell_id, cell_element, cell] = registry.create_sheet_cell_element(TablePosition(column, row)); cell.ixfe = ixfe; @@ -53,7 +56,7 @@ void add_cell(ElementRegistry ®istry, const ElementIdentifier sheet_id, void parse_globals(BiffReader &reader, std::vector &sheets, std::vector &shared_strings, GlobalStyles &styles) { - reader.expect_bof(); + reader.expect_bof(0x0005); while (reader.next_record() && reader.record_type() != biff_eof) { switch (reader.record_type()) { @@ -85,10 +88,9 @@ void parse_globals(BiffReader &reader, std::vector &sheets, } break; case biff_sst: { const auto head = reader.read(); - if (head.cstUnique < 0) { - throw std::runtime_error("xls: negative SST string count"); + if (head.cstUnique < 0 || head.cstTotal < head.cstUnique) { + throw std::runtime_error("xls: invalid SST string count"); } - shared_strings.reserve(static_cast(head.cstUnique)); for (std::int32_t i = 0; i < head.cstUnique; ++i) { shared_strings.push_back(reader.read_xl_unicode_rich_extended_string()); } @@ -97,6 +99,9 @@ void parse_globals(BiffReader &reader, std::vector &sheets, break; } } + if (reader.record_type() != biff_eof || reader.remaining() != 0) { + throw std::runtime_error("xls: missing or malformed EOF record"); + } } /// Sheet substream: dimensions plus one element per non-empty cell. @@ -104,7 +109,7 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, const ElementIdentifier sheet_id, const BoundSheet &info, const std::vector &shared_strings) { reader.seek(info.offset); - reader.expect_bof(); + reader.expect_bof(0x0010); ElementRegistry::Sheet &sheet = registry.sheet_element_at(sheet_id); sheet.name = info.name; @@ -121,6 +126,10 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, switch (reader.record_type()) { case biff_dimensions: { const auto dimensions = reader.read(); + if (dimensions.rwMic > dimensions.rwMac || dimensions.rwMac > 65536 || + dimensions.colMic > dimensions.colMac || dimensions.colMac > 256) { + throw std::runtime_error("xls: invalid worksheet dimensions"); + } sheet.dimensions = TableDimensions(dimensions.rwMac, dimensions.colMac); } break; case biff_labelsst: { @@ -145,12 +154,18 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, throw std::runtime_error("xls: malformed MulRk record"); } const std::size_t count = (reader.remaining() - 2) / 6; + if (count == 0 || column_first >= 256 || count > 256u - column_first) { + throw std::runtime_error("xls: invalid MulRk column range"); + } for (std::size_t i = 0; i < count; ++i) { const std::uint16_t ixfe = reader.read_u16(); const auto rk = reader.read(); add_cell(registry, sheet_id, column_first + i, row, ixfe, format_number(rk.decode())); } + if (reader.read_u16() != column_first + count - 1) { + throw std::runtime_error("xls: inconsistent MulRk final column"); + } } break; case biff_number: { const auto number = reader.read(); @@ -171,6 +186,9 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, : (boolerr.bBoolErr != 0 ? "TRUE" : "FALSE")); } break; case biff_formula: { + if (pending_string_cell) { + throw std::runtime_error("xls: missing formula string result"); + } const auto formula = reader.read(); const TablePosition position(formula.cell.col, formula.cell.rw); if (formula.val.is_xnum()) { @@ -211,6 +229,10 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, break; } } + if (reader.record_type() != biff_eof || reader.remaining() != 0 || + pending_string_cell) { + throw std::runtime_error("xls: incomplete worksheet substream"); + } } } // namespace @@ -222,7 +244,11 @@ ElementIdentifier spreadsheet::parse_tree(ElementRegistry ®istry, StyleRegistry &style_registry, const abstract::ReadableFilesystem &files) { - const auto workbook_stream = files.open(AbsPath("/Workbook"))->stream(); + const auto workbook_file = files.open(AbsPath("/Workbook")); + if (workbook_file == nullptr) { + throw std::runtime_error("xls: missing Workbook stream"); + } + const auto workbook_stream = workbook_file->stream(); BiffReader reader(*workbook_stream); std::vector bound_sheets; diff --git a/test/src/internal/oldms/xls_test.cpp b/test/src/internal/oldms/xls_test.cpp index 935b891ae..384e39b53 100644 --- a/test/src/internal/oldms/xls_test.cpp +++ b/test/src/internal/oldms/xls_test.cpp @@ -18,7 +18,9 @@ #include #include +#include #include +#include #include #include #include @@ -177,7 +179,8 @@ std::string make_bof(const std::uint16_t dt) { /// One-sheet workbook stream: `globals` records are wrapped with BOF/ /// BoundSheet8/EOF, the sheet substream holds the given `cells`. std::string make_workbook(const std::string &globals, - const std::vector &cells) { + const std::vector &cells, + const std::uint16_t cell_type = 0x0204) { const auto build_globals = [&](const std::uint32_t sheet_offset) { std::string result; append_record(result, 0x0809 /* BOF */, make_bof(0x0005)); @@ -197,7 +200,7 @@ std::string make_workbook(const std::string &globals, std::string sheet; append_record(sheet, 0x0809 /* BOF */, make_bof(0x0010)); for (const std::string &cell : cells) { - append_record(sheet, 0x0204 /* Label */, cell); + append_record(sheet, cell_type, cell); } append_record(sheet, 0x000A /* EOF */, ""); @@ -385,3 +388,80 @@ TEST(OldMs, xls_file_example_5000) { EXPECT_EQ(collect_text(sheet.cell(2, 5000)), "Alkire"); EXPECT_EQ(collect_text(sheet.cell(7, 5000)), "6125"); } + +TEST(OldMs, xls_truncated_records_are_not_end_of_stream) { + using internal::oldms::spreadsheet::BiffReader; + for (const std::size_t bytes : {1u, 2u, 3u}) { + std::istringstream in(std::string(bytes, '\0')); + BiffReader reader(in); + EXPECT_THROW(reader.next_record(), std::runtime_error); + } + std::string record; + append_record(record, 0x7777, "payload"); + record.pop_back(); + std::istringstream in(record); + BiffReader reader(in); + ASSERT_TRUE(reader.next_record()); + EXPECT_THROW(reader.next_record(), std::runtime_error); + + std::istringstream skipped(record); + BiffReader skipping(skipped); + ASSERT_TRUE(skipping.next_record()); + EXPECT_THROW(skipping.skip_bytes(7), std::runtime_error); + std::istringstream empty; + BiffReader exhausted(empty); + EXPECT_FALSE(exhausted.next_record()); +} + +TEST(OldMs, xls_substreams_and_counts_are_validated) { + const std::string valid = make_workbook({}, {}); + EXPECT_NO_THROW(static_cast(open_workbook(valid))); + EXPECT_THROW(open_workbook(valid.substr(0, valid.size() - 4)), + std::runtime_error); + std::string wrong_bof = valid; + wrong_bof[6] = '\x10'; // Worksheet BOF where workbook globals are required. + EXPECT_THROW(open_workbook(wrong_bof), std::runtime_error); + + std::string sst; + append_u32(sst, std::numeric_limits::max()); + append_u32(sst, std::numeric_limits::max()); + std::string globals; + append_record(globals, 0x00FC, sst); + EXPECT_THROW(open_workbook(make_workbook(globals, {})), std::runtime_error); + EXPECT_THROW(open_workbook(make_workbook({}, {make_label(0, 256, 0, "x")})), + std::runtime_error); + + std::string formula(12, '\0'); // CellRef and string FormulaValue prefix. + append_u16(formula, 0xFFFF); + formula.resize(20, '\0'); + EXPECT_THROW(open_workbook(make_workbook({}, {formula}, 0x0006)), + std::runtime_error); +} + +TEST(OldMs, xls_mulrk_checks_its_final_column) { + std::string cells; + append_u16(cells, 0); // row + append_u16(cells, 2); // first column + append_u16(cells, 0); // XF + append_u32(cells, (12u << 2) | 2u); + append_u16(cells, 2); // final column + const Document document = open_workbook(make_workbook({}, {cells}, 0x00BD)); + EXPECT_EQ( + collect_text(document.root_element().first_child().as_sheet().cell(2, 0)), + "12"); + cells[cells.size() - 2] = 3; + EXPECT_THROW(open_workbook(make_workbook({}, {cells}, 0x00BD)), + std::runtime_error); +} + +TEST(OldMs, xls_number_format_ignores_numeric_locale) { + struct LocaleGuard final { + std::string previous{std::setlocale(LC_NUMERIC, nullptr)}; + ~LocaleGuard() { std::setlocale(LC_NUMERIC, previous.c_str()); } + } guard; + if (std::setlocale(LC_NUMERIC, "de_DE.UTF-8") == nullptr && + std::setlocale(LC_NUMERIC, "de_DE.utf8") == nullptr) { + GTEST_SKIP() << "German locale unavailable"; + } + EXPECT_EQ(internal::oldms::spreadsheet::format_number(123.45), "123.45"); +} From 916f8c44fe5f3d47329b7f759adc05983cb1502b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 11:20:17 +0200 Subject: [PATCH 2/2] fix(xls): keep small quirks from refusing the workbook LibreOffice opens a workbook with these quirks, so the parser now accepts them too: a cell outside the BIFF8 grid is dropped, Dimensions is clamped, MulRk ignores colLast, a string formula without its String record stays empty, a clean end of stream ends a substream, and cstTotal is not checked. The record bounds stay strict. Skipped bodies use byte_stream::skip. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- CHANGELOG.md | 5 +-- src/odr/internal/oldms/spreadsheet/AGENTS.md | 21 +++++----- src/odr/internal/oldms/spreadsheet/xls_io.cpp | 8 ++-- .../internal/oldms/spreadsheet/xls_parser.cpp | 39 +++++++------------ test/src/internal/oldms/xls_test.cpp | 24 ++++++++---- 5 files changed, 47 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd48e266d..d0bab41f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,8 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- Legacy Excel rejects truncated BIFF records, missing substream terminators, - invalid worksheet ranges, and inconsistent multi-cell records. Shared-string - counts no longer trigger allocation before reading; numbers ignore the host locale. +- Legacy Excel rejects truncated BIFF records and a wrong substream type. It + drops cells outside the BIFF8 grid, and numbers ignore the host locale. - Legacy Word bounds font and piece tables by their declared lengths, checks piece coverage and offsets, and reads PLC entries without unaligned access. diff --git a/src/odr/internal/oldms/spreadsheet/AGENTS.md b/src/odr/internal/oldms/spreadsheet/AGENTS.md index 4a628c43b..5f6bf2db2 100644 --- a/src/odr/internal/oldms/spreadsheet/AGENTS.md +++ b/src/odr/internal/oldms/spreadsheet/AGENTS.md @@ -47,12 +47,16 @@ each hop, because the continuation re-declares compressed versus UTF-16 for the remainder (§2.5.293). Formatting runs (`cRun`·4 B) and phonetic data (`cbExtRst` B) are read and skipped. -**Fail early.** Throw on: a missing or non-BIFF8 `BOF` (`vers != 0x0600`); a -non-`CONTINUE` record where a continuation is required; an out-of-range SST -index; a malformed `MulRk` body or column range; an unknown `FormulaValue` type; -invalid worksheet dimensions; a truncated header or skipped body; a missing -substream EOF or pending formula string result. SST storage grows only as -strings are read. Skip records that are not modelled. +**Fail early.** Throw on: a missing or non-BIFF8 `BOF` (`vers != 0x0600`), or +one with the wrong substream type; a non-`CONTINUE` record where a continuation +is required; an out-of-range SST index; a malformed `MulRk` body; an unknown +`FormulaValue` type; a truncated record header or body. SST storage grows only +as strings are read. Skip records that are not modelled. + +**Pass through**, as LibreOffice does: a cell outside the BIFF8 grid is +dropped; `Dimensions` is clamped to the grid; `MulRk` takes its cell count from +the body size and ignores `colLast`; a string formula without its `String` +record stays empty; a clean end of stream ends a substream without `EOF`. **Cell formatting is resolved at parse time, per XF, in the `StyleRegistry`.** The parser fills both registries, because BIFF keeps styles and content in @@ -85,9 +89,8 @@ returns the fill. 100), bit 1 `fInt` (a 30-bit signed int, else the high 30 bits of an IEEE double). - Numbers use locale-independent `fmt` with 15 significant digits, close to - Excel's "General". Booleans are `TRUE` and - `FALSE`. Errors (§2.5.10) are `#DIV/0!`, `#VALUE!`, `#REF!`, `#NAME?`, - `#NUM!`, `#N/A` and `#NULL!`. + Excel's "General". Booleans are `TRUE` and `FALSE`. Errors (§2.5.10) are + `#DIV/0!`, `#VALUE!`, `#REF!`, `#NAME?`, `#NUM!`, `#N/A` and `#NULL!`. - A date cell shows its raw serial number (open work §1). ## Tests diff --git a/src/odr/internal/oldms/spreadsheet/xls_io.cpp b/src/odr/internal/oldms/spreadsheet/xls_io.cpp index 13bc26247..e9624dd55 100644 --- a/src/odr/internal/oldms/spreadsheet/xls_io.cpp +++ b/src/odr/internal/oldms/spreadsheet/xls_io.cpp @@ -6,10 +6,11 @@ #include #include -#include #include #include +#include + namespace odr::internal::oldms::spreadsheet { BiffReader::BiffReader(std::istream &in) : m_in{&in} {} @@ -89,10 +90,7 @@ void BiffReader::skip_bytes(const std::size_t count) { next_continue(); } const std::size_t take = std::min(left, m_remaining); - m_in->ignore(static_cast(take)); - if (!*m_in || m_in->gcount() != static_cast(take)) { - throw std::runtime_error("xls: truncated record body"); - } + util::byte_stream::skip(*m_in, take); left -= take; m_remaining -= take; } diff --git a/src/odr/internal/oldms/spreadsheet/xls_parser.cpp b/src/odr/internal/oldms/spreadsheet/xls_parser.cpp index 4545d7c9a..ea3bfabe4 100644 --- a/src/odr/internal/oldms/spreadsheet/xls_parser.cpp +++ b/src/odr/internal/oldms/spreadsheet/xls_parser.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -17,6 +18,9 @@ namespace odr::internal::oldms::spreadsheet { namespace { +constexpr std::uint32_t max_columns = 256; +constexpr std::uint32_t max_rows = 65536; + struct BoundSheet { std::uint32_t offset{0}; std::string name; @@ -34,8 +38,9 @@ struct GlobalStyles { void add_cell(ElementRegistry ®istry, const ElementIdentifier sheet_id, const std::uint32_t column, const std::uint32_t row, const std::uint16_t ixfe, std::string text) { - if (column >= 256 || row >= 65536) { - throw std::runtime_error("xls: cell outside BIFF8 worksheet bounds"); + // LibreOffice also drops a cell outside the BIFF8 grid. + if (column >= max_columns || row >= max_rows) { + return; } auto [cell_id, cell_element, cell] = registry.create_sheet_cell_element(TablePosition(column, row)); @@ -88,8 +93,8 @@ void parse_globals(BiffReader &reader, std::vector &sheets, } break; case biff_sst: { const auto head = reader.read(); - if (head.cstUnique < 0 || head.cstTotal < head.cstUnique) { - throw std::runtime_error("xls: invalid SST string count"); + if (head.cstUnique < 0) { + throw std::runtime_error("xls: negative SST string count"); } for (std::int32_t i = 0; i < head.cstUnique; ++i) { shared_strings.push_back(reader.read_xl_unicode_rich_extended_string()); @@ -99,9 +104,6 @@ void parse_globals(BiffReader &reader, std::vector &sheets, break; } } - if (reader.record_type() != biff_eof || reader.remaining() != 0) { - throw std::runtime_error("xls: missing or malformed EOF record"); - } } /// Sheet substream: dimensions plus one element per non-empty cell. @@ -126,11 +128,9 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, switch (reader.record_type()) { case biff_dimensions: { const auto dimensions = reader.read(); - if (dimensions.rwMic > dimensions.rwMac || dimensions.rwMac > 65536 || - dimensions.colMic > dimensions.colMac || dimensions.colMac > 256) { - throw std::runtime_error("xls: invalid worksheet dimensions"); - } - sheet.dimensions = TableDimensions(dimensions.rwMac, dimensions.colMac); + sheet.dimensions = TableDimensions( + std::min(dimensions.rwMac, max_rows), + std::min(dimensions.colMac, max_columns)); } break; case biff_labelsst: { const auto label = reader.read(); @@ -154,18 +154,12 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, throw std::runtime_error("xls: malformed MulRk record"); } const std::size_t count = (reader.remaining() - 2) / 6; - if (count == 0 || column_first >= 256 || count > 256u - column_first) { - throw std::runtime_error("xls: invalid MulRk column range"); - } for (std::size_t i = 0; i < count; ++i) { const std::uint16_t ixfe = reader.read_u16(); const auto rk = reader.read(); add_cell(registry, sheet_id, column_first + i, row, ixfe, format_number(rk.decode())); } - if (reader.read_u16() != column_first + count - 1) { - throw std::runtime_error("xls: inconsistent MulRk final column"); - } } break; case biff_number: { const auto number = reader.read(); @@ -186,9 +180,8 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, : (boolerr.bBoolErr != 0 ? "TRUE" : "FALSE")); } break; case biff_formula: { - if (pending_string_cell) { - throw std::runtime_error("xls: missing formula string result"); - } + // Without its String record, a formula string result stays empty. + pending_string_cell.reset(); const auto formula = reader.read(); const TablePosition position(formula.cell.col, formula.cell.rw); if (formula.val.is_xnum()) { @@ -229,10 +222,6 @@ void parse_sheet(BiffReader &reader, ElementRegistry ®istry, break; } } - if (reader.record_type() != biff_eof || reader.remaining() != 0 || - pending_string_cell) { - throw std::runtime_error("xls: incomplete worksheet substream"); - } } } // namespace diff --git a/test/src/internal/oldms/xls_test.cpp b/test/src/internal/oldms/xls_test.cpp index 384e39b53..40d9a5624 100644 --- a/test/src/internal/oldms/xls_test.cpp +++ b/test/src/internal/oldms/xls_test.cpp @@ -416,7 +416,9 @@ TEST(OldMs, xls_truncated_records_are_not_end_of_stream) { TEST(OldMs, xls_substreams_and_counts_are_validated) { const std::string valid = make_workbook({}, {}); EXPECT_NO_THROW(static_cast(open_workbook(valid))); - EXPECT_THROW(open_workbook(valid.substr(0, valid.size() - 4)), + EXPECT_NO_THROW( + static_cast(open_workbook(valid.substr(0, valid.size() - 4)))); + EXPECT_THROW(open_workbook(valid.substr(0, valid.size() - 5)), std::runtime_error); std::string wrong_bof = valid; wrong_bof[6] = '\x10'; // Worksheet BOF where workbook globals are required. @@ -428,17 +430,23 @@ TEST(OldMs, xls_substreams_and_counts_are_validated) { std::string globals; append_record(globals, 0x00FC, sst); EXPECT_THROW(open_workbook(make_workbook(globals, {})), std::runtime_error); - EXPECT_THROW(open_workbook(make_workbook({}, {make_label(0, 256, 0, "x")})), - std::runtime_error); +} + +TEST(OldMs, xls_tolerates_cells_outside_the_grid_and_missing_results) { + const Document outside = + open_workbook(make_workbook({}, {make_label(0, 256, 0, "x")})); + EXPECT_EQ(collect_text( + outside.root_element().first_child().as_sheet().cell(256, 0)), + ""); std::string formula(12, '\0'); // CellRef and string FormulaValue prefix. append_u16(formula, 0xFFFF); formula.resize(20, '\0'); - EXPECT_THROW(open_workbook(make_workbook({}, {formula}, 0x0006)), - std::runtime_error); + EXPECT_NO_THROW(static_cast( + open_workbook(make_workbook({}, {formula, formula}, 0x0006)))); } -TEST(OldMs, xls_mulrk_checks_its_final_column) { +TEST(OldMs, xls_mulrk_takes_its_cells_from_the_body_size) { std::string cells; append_u16(cells, 0); // row append_u16(cells, 2); // first column @@ -450,8 +458,8 @@ TEST(OldMs, xls_mulrk_checks_its_final_column) { collect_text(document.root_element().first_child().as_sheet().cell(2, 0)), "12"); cells[cells.size() - 2] = 3; - EXPECT_THROW(open_workbook(make_workbook({}, {cells}, 0x00BD)), - std::runtime_error); + EXPECT_NO_THROW( + static_cast(open_workbook(make_workbook({}, {cells}, 0x00BD)))); } TEST(OldMs, xls_number_format_ignores_numeric_locale) {