From cb8ebe8a0048f033bc25a60f0d8a2f9af57c64ad Mon Sep 17 00:00:00 2001 From: ykiko Date: Sun, 29 Mar 2026 18:41:20 +0800 Subject: [PATCH] feat(lsp): return optional from PositionMapper boundary methods to_position and to_offset receive untrusted LSP client input that may be out of range. Return std::optional instead of asserting so callers can handle invalid positions gracefully. Internal helpers keep their asserts since they operate on already-validated data. Co-Authored-By: Claude Opus 4.6 (1M context) --- include/eventide/ipc/lsp/position.h | 7 ++- src/ipc/lsp/position.cpp | 27 ++++++-- tests/unit/ipc/lsp/position_tests.cpp | 88 ++++++++++++++++++++++----- 3 files changed, 99 insertions(+), 23 deletions(-) diff --git a/include/eventide/ipc/lsp/position.h b/include/eventide/ipc/lsp/position.h index dc946999..d871863e 100644 --- a/include/eventide/ipc/lsp/position.h +++ b/include/eventide/ipc/lsp/position.h @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -49,10 +50,12 @@ class PositionMapper { std::uint32_t end_byte_column) const; /// Converts a byte offset to LSP `Position{line, character}`. - protocol::Position to_position(std::uint32_t offset) const; + /// Returns `std::nullopt` when the offset is out of range. + std::optional to_position(std::uint32_t offset) const; /// Converts LSP position to byte offset in the original text. - std::uint32_t to_offset(protocol::Position position) const; + /// Returns `std::nullopt` when the position is out of range. + std::optional to_offset(protocol::Position position) const; /// Measures `text` length in the current position encoding. std::uint32_t measure(std::string_view text) const; diff --git a/src/ipc/lsp/position.cpp b/src/ipc/lsp/position.cpp index ad96f951..b138448f 100644 --- a/src/ipc/lsp/position.cpp +++ b/src/ipc/lsp/position.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include namespace { @@ -234,18 +235,29 @@ std::uint32_t PositionMapper::length(std::uint32_t line, return measure(content.substr(start + begin_byte_column, size)); } -protocol::Position PositionMapper::to_position(std::uint32_t offset) const { +std::optional PositionMapper::to_position(std::uint32_t offset) const { + if(offset > content.size()) [[unlikely]] { + return std::nullopt; + } auto line = line_of(offset); auto column = offset - line_start(line); + if(line_start(line) + column > line_end_exclusive(line)) [[unlikely]] { + return std::nullopt; + } return protocol::Position{ .line = line, .character = character(line, column), }; } -std::uint32_t PositionMapper::to_offset(protocol::Position position) const { +std::optional PositionMapper::to_offset(protocol::Position position) const { auto line = position.line; auto target = position.character; + + if(line >= line_starts.size()) [[unlikely]] { + return std::nullopt; + } + auto begin = line_start(line); auto end = line_end_exclusive(line); @@ -254,7 +266,9 @@ std::uint32_t PositionMapper::to_offset(protocol::Position position) const { } if(encoding == PositionEncoding::UTF8) { - assert(begin + target <= end && "character out of range"); + if(begin + target > end) [[unlikely]] { + return std::nullopt; + } return begin + target; } @@ -263,7 +277,9 @@ std::uint32_t PositionMapper::to_offset(protocol::Position position) const { for(std::size_t index = 0; index < text.size();) { auto [utf8, utf16] = next_codepoint_sizes(text, index); auto step = (encoding == PositionEncoding::UTF16) ? utf16 : 1; - assert(target >= step && "character out of range"); + if(target < step) [[unlikely]] { + return std::nullopt; + } target -= step; offset += utf8; index += utf8; @@ -272,8 +288,7 @@ std::uint32_t PositionMapper::to_offset(protocol::Position position) const { } } - assert(false && "character out of range"); - return end; + return std::nullopt; } } // namespace eventide::ipc::lsp diff --git a/tests/unit/ipc/lsp/position_tests.cpp b/tests/unit/ipc/lsp/position_tests.cpp index 0560a59d..23358a6c 100644 --- a/tests/unit/ipc/lsp/position_tests.cpp +++ b/tests/unit/ipc/lsp/position_tests.cpp @@ -22,8 +22,9 @@ TEST_CASE(utf16_column_counts) { PositionMapper converter(content, PositionEncoding::UTF16); auto position = converter.to_position(4); - ASSERT_EQ(position.line, 0U); - ASSERT_EQ(position.character, 2U); + ASSERT_TRUE(position.has_value()); + ASSERT_EQ(position->line, 0U); + ASSERT_EQ(position->character, 2U); } TEST_CASE(round_trip_offsets) { @@ -34,7 +35,10 @@ TEST_CASE(round_trip_offsets) { PositionMapper converter(content, encoding); for(auto offset: offsets) { auto position = converter.to_position(offset); - ASSERT_EQ(converter.to_offset(position), offset); + ASSERT_TRUE(position.has_value()); + auto mapped = converter.to_offset(*position); + ASSERT_TRUE(mapped.has_value()); + ASSERT_EQ(*mapped, offset); } } } @@ -66,19 +70,28 @@ TEST_CASE(position_offset_values) { for(const auto& sample: samples) { auto p8 = utf8_converter.to_position(sample.offset); - EXPECT_EQ(p8.line, sample.line); - EXPECT_EQ(p8.character, sample.utf8_character); - EXPECT_EQ(utf8_converter.to_offset(p8), sample.offset); + ASSERT_TRUE(p8.has_value()); + EXPECT_EQ(p8->line, sample.line); + EXPECT_EQ(p8->character, sample.utf8_character); + auto o8 = utf8_converter.to_offset(*p8); + ASSERT_TRUE(o8.has_value()); + EXPECT_EQ(*o8, sample.offset); auto p16 = utf16_converter.to_position(sample.offset); - EXPECT_EQ(p16.line, sample.line); - EXPECT_EQ(p16.character, sample.utf16_character); - EXPECT_EQ(utf16_converter.to_offset(p16), sample.offset); + ASSERT_TRUE(p16.has_value()); + EXPECT_EQ(p16->line, sample.line); + EXPECT_EQ(p16->character, sample.utf16_character); + auto o16 = utf16_converter.to_offset(*p16); + ASSERT_TRUE(o16.has_value()); + EXPECT_EQ(*o16, sample.offset); auto p32 = utf32_converter.to_position(sample.offset); - EXPECT_EQ(p32.line, sample.line); - EXPECT_EQ(p32.character, sample.utf32_character); - EXPECT_EQ(utf32_converter.to_offset(p32), sample.offset); + ASSERT_TRUE(p32.has_value()); + EXPECT_EQ(p32->line, sample.line); + EXPECT_EQ(p32->character, sample.utf32_character); + auto o32 = utf32_converter.to_offset(*p32); + ASSERT_TRUE(o32.has_value()); + EXPECT_EQ(*o32, sample.offset); } } @@ -140,7 +153,10 @@ TEST_CASE(roundtrip_multiline_boundaries) { PositionMapper converter(content, encoding); for(auto offset: boundaries) { auto position = converter.to_position(offset); - ASSERT_EQ(converter.to_offset(position), offset); + ASSERT_TRUE(position.has_value()); + auto mapped = converter.to_offset(*position); + ASSERT_TRUE(mapped.has_value()); + ASSERT_EQ(*mapped, offset); } } } @@ -179,8 +195,10 @@ TEST_CASE(invalid_position_stability) { PositionMapper converter(content, encoding); for(std::uint32_t offset = 0; offset <= content.size(); ++offset) { auto position = converter.to_position(offset); - auto mapped_offset = converter.to_offset(position); - EXPECT_TRUE(mapped_offset <= content.size()); + ASSERT_TRUE(position.has_value()); + auto mapped_offset = converter.to_offset(*position); + ASSERT_TRUE(mapped_offset.has_value()); + EXPECT_TRUE(*mapped_offset <= content.size()); } } }; @@ -216,6 +234,46 @@ TEST_CASE(strict_utf8_validation) { expect_invalid_sequence('a', 0xF0u, 0x9Fu, 'b'); } +TEST_CASE(to_position_out_of_range) { + std::string_view content = "abc\ndef"; + PositionMapper converter(content, PositionEncoding::UTF8); + + // Offset beyond content size. + EXPECT_FALSE(converter.to_position(100).has_value()); + EXPECT_FALSE(converter.to_position(8).has_value()); + + // Offset at content size is valid (EOF position). + EXPECT_TRUE(converter.to_position(7).has_value()); +} + +TEST_CASE(to_offset_line_out_of_range) { + std::string_view content = "abc\ndef"; + PositionMapper converter(content, PositionEncoding::UTF8); + + // Line beyond document. + EXPECT_FALSE(converter.to_offset({.line = 5, .character = 0}).has_value()); + EXPECT_FALSE(converter.to_offset({.line = 2, .character = 0}).has_value()); + + // Valid last line. + EXPECT_TRUE(converter.to_offset({.line = 1, .character = 0}).has_value()); +} + +TEST_CASE(to_offset_character_out_of_range) { + std::string_view content = "abc\ndef"; + + for(auto encoding: {PositionEncoding::UTF8, PositionEncoding::UTF16, PositionEncoding::UTF32}) { + PositionMapper converter(content, encoding); + + // Character beyond line length. + EXPECT_FALSE(converter.to_offset({.line = 0, .character = 10}).has_value()); + EXPECT_FALSE(converter.to_offset({.line = 1, .character = 4}).has_value()); + + // Valid end of line. + EXPECT_TRUE(converter.to_offset({.line = 0, .character = 3}).has_value()); + EXPECT_TRUE(converter.to_offset({.line = 1, .character = 3}).has_value()); + } +} + }; // TEST_SUITE(language_position) } // namespace eventide::ipc::lsp