diff --git a/doc/dataflow.html b/doc/dataflow.html index 4ae3b3010..de1912d07 100644 --- a/doc/dataflow.html +++ b/doc/dataflow.html @@ -126,10 +126,29 @@

Dereferencing

Comparison

The default implementation of iterator equality of iterator_adaptor just invokes the equality operator on the base iterators. Generally this is satisfactory. -However, this implies that other operations (E. G. dereference) do not prematurely +However, this implies that other operations (E. G. dereference) do not prematurely increment the base iterator. Avoiding this can be surprisingly tricky in some cases. (E.G. transform_width) +

+An iterator whose output elements do not line up with its input ones cannot +always avoid it. transform_width +learns that the input is over only by reading past the end of it, which is +undefined behaviour, and it does so whenever the length of the input is not a +common multiple of the two widths. Decoding base64, any length of the form +4n + 1 ends in six bits which do not make up a byte, and those six bits send +the iterator looking for the two it is missing. + +

+Passing the end of the input as a second constructor argument avoids this. +The iterator then stops there, discarding whatever is left over rather than +reading on for it. Both +transform_width and +remove_whitespace accept it. The one +argument form behaves as it always has, and remains the one to use for a +composed sequence whose length divides evenly, which is how the archives +themselves use these iterators. +

Iterators which fulfill the above requirements should be composable and the above sample code should implement our binary to base64 conversion. diff --git a/include/boost/archive/iterators/transform_width.hpp b/include/boost/archive/iterators/transform_width.hpp index bd64f78d9..5289071ec 100644 --- a/include/boost/archive/iterators/transform_width.hpp +++ b/include/boost/archive/iterators/transform_width.hpp @@ -72,8 +72,19 @@ class transform_width : } bool equal_impl(const this_t & rhs){ - if(BitsIn < BitsOut) // discard any left over bits + if(BitsIn < BitsOut){ // discard any left over bits + if(m_bounded){ + // Fill here rather than on dereference. The bits left when + // the input runs out part way through an output value do not + // make a whole one, so the sequence has to end before that + // value is handed out. + if(! m_buffer_out_full && ! m_exhausted){ + fill(); + } + return m_exhausted; + } return this->base_reference() == rhs.base_reference(); + } else{ // BitsIn > BitsOut // zero fill if(this->base_reference() == rhs.base_reference()){ @@ -105,6 +116,12 @@ class transform_width : // flag to indicate we've reached end of data. bool m_end_of_sequence; + // end of the input, when one has been given + Base m_end; + bool m_bounded; + // set once the input can yield no further whole output value + bool m_exhausted; + public: // make composable by using templated constructor template @@ -117,7 +134,28 @@ class transform_width : //used because m_remaining_bits == 0) m_buffer_in(0), m_remaining_bits(0), - m_end_of_sequence(false) + m_end_of_sequence(false), + // m_end means nothing unless bounded. It is built from start only + // because Base need not be default constructible: it is usually + // another adaptor of this family, and those have just the templated + // constructor below. + m_end(Base(static_cast< T >(start))), + m_bounded(false), + m_exhausted(false) + {} + // Knowing where the input ends is what lets the iterator stop before + // reading past it, which the form above cannot do. + template + transform_width(T start, T end) : + super_t(Base(static_cast< T >(start))), + m_buffer_out_full(false), + m_buffer_out(0), + m_buffer_in(0), + m_remaining_bits(0), + m_end_of_sequence(false), + m_end(Base(static_cast< T >(end))), + m_bounded(true), + m_exhausted(false) {} }; @@ -136,6 +174,12 @@ void transform_width::fill() { m_buffer_in = 0; m_remaining_bits = missing_bits; } + else if(m_bounded && this->base_reference() == m_end){ + // The input ended part way through an output value. What is + // left cannot make a whole one, so drop it. + m_exhausted = true; + return; + } else{ m_buffer_in = * this->base_reference()++; m_remaining_bits = BitsIn; diff --git a/test/test_iterators_base64.cpp b/test/test_iterators_base64.cpp index 3990bb0c8..7b13e1afe 100644 --- a/test/test_iterators_base64.cpp +++ b/test/test_iterators_base64.cpp @@ -7,7 +7,9 @@ // http://www.boost.org/LICENSE_1_0.txt) #include +#include #include +#include #if (defined _MSC_VER) && (_MSC_VER == 1200) # pragma warning (disable : 4786) // too long name, harmless warning @@ -86,9 +88,38 @@ void test_base64(unsigned int size){ } +// A base64 sequence whose length is not a whole number of four character +// groups carries some bits which do not make up a byte. Decoding must drop +// them and stop, rather than look for the byte's remaining bits beyond the +// end of the input. Reported by ROCKFAL1 in +// https://github.com/boostorg/serialization/issues/324. Thanks! +void test_base64_partial_group(){ + typedef boost::archive::iterators::transform_width< + boost::archive::iterators::binary_from_base64, 8, 6 + > decoder; + + // "1234567890" encoded, less the padding + const std::string encoded("MTIzNDU2Nzg5MA"); + const std::string decoded("1234567890"); + + for(std::size_t n = 0; n <= encoded.size(); ++n){ + const char * const first = encoded.data(); + const char * const last = first + n; + std::string result; + std::copy( + decoder(first, last), + decoder(last), + std::back_inserter(result) + ); + // six bits in, eight bits out, and no partial byte at the end + BOOST_CHECK(result == decoded.substr(0, n * 6 / 8)); + } +} + int test_main( int /*argc*/, char* /*argv*/[] ) { + test_base64_partial_group(); test_base64(1); test_base64(2); test_base64(3);