From 2666e37955b8bdf05afe114a8f8c72ac5789bf65 Mon Sep 17 00:00:00 2001 From: Mungo Gill Date: Wed, 19 Nov 2025 13:17:13 +0000 Subject: [PATCH] fix iterator invalidation issue by storing indices not iterators --- include/boost/buffers/slice.hpp | 58 ++++++++++++---- test/unit/CMakeLists.txt | 4 +- test/unit/slice.cpp | 8 ++- test/unit/test_buffers.hpp | 113 ++++++++++++++++++++++++++++++-- 4 files changed, 163 insertions(+), 20 deletions(-) diff --git a/include/boost/buffers/slice.hpp b/include/boost/buffers/slice.hpp index fe091abe..8a06c7e3 100644 --- a/include/boost/buffers/slice.hpp +++ b/include/boost/buffers/slice.hpp @@ -64,9 +64,12 @@ class slice_of using iter_type = decltype( std::declval().begin()); + using difference_type = + typename std::iterator_traits::difference_type; + BufferSequence bs_; - iter_type begin_; - iter_type end_; + difference_type begin_ = 0; // index of first buffer in sequence + difference_type end_ = 0; // 1 + index of last buffer in sequence std::size_t len_ = 0; // length of bs_ std::size_t size_ = 0; // total bytes std::size_t prefix_ = 0; // used prefix bytes @@ -92,11 +95,12 @@ class slice_of slice_of( BufferSequence const& bs) : bs_(bs) - , begin_(buffers::begin(bs_)) - , end_(buffers::end(bs_)) { - auto it = begin_; - while(it != end_) + iter_type it = buffers::begin(bs_); + iter_type eit = buffers::end(bs_); + begin_ = 0; + end_ = std::distance(it, eit); + while(it != eit) { value_type b(*it); size_ += b.size(); @@ -127,18 +131,39 @@ class slice_of } private: + iter_type + begin_iter_impl() const noexcept + { + iter_type it = buffers::begin(bs_); + std::advance(it, begin_); + return it; + } + + iter_type + end_iter_impl() const noexcept + { + iter_type it = buffers::begin(bs_); + std::advance(it, end_); + return it; + } + void remove_prefix_impl( std::size_t n) { + if(n > size_) + n = size_; + // nice hack to simplify the loop (M. Nejati) n += prefix_; size_ += prefix_; prefix_ = 0; + iter_type it = begin_iter_impl(); + while(n > 0 && begin_ != end_) { - value_type b = *begin_; + value_type b = *it; if(n < b.size()) { prefix_ = n; @@ -148,6 +173,7 @@ class slice_of n -= b.size(); size_ -= b.size(); ++begin_; + ++it; --len_; } } @@ -162,12 +188,19 @@ class slice_of return; } BOOST_ASSERT(begin_ != end_); + + if(n > size_) + n = size_; + n += suffix_; size_ += suffix_; suffix_ = 0; - iter_type it = end_; - --it; - while(it != begin_) + + iter_type bit = begin_iter_impl(); + iter_type it = end_iter_impl(); + it--; + + while(it != bit) { value_type b = *it; if(n < b.size()) @@ -192,6 +225,7 @@ class slice_of } end_ = begin_; len_ = 0; + size_ = 0; } void @@ -376,7 +410,7 @@ begin() const noexcept -> const_iterator { return const_iterator( - this->begin_, prefix_, suffix_, 0, len_); + begin_iter_impl(), prefix_, suffix_, 0, len_); } template @@ -386,7 +420,7 @@ end() const noexcept -> const_iterator { return const_iterator( - this->end_, prefix_, suffix_, len_, len_); + end_iter_impl(), prefix_, suffix_, len_, len_); } //------------------------------------------------ diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt index 8f9ae532..3795d92a 100644 --- a/test/unit/CMakeLists.txt +++ b/test/unit/CMakeLists.txt @@ -7,7 +7,9 @@ # Official repository: https://github.com/cppalliance/buffers # -add_subdirectory(../../../url/extra/test_suite test_suite) +if (NOT TARGET boost_url_test_suite) + add_subdirectory(../../../url/extra/test_suite test_suite) +endif() file(GLOB_RECURSE PFILES CONFIGURE_DEPENDS *.cpp *.hpp) list(APPEND PFILES diff --git a/test/unit/slice.cpp b/test/unit/slice.cpp index 72c31ed5..0bc31e2d 100644 --- a/test/unit/slice.cpp +++ b/test/unit/slice.cpp @@ -17,6 +17,7 @@ #include #include +#include #include "test_buffers.hpp" #include "test_suite.hpp" @@ -119,7 +120,8 @@ struct slice_test test::check_iterators(b, s); } - using seq_type = std::array; + // Use a vector so that iterator invalidation is observable during testing. + using seq_type = std::vector; void grind_back( @@ -176,8 +178,8 @@ struct slice_test run() { std::string s; - seq_type bs = make_buffers(s, - "boost.", "buffers.", "slice_" ); + auto a = make_buffers(s, "boost.", "buffers.", "slice_"); + seq_type bs(a.begin(), a.end()); test::check_sequence(bs, s); //check(bs, s); //grind(bs, s); diff --git a/test/unit/test_buffers.hpp b/test/unit/test_buffers.hpp index 63adff9f..5e23e572 100644 --- a/test/unit/test_buffers.hpp +++ b/test/unit/test_buffers.hpp @@ -17,6 +17,7 @@ #include #include #include + #include // Trick boostdep into requiring URL @@ -255,7 +256,8 @@ template void grind_front( ConstBufferSequence const& bs0, - core::string_view pat0) + core::string_view pat0, + int levels) { for(std::size_t n = 0; n <= pat0.size() + 1; ++n) { @@ -265,6 +267,18 @@ grind_front( remove_prefix(bs, n); check_eq(bs, pat); check_iterators(bs, pat); + + if(levels) + { + // Take a copy, blank out the original to invalidate any + // iterators, and redo the test + slice_type bsc(bs); + { + slice_type dummy{}; + std::swap(bs, dummy); + } + grind_front(bsc, pat, levels-1); + } } { auto pat = kept_front(pat0, n); @@ -272,6 +286,18 @@ grind_front( keep_prefix(bs, n); check_eq(bs, pat); check_iterators(bs, pat); + + if(levels) + { + // Take a copy, blank out the original to invalidate any + // iterators, and redo the test + slice_type bsc(bs); + { + slice_type dummy{}; + std::swap(bs, dummy); + } + grind_front(bsc, pat, levels-1); + } } } } @@ -280,7 +306,8 @@ template void grind_back( ConstBufferSequence const& bs0, - core::string_view pat0) + core::string_view pat0, + int levels) { for(std::size_t n = 0; n <= pat0.size() + 1; ++n) { @@ -290,6 +317,18 @@ grind_back( remove_suffix(bs, n); check_eq(bs, pat); check_iterators(bs, pat); + + if(levels) + { + // Take a copy, blank out the original to invalidate any + // iterators, and redo the test + slice_type bsc(bs); + { + slice_type dummy{}; + std::swap(bs, dummy); + } + grind_back(bsc, pat, levels-1); + } } { auto pat = kept_back(pat0, n); @@ -297,6 +336,70 @@ grind_back( keep_suffix(bs, n); check_eq(bs, pat); check_iterators(bs, pat); + + if(levels) + { + // Take a copy, blank out the original to invalidate any + // iterators, and redo the test + slice_type bsc(bs); + { + slice_type dummy{}; + std::swap(bs, dummy); + } + grind_back(bsc, pat, levels-1); + } + } + } +} + +template +void +grind_both( + ConstBufferSequence const& bs0, + core::string_view pat0, + int levels) +{ + for(std::size_t n = 0; n <= pat0.size() / 2 + 2; ++n) + { + { + auto pat = trimmed_back(trimmed_front(pat0, n), n); + slice_type bs(bs0); + remove_prefix(bs, n); + remove_suffix(bs, n); + check_eq(bs, pat); + check_iterators(bs, pat); + + if(levels) + { + // Take a copy, blank out the original to invalidate any + // iterators, and redo the test + slice_type bsc(bs); + { + slice_type dummy{}; + std::swap(bs, dummy); + } + grind_both(bsc, pat, levels - 1); + } + } + { + auto pat = kept_back(kept_front(pat0, n), n); + slice_type bs(bs0); + keep_prefix(bs, n); + keep_suffix(bs, n); + check_eq(bs, pat); + check_iterators(bs, pat); + + if(levels) + { + // Take a copy, blank out the original to invalidate any + // iterators, and redo the test + slice_type bsc(bs); + { + slice_type dummy{}; + std::swap(bs, dummy); + } + grind_both(bsc, pat, levels - 1); + } } } } @@ -307,8 +410,9 @@ check_slice( ConstBufferSequence const& bs, core::string_view pat) { - grind_front(bs, pat); - grind_back(bs, pat); + grind_front(bs, pat, 1); + grind_back(bs, pat, 1); + grind_both(bs, pat, 1); } // Test API and behavior of a BufferSequence @@ -320,6 +424,7 @@ check_sequence( BOOST_STATIC_ASSERT(is_const_buffer_sequence::value); check_iterators(t, pat); + check_slice(t, pat); }