From 0da51b151c3ff219620d59add6f2c5c7c0e3dcc4 Mon Sep 17 00:00:00 2001 From: Gennaro Prota Date: Thu, 30 Jul 2026 12:22:13 +0200 Subject: [PATCH 1/2] Preserve XML escape iterator state across a copy `xml_escape` and `xml_unescape` had a hand-written copy constructor, added long ago as an Intel 7.1 workaround, that reconstructed the `escape<>` / `unescape<>` base from `rhs.base_reference()` alone. That runs the base's one-argument constructor and resets its in-progress state (`m_full`, `m_bnext`, `m_bend`, `m_current_value`), so a copy taken in the middle of an escape sequence forgot where it was and restarted the sequence. This removes the hand-written copy constructors and use the compiler-generated ones, which copy the full base state. Intel 7.1 (from 2003) is long gone from the supported toolchains. `xml_unescape` had the identical defect on the read side and is fixed the same way. Closes #229. --- .../boost/archive/iterators/xml_escape.hpp | 4 -- .../boost/archive/iterators/xml_unescape.hpp | 4 -- test/Jamfile.v2 | 1 + test/test_xml_escape_boundary.cpp | 57 +++++++++++++++++++ 4 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 test/test_xml_escape_boundary.cpp diff --git a/include/boost/archive/iterators/xml_escape.hpp b/include/boost/archive/iterators/xml_escape.hpp index d58e8d6c6..83465fd44 100644 --- a/include/boost/archive/iterators/xml_escape.hpp +++ b/include/boost/archive/iterators/xml_escape.hpp @@ -42,10 +42,6 @@ class xml_escape xml_escape(T start) : super_t(Base(static_cast< T >(start))) {} - // intel 7.1 doesn't like default copy constructor - xml_escape(const xml_escape & rhs) : - super_t(rhs.base_reference()) - {} }; template diff --git a/include/boost/archive/iterators/xml_unescape.hpp b/include/boost/archive/iterators/xml_unescape.hpp index 26244ade6..6573053cc 100644 --- a/include/boost/archive/iterators/xml_unescape.hpp +++ b/include/boost/archive/iterators/xml_unescape.hpp @@ -58,10 +58,6 @@ class xml_unescape xml_unescape(T start) : super_t(Base(static_cast< T >(start))) {} - // intel 7.1 doesn't like default copy constructor - xml_unescape(const xml_unescape & rhs) : - super_t(rhs.base_reference()) - {} }; template diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 1abddd73b..d7f4ad747 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -155,6 +155,7 @@ if ! $(BOOST_ARCHIVE_LIST) { [ test-bsl-run test_reset_object_address : A ] [ test-bsl-run test_void_cast ] [ test-bsl-run test_xml_save_during_unwind ] + [ test-bsl-run test_xml_escape_boundary : : ../build//boost_wserialization : [ requires std_wstreambuf ] ] [ test-bsl-run test_xml_trailing_whitespace ] [ test-bsl-run test_xml_missing_nvp ] [ test-bsl-run test_mult_archive_types : : : [ requires std_wstreambuf ] ] diff --git a/test/test_xml_escape_boundary.cpp b/test/test_xml_escape_boundary.cpp new file mode 100644 index 000000000..ae066e7ac --- /dev/null +++ b/test/test_xml_escape_boundary.cpp @@ -0,0 +1,57 @@ +/////////1/////////2/////////3/////////4/////////5/////////6/////////7/////////8 +// test_xml_escape_boundary.cpp + +// Copyright 2026 Gennaro Prota +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// Regression test for issue #229. Writing a std::string that contains a +// character needing an XML escape (here '"' -> ") whose expansion +// straddles the 32 character internal buffer of wchar_from_mb used to +// produce malformed output such as "&qu"": the xml_escape iterator was +// copied in mid escape sequence and its hand-written copy constructor dropped +// the escape<> base's state, restarting the sequence. Check that a wide XML +// archive escapes such a string correctly and round-trips it. + +#include +#include + +#include +#include +#include +#include + +#include "test_tools.hpp" + +int test_main(int /* argc */, char * /* argv */ []){ + // The '"' sits at index 29, so its " expansion crosses the 32 char + // wchar_from_mb buffer boundary. + const std::string instring("01234567890123456789012345678\"-here-is-the-error"); + + std::wstring archived; + { + std::wostringstream os; + { + boost::archive::xml_woarchive oa(os); + oa << boost::serialization::make_nvp("err", instring); + } + archived = os.str(); + } + + // The escape must appear exactly once and intact, never as the truncated + // "&qu"" the bug produced. + BOOST_CHECK(archived.find(L"&qu"") == std::wstring::npos); + BOOST_CHECK(archived.find(L""") != std::wstring::npos); + + // And it must round-trip. + std::string result; + { + std::wistringstream is(archived); + boost::archive::xml_wiarchive ia(is); + ia >> boost::serialization::make_nvp("err", result); + } + BOOST_CHECK(instring == result); + + return EXIT_SUCCESS; +} From be367b734bd7529e175707bd9294e1e64d7112d8 Mon Sep 17 00:00:00 2001 From: Gennaro Prota Date: Thu, 30 Jul 2026 15:58:42 +0200 Subject: [PATCH 2/2] Preserve transform_width and remove_whitespace state across a copy Same defect class as the `xml_escape` / `xml_unescape` fix in the previous commit: a hand-written copy constructor, written for Intel 7.1, that silently dropped part of the iterator's state. The bug is not reachable through the archive save/load paths, which drive the base64 pipeline with a single iterator instance, but these iterators are public and meant to be composed, and `std::copy` and buffered adaptors (as in issue #229) copy an iterator in mid stream. Remove both hand-written copy constructors and use the compiler- generated ones, which copy the full state. Refs #229. --- .../archive/iterators/remove_whitespace.hpp | 4 - .../archive/iterators/transform_width.hpp | 9 -- test/Jamfile.v2 | 1 + test/test_iterators_copy.cpp | 107 ++++++++++++++++++ 4 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 test/test_iterators_copy.cpp diff --git a/include/boost/archive/iterators/remove_whitespace.hpp b/include/boost/archive/iterators/remove_whitespace.hpp index 11f80888b..a89c33314 100644 --- a/include/boost/archive/iterators/remove_whitespace.hpp +++ b/include/boost/archive/iterators/remove_whitespace.hpp @@ -154,10 +154,6 @@ class remove_whitespace : remove_whitespace(T start) : super_t(Base(static_cast< T >(start))) {} - // intel 7.1 doesn't like default copy constructor - remove_whitespace(const remove_whitespace & rhs) : - super_t(rhs.base_reference()) - {} }; } // namespace iterators diff --git a/include/boost/archive/iterators/transform_width.hpp b/include/boost/archive/iterators/transform_width.hpp index fdff5c0c4..bd64f78d9 100644 --- a/include/boost/archive/iterators/transform_width.hpp +++ b/include/boost/archive/iterators/transform_width.hpp @@ -119,15 +119,6 @@ class transform_width : m_remaining_bits(0), m_end_of_sequence(false) {} - // intel 7.1 doesn't like default copy constructor - transform_width(const transform_width & rhs) : - super_t(rhs.base_reference()), - m_buffer_out_full(rhs.m_buffer_out_full), - m_buffer_out(rhs.m_buffer_out), - m_buffer_in(rhs.m_buffer_in), - m_remaining_bits(rhs.m_remaining_bits), - m_end_of_sequence(false) - {} }; template< diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index d7f4ad747..ee765893c 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -161,6 +161,7 @@ if ! $(BOOST_ARCHIVE_LIST) { [ test-bsl-run test_mult_archive_types : : : [ requires std_wstreambuf ] ] [ test-bsl-run test_iterators : : : [ requires std_wstreambuf ] ] [ test-bsl-run test_iterators_base64 ] + [ test-bsl-run test_iterators_copy ] [ test-bsl-run test_smart_cast ] [ test-bsl-run test_codecvt_null ] [ test-bsl-run test_singleton ] diff --git a/test/test_iterators_copy.cpp b/test/test_iterators_copy.cpp new file mode 100644 index 000000000..39b792ea4 --- /dev/null +++ b/test/test_iterators_copy.cpp @@ -0,0 +1,107 @@ +/////////1/////////2/////////3/////////4/////////5/////////6/////////7/////////8 +// test_iterators_copy.cpp + +// Copyright 2026 Gennaro Prota +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt or copy at +// https://www.boost.org/LICENSE_1_0.txt) + +// Regression test for issue #229. + +#include +#include +#include +#include + +#include + +#include +#include +#include +#include +#include + +#include "test_tools.hpp" + +// Traverse [it, end) but copy construct the iterator at every step and +// continue from the copy, the way std::copy unwraps an iterator. The `== end` +// comparison is what makes transform_width set its end-of-sequence flag, so we +// must keep comparing against end for the copy to have that state to preserve. +template +void drain_to_end_by_copy(Iterator it, Iterator end, Output out){ + while(! (it == end)){ + Iterator cur = it; // copy constructor under test + *out++ = *cur; + ++cur; + it = cur; + } +} + +// Read n elements, copy constructing the iterator at every step. Used for the +// decode pipeline, which has no end-of-sequence padding but does drive +// remove_whitespace (whose copy must preserve its cached-value flag). +template +void drain_n_by_copy(Iterator it, std::size_t n, Output out){ + for(std::size_t i = 0; i < n; ++i){ + Iterator cur = it; // copy constructor under test + *out++ = *cur; + ++cur; + it = cur; + } +} + +template +void test_base64_copy(unsigned int size){ + CharType rawdata[150]; + for(unsigned int i = 0; i < size; ++i) + rawdata[i] = static_cast(std::rand() & 0xff); + + typedef boost::archive::iterators::insert_linebreaks< + boost::archive::iterators::base64_from_binary< + boost::archive::iterators::transform_width< + CharType *, 6, sizeof(CharType) * 8 + > + >, 76 + > encode; + + // Straight encode (single iterator instance, as std::copy drives it). + std::list plain; + std::copy( + encode(rawdata), encode(rawdata + size), std::back_inserter(plain) + ); + + // Same encode, but copy constructing the iterator at every step. Without + // a correct transform_width copy constructor the final (zero padded) + // group is produced from lost state, so the tails differ. + std::list copied; + drain_to_end_by_copy( + encode(rawdata), encode(rawdata + size), std::back_inserter(copied) + ); + BOOST_CHECK(plain == copied); + + // Decode back to the original bytes, again copy constructing at every + // step. This drives remove_whitespace over the line breaks inserted + // above (present once the base64 exceeds 76 characters). + typedef boost::archive::iterators::transform_width< + boost::archive::iterators::binary_from_base64< + boost::archive::iterators::remove_whitespace< + typename std::list::iterator + > + >, sizeof(CharType) * 8, 6 + > decode; + std::list decoded; + drain_n_by_copy(decode(plain.begin()), size, std::back_inserter(decoded)); + BOOST_CHECK(std::equal(rawdata, rawdata + size, decoded.begin())); +} + +int test_main(int /* argc */, char * /* argv */ []){ + for(unsigned int s = 1; s <= 4; ++s) + test_base64_copy(s); + test_base64_copy(150); + #ifndef BOOST_NO_CWCHAR + for(unsigned int s = 1; s <= 4; ++s) + test_base64_copy(s); + test_base64_copy(150); + #endif + return EXIT_SUCCESS; +}