From 2513f8a4525ac50685e3070b38d8729a81a4916b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 00:59:35 +0200 Subject: [PATCH 1/2] fix: check text cursor and compact side-table bounds --- src/odr/internal/common/element_registry.hpp | 3 ++ src/odr/internal/common/text_cursor.hpp | 10 ++++- test/CMakeLists.txt | 1 + .../internal/common/container_bounds_test.cpp | 37 +++++++++++++++++++ 4 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 test/src/internal/common/container_bounds_test.cpp diff --git a/src/odr/internal/common/element_registry.hpp b/src/odr/internal/common/element_registry.hpp index 4436c1367..11f8b8c36 100644 --- a/src/odr/internal/common/element_registry.hpp +++ b/src/odr/internal/common/element_registry.hpp @@ -58,6 +58,9 @@ template class SortedSideTable final { public: T &emplace(const ElementIdentifier id, T value) { + if (id > std::numeric_limits::max()) { + throw std::out_of_range("SortedSideTable::emplace: identifier too wide"); + } if (!m_entries.empty() && m_entries.back().first >= id) { throw std::invalid_argument( "SortedSideTable::emplace: identifier out of order"); diff --git a/src/odr/internal/common/text_cursor.hpp b/src/odr/internal/common/text_cursor.hpp index a1e13422e..3fd0a863e 100644 --- a/src/odr/internal/common/text_cursor.hpp +++ b/src/odr/internal/common/text_cursor.hpp @@ -3,6 +3,7 @@ #include #include +#include #include namespace odr::internal { @@ -32,7 +33,12 @@ class TextCursor { return c; } - void advance(const std::size_t count) { m_rest.remove_prefix(count); } + void advance(const std::size_t count) { + if (count > m_rest.size()) { + throw std::out_of_range("TextCursor::advance: past end of input"); + } + m_rest.remove_prefix(count); + } /// Back to a view @ref rest answered earlier, undoing what was read since. void seek(const std::string_view at) { m_rest = at; } @@ -46,7 +52,7 @@ class TextCursor { /// Whitespace ahead of it is filler; the text itself is the token. [[nodiscard]] bool consume(const char c) { skip_whitespace(); - if (peek() != c) { + if (empty() || peek() != c) { return false; } advance(1); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e230dca1c..6e1b02ca7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -54,6 +54,7 @@ add_executable(odr_test "src/internal/cfb/cfb_archive_test.cpp" + "src/internal/common/container_bounds_test.cpp" "src/internal/common/filesystem_test.cpp" "src/internal/common/list_numbering_test.cpp" "src/internal/common/path_test.cpp" diff --git a/test/src/internal/common/container_bounds_test.cpp b/test/src/internal/common/container_bounds_test.cpp new file mode 100644 index 000000000..5e22b66a7 --- /dev/null +++ b/test/src/internal/common/container_bounds_test.cpp @@ -0,0 +1,37 @@ +#include +#include + +#include + +#include +#include +#include +#include + +using namespace odr::internal; + +TEST(TextCursor, rejects_overread_and_distinguishes_nul_from_end) { + TextCursor cursor(std::string_view("\0x", 2)); + EXPECT_THROW(cursor.advance(3), std::out_of_range); + EXPECT_EQ(cursor.rest().size(), 2); + EXPECT_TRUE(cursor.consume('\0')); + EXPECT_EQ(cursor.take(), 'x'); + EXPECT_FALSE(cursor.consume('\0')); + EXPECT_TRUE(cursor.empty()); + EXPECT_NO_THROW(cursor.advance(0)); + EXPECT_THROW(cursor.advance(1), std::out_of_range); +} + +TEST(SortedSideTable, rejects_wide_ids_without_corrupting_order) { + SortedSideTable table; + table.emplace(1, 10); + table.emplace(255, 20); + EXPECT_THROW(table.emplace(256, 30), std::out_of_range); + EXPECT_THROW( + table.emplace(std::numeric_limits::max(), 40), + std::out_of_range); + EXPECT_EQ(table.at(1), 10); + EXPECT_EQ(table.at(255), 20); + EXPECT_EQ(table.find(256), nullptr); + EXPECT_EQ(table.find(0), nullptr); +} From 6be6d26b6b6a8b07574ba5a95a802114f2ad0d98 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 16:03:51 +0200 Subject: [PATCH 2/2] test: name the bound tests after their source files The other tests in test/src/internal/common are named after the file they test, so the TextCursor and SortedSideTable tests move from container_bounds_test.cpp to text_cursor_test.cpp and element_registry_test.cpp. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- test/CMakeLists.txt | 3 ++- ...nds_test.cpp => element_registry_test.cpp} | 14 ------------- test/src/internal/common/text_cursor_test.cpp | 20 +++++++++++++++++++ 3 files changed, 22 insertions(+), 15 deletions(-) rename test/src/internal/common/{container_bounds_test.cpp => element_registry_test.cpp} (56%) create mode 100644 test/src/internal/common/text_cursor_test.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 6e1b02ca7..0f5566c00 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -54,13 +54,14 @@ add_executable(odr_test "src/internal/cfb/cfb_archive_test.cpp" - "src/internal/common/container_bounds_test.cpp" + "src/internal/common/element_registry_test.cpp" "src/internal/common/filesystem_test.cpp" "src/internal/common/list_numbering_test.cpp" "src/internal/common/path_test.cpp" "src/internal/common/table_cursor_test.cpp" "src/internal/common/sheet_cell_source_test.cpp" "src/internal/common/table_range_test.cpp" + "src/internal/common/text_cursor_test.cpp" "src/internal/crypto/crypto_util_test.cpp" diff --git a/test/src/internal/common/container_bounds_test.cpp b/test/src/internal/common/element_registry_test.cpp similarity index 56% rename from test/src/internal/common/container_bounds_test.cpp rename to test/src/internal/common/element_registry_test.cpp index 5e22b66a7..7785ebe4f 100644 --- a/test/src/internal/common/container_bounds_test.cpp +++ b/test/src/internal/common/element_registry_test.cpp @@ -1,27 +1,13 @@ #include -#include #include #include #include #include -#include using namespace odr::internal; -TEST(TextCursor, rejects_overread_and_distinguishes_nul_from_end) { - TextCursor cursor(std::string_view("\0x", 2)); - EXPECT_THROW(cursor.advance(3), std::out_of_range); - EXPECT_EQ(cursor.rest().size(), 2); - EXPECT_TRUE(cursor.consume('\0')); - EXPECT_EQ(cursor.take(), 'x'); - EXPECT_FALSE(cursor.consume('\0')); - EXPECT_TRUE(cursor.empty()); - EXPECT_NO_THROW(cursor.advance(0)); - EXPECT_THROW(cursor.advance(1), std::out_of_range); -} - TEST(SortedSideTable, rejects_wide_ids_without_corrupting_order) { SortedSideTable table; table.emplace(1, 10); diff --git a/test/src/internal/common/text_cursor_test.cpp b/test/src/internal/common/text_cursor_test.cpp new file mode 100644 index 000000000..dcd3b004f --- /dev/null +++ b/test/src/internal/common/text_cursor_test.cpp @@ -0,0 +1,20 @@ +#include + +#include + +#include +#include + +using namespace odr::internal; + +TEST(TextCursor, rejects_overread_and_distinguishes_nul_from_end) { + TextCursor cursor(std::string_view("\0x", 2)); + EXPECT_THROW(cursor.advance(3), std::out_of_range); + EXPECT_EQ(cursor.rest().size(), 2); + EXPECT_TRUE(cursor.consume('\0')); + EXPECT_EQ(cursor.take(), 'x'); + EXPECT_FALSE(cursor.consume('\0')); + EXPECT_TRUE(cursor.empty()); + EXPECT_NO_THROW(cursor.advance(0)); + EXPECT_THROW(cursor.advance(1), std::out_of_range); +}