From 179d839c61104e599fa3e998d924871703b4de87 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Fri, 28 Mar 2014 14:11:37 +0000 Subject: [PATCH 1/9] XMLOutputArchive: Do not take c_str from a temporary string --- include/cereal/archives/xml.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/cereal/archives/xml.hpp b/include/cereal/archives/xml.hpp index 3b1440c1d..8078ba224 100644 --- a/include/cereal/archives/xml.hpp +++ b/include/cereal/archives/xml.hpp @@ -227,7 +227,8 @@ namespace cereal itsOS << value << std::ends; // allocate strings for all of the data in the XML object - auto dataPtr = itsXML.allocate_string( itsOS.str().c_str() ); + const std::string s = itsOS.str(); + auto dataPtr = itsXML.allocate_string( s.c_str() ); // insert into the XML itsNodes.top().node->append_node( itsXML.allocate_node( rapidxml::node_data, nullptr, dataPtr ) ); From 5ef2e50ef3935b7ecd973f7e88bb03b5d910868a Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 12:48:27 +0100 Subject: [PATCH 2/9] Made XMLOutputArchive::NodeInfo::name a std::string --- include/cereal/archives/xml.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/cereal/archives/xml.hpp b/include/cereal/archives/xml.hpp index 8078ba224..f73834bee 100644 --- a/include/cereal/archives/xml.hpp +++ b/include/cereal/archives/xml.hpp @@ -278,12 +278,12 @@ namespace cereal const char * nm = nullptr ) : node( n ), counter( 0 ), - name( nm ) + name( nm ? std::string(nm) : std::string() ) { } rapidxml::xml_node<> * node; //!< A pointer to this node size_t counter; //!< The counter for naming child nodes - const char * name; //!< The name for the next child node + std::string name; //!< The name for the next child node //! Gets the name for the next child node created from this node /*! The name will be automatically generated using the counter if @@ -291,10 +291,10 @@ namespace cereal set, that name will be returned only once */ std::string getValueName() { - if( name ) + if( !name.empty() ) { auto n = name; - name = nullptr; + name = std::string(); return {n}; } else From a5d52c7b19e6a15947e8a3a20ee77e805001c64a Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 12:59:57 +0100 Subject: [PATCH 3/9] Made XMLInputArchive::NodeInfo::name a std::string --- include/cereal/archives/xml.hpp | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/include/cereal/archives/xml.hpp b/include/cereal/archives/xml.hpp index f73834bee..08b2ece97 100644 --- a/include/cereal/archives/xml.hpp +++ b/include/cereal/archives/xml.hpp @@ -433,12 +433,12 @@ namespace cereal void startNode() { auto next = itsNodes.top().child; // By default we would move to the next child node - auto const expectedName = itsNodes.top().name; // this is the expected name from the NVP, if provided + const std::string &expectedName = itsNodes.top().name; // this is the expected name from the NVP, if provided // If we were given an NVP name, look for it in the current level of the document. // We only need to do this if either we have exhausted the siblings of the current level or // the NVP name does not match the name of the node we would normally read next - if( expectedName && ( next == nullptr || std::strcmp( next->name(), expectedName ) != 0 ) ) + if( !expectedName.empty() && ( next == nullptr || expectedName != next->name() ) ) { next = itsNodes.top().search( expectedName ); @@ -459,13 +459,13 @@ namespace cereal itsNodes.top().advance(); // Reset name - itsNodes.top().name = nullptr; + itsNodes.top().name.clear(); } //! Sets the name for the next node created with startNode void setNextName( const char * name ) { - itsNodes.top().name = name; + itsNodes.top().name = name ? std::string(name) : std::string(); } //! Loads a bool from the current top node @@ -611,7 +611,7 @@ namespace cereal node( n ), child( n->first_node() ), size( XMLInputArchive::getNumChildren( n ) ), - name( nullptr ) + name() { } //! Advances to the next sibling node of the child @@ -626,18 +626,19 @@ namespace cereal } //! Searches for a child with the given name in this node - /*! @param searchName The name to search for (must be null terminated) + /*! @param searchName The name to search for @return The node if found, nullptr otherwise */ - rapidxml::xml_node<> * search( const char * searchName ) + rapidxml::xml_node<> * search( const std::string &searchName ) { - if( searchName ) + if( !searchName.empty() ) { size_t new_size = XMLInputArchive::getNumChildren( node ); - const size_t name_size = rapidxml::internal::measure( searchName ); + const size_t name_size = rapidxml::internal::measure( searchName.c_str() ); for( auto new_child = node->first_node(); new_child != nullptr; new_child = new_child->next_sibling() ) { - if( rapidxml::internal::compare( new_child->name(), new_child->name_size(), searchName, name_size, true ) ) + if( rapidxml::internal::compare( new_child->name(), new_child->name_size(), + searchName.c_str(), name_size, true ) ) { size = new_size; child = new_child; @@ -654,7 +655,7 @@ namespace cereal rapidxml::xml_node<> * node; //!< A pointer to this node rapidxml::xml_node<> * child; //!< A pointer to its current child size_t size; //!< The remaining number of children for this node - const char * name; //!< The NVP name for next next child node + std::string name; //!< The NVP name for next next child node }; // NodeInfo //! @} From 3000a17b0be3cd5a1ae863cf590cd3641cf4b814 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 13:11:26 +0100 Subject: [PATCH 4/9] Made JSONOutputArchive::itsNextName a std::string --- include/cereal/archives/json.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/cereal/archives/json.hpp b/include/cereal/archives/json.hpp index d59c1857c..fb0e476df 100644 --- a/include/cereal/archives/json.hpp +++ b/include/cereal/archives/json.hpp @@ -143,7 +143,7 @@ namespace cereal OutputArchive(this), itsWriteStream(stream), itsWriter(itsWriteStream, options.itsPrecision), - itsNextName(nullptr) + itsNextName() { itsWriter.SetIndent( options.itsIndentChar, options.itsIndentLength ); itsNameCounter.push(0); @@ -215,7 +215,7 @@ namespace cereal //! Sets the name for the next node created with startNode void setNextName( const char * name ) { - itsNextName = name; + itsNextName = name ? std::string(name) : std::string(); } //! Saves a bool to the current node @@ -327,7 +327,7 @@ namespace cereal // Array types do not output names if(nodeType == NodeType::InArray) return; - if(itsNextName == nullptr) + if(itsNextName.empty()) { std::string name = "value" + std::to_string( itsNameCounter.top()++ ) + "\0"; saveValue(name); @@ -335,7 +335,7 @@ namespace cereal else { saveValue(itsNextName); - itsNextName = nullptr; + itsNextName.clear(); } } @@ -350,7 +350,7 @@ namespace cereal private: WriteStream itsWriteStream; //!< Rapidjson write stream JSONWriter itsWriter; //!< Rapidjson writer - char const * itsNextName; //!< The next name + std::string itsNextName; //!< The next name std::stack itsNameCounter; //!< Counter for creating unique names for unnamed nodes std::stack itsNodeStack; }; // JSONOutputArchive From b7f8480beb46ecc303851a8aeea0d7888c09a772 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 13:16:08 +0100 Subject: [PATCH 5/9] Made JSONInputArchive::itsNextName a std::string --- include/cereal/archives/json.hpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/include/cereal/archives/json.hpp b/include/cereal/archives/json.hpp index fb0e476df..8614a0ba8 100644 --- a/include/cereal/archives/json.hpp +++ b/include/cereal/archives/json.hpp @@ -408,7 +408,7 @@ namespace cereal /*! @param stream The stream to read from */ JSONInputArchive(std::istream & stream) : InputArchive(this), - itsNextName( nullptr ), + itsNextName(), itsReadStream(stream) { itsDocument.ParseStream<0>(itsReadStream); @@ -423,7 +423,7 @@ namespace cereal to loading in/out of order */ void loadBinaryValue( void * data, size_t size, const char * name = nullptr ) { - itsNextName = name; + itsNextName = name ? std::string(name) : std::string(); std::string encoded; loadValue( encoded ); @@ -433,7 +433,7 @@ namespace cereal throw Exception("Decoded binary data size does not match specified size"); std::memcpy( data, decoded.data(), decoded.size() ); - itsNextName = nullptr; + itsNextName.clear(); }; private: @@ -488,12 +488,11 @@ namespace cereal //! Adjust our position such that we are at the node with the given name /*! @throws Exception if no such named node exists */ - inline void search( const char * searchName ) + inline void search( const std::string &searchName ) { - const auto len = std::strlen( searchName ); size_t index = 0; for( auto it = itsMemberItBegin; it != itsMemberItEnd; ++it, ++index ) - if( std::strncmp( searchName, it->name.GetString(), len ) == 0 ) + if( searchName == it->name.GetString() ) { itsIndex = index; return; @@ -521,17 +520,17 @@ namespace cereal inline void search() { // The name an NVP provided with setNextName() - if( itsNextName ) + if( !itsNextName.empty() ) { // The actual name of the current node auto const actualName = itsIteratorStack.back().name(); // Do a search if we don't see a name coming up, or if the names don't match - if( !actualName || std::strcmp( itsNextName, actualName ) != 0 ) + if( !actualName || itsNextName != actualName ) itsIteratorStack.back().search( itsNextName ); } - itsNextName = nullptr; + itsNextName.clear(); } public: @@ -565,7 +564,7 @@ namespace cereal //! Sets the name for the next node created with startNode void setNextName( const char * name ) { - itsNextName = name; + itsNextName = name ? std::string(name) : std::string(); } //! Loads a value from the current node - small signed overload @@ -637,7 +636,7 @@ namespace cereal //! @} private: - const char * itsNextName; //!< Next name set by NVP + std::string itsNextName; //!< Next name set by NVP ReadStream itsReadStream; //!< Rapidjson write stream std::vector itsIteratorStack; //!< 'Stack' of rapidJSON iterators rapidjson::Document itsDocument; //!< Rapidjson document From c8b5ee53dc6ba6e01dd8e1ccda283286a64c9530 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 15:04:17 +0100 Subject: [PATCH 6/9] XMLInputArchive: Removed commented code --- include/cereal/archives/xml.hpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/include/cereal/archives/xml.hpp b/include/cereal/archives/xml.hpp index 08b2ece97..031014397 100644 --- a/include/cereal/archives/xml.hpp +++ b/include/cereal/archives/xml.hpp @@ -372,13 +372,6 @@ namespace cereal } catch( rapidxml::parse_error const & ) { - //std::cerr << "-----Original-----" << std::endl; - //stream.seekg(0); - //std::cout << std::string( std::istreambuf_iterator( stream ), std::istreambuf_iterator() ) << std::endl; - - //std::cerr << "-----Error-----" << std::endl; - //std::cerr << e.what() << std::endl; - //std::cerr << e.where() << std::endl; throw Exception("XML Parsing failed - likely due to invalid characters or invalid naming"); } From 7853bea8a9982d17422ee9a0a246e907a49ab193 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 15:05:29 +0100 Subject: [PATCH 7/9] JSONInputArchive: Corrected a typo --- include/cereal/archives/json.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/cereal/archives/json.hpp b/include/cereal/archives/json.hpp index 8614a0ba8..55fd14ffe 100644 --- a/include/cereal/archives/json.hpp +++ b/include/cereal/archives/json.hpp @@ -637,7 +637,7 @@ namespace cereal private: std::string itsNextName; //!< Next name set by NVP - ReadStream itsReadStream; //!< Rapidjson write stream + ReadStream itsReadStream; //!< Rapidjson read stream std::vector itsIteratorStack; //!< 'Stack' of rapidJSON iterators rapidjson::Document itsDocument; //!< Rapidjson document }; From 1298bb2404ba2a1381b0b3b0ab5c534bd021082c Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 14:55:42 +0100 Subject: [PATCH 8/9] Added the concatenation unit test This tests the case where multiple archives are concatenated together in a single stream. For example this can happen if multiple archives are being streamed from a pipe. The JSON and XML variants of the test are disabled, because they don't pass the test. --- unittests/concatenation.cpp | 87 +++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 unittests/concatenation.cpp diff --git a/unittests/concatenation.cpp b/unittests/concatenation.cpp new file mode 100644 index 000000000..5c5bb0496 --- /dev/null +++ b/unittests/concatenation.cpp @@ -0,0 +1,87 @@ +/* + Copyright (c) 2014, Randolph Voorhies, Shane Grant, Joel Holdsworth + All rights reserved. + + Redistribution and use in source and binary forms, with or without + modification, are permitted provided that the following conditions are met: + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in the + documentation and/or other materials provided with the distribution. + * Neither the name of cereal nor the + names of its contributors may be used to endorse or promote products + derived from this software without specific prior written permission. + + THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + DISCLAIMED. IN NO EVENT SHALL RANDOLPH VOORHIES AND SHANE GRANT BE LIABLE FOR ANY + DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ +#include "common.hpp" +#include + +template +void test_concatenation() +{ + // Strings with newlines are being used to check that input archives to not + // treat them incorrectly as document terminators + std::basic_string o_string = "Apples\n\nPears"; + std::basic_string o_string2 = "Oranges\n\nLemons"; + + // Create a stream containing to concatenated archives + std::ostringstream os; + { + OArchive oar(os); + oar(o_string); + } + { + OArchive oar(os); + oar(o_string2); + } + + std::basic_string i_string; + std::basic_string i_string2; + + // Now try and read them back + std::istringstream is(os.str()); + { + IArchive iar(is); + iar(i_string); + } + { + IArchive iar(is); + iar(i_string2); + } + + BOOST_CHECK_EQUAL(i_string, o_string); + BOOST_CHECK_EQUAL(i_string2, o_string2); +} + +BOOST_AUTO_TEST_CASE( binary_string ) +{ + test_concatenation(); +} + +BOOST_AUTO_TEST_CASE( portable_binary_string ) +{ + test_concatenation(); +} + +#if 0 +BOOST_AUTO_TEST_CASE( xml_string_basic ) +{ + test_concatenation(); +} + +BOOST_AUTO_TEST_CASE( json_string_basic ) +{ + test_concatenation(); +} +#endif From 0c7d5adff561e1b06575e8a1d34449ff09564e51 Mon Sep 17 00:00:00 2001 From: Joel Holdsworth Date: Mon, 31 Mar 2014 16:01:45 +0100 Subject: [PATCH 9/9] XMLInputArchive: Support concatenated documents --- include/cereal/archives/xml.hpp | 40 ++++++++++++++++++++++++++------- unittests/concatenation.cpp | 2 +- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/include/cereal/archives/xml.hpp b/include/cereal/archives/xml.hpp index 031014397..b1e40e5ca 100644 --- a/include/cereal/archives/xml.hpp +++ b/include/cereal/archives/xml.hpp @@ -362,18 +362,42 @@ namespace cereal @param stream The stream to read from. Can be a stringstream or a file. */ XMLInputArchive( std::istream & stream ) : - InputArchive( this ), - itsData( std::istreambuf_iterator( stream ), std::istreambuf_iterator() ) + InputArchive( this ) { - try + bool valid = false; + std::string line, document; + + while (!valid && stream) { - itsData.push_back('\0'); // rapidxml will do terrible things without the data being null terminated - itsXML.parse( reinterpret_cast( itsData.data() ) ); + // Read until we get an empty line - this may indicate that the document has ended + while (getline(stream, line)) + { + document += line + '\n'; + if (line.empty()) + break; + } + + // Set this as the document data + // Make a fresh copy every time, because RapidXml parsing is destructive + itsData.clear(); + itsData.reserve(document.length() + 1); + itsData.insert(itsData.begin(), document.cbegin(), document.cend()); + itsData.push_back('\0'); + + // If the line is empty, this may indicate the document has ended, if so try and parse it + try + { + itsXML.parse( + reinterpret_cast(itsData.data()) ); + valid = true; + } + catch( rapidxml::parse_error const & ) + { + } } - catch( rapidxml::parse_error const & ) - { + + if (!valid) throw Exception("XML Parsing failed - likely due to invalid characters or invalid naming"); - } // Parse the root auto root = itsXML.first_node( xml_detail::CEREAL_XML_STRING ); diff --git a/unittests/concatenation.cpp b/unittests/concatenation.cpp index 5c5bb0496..40fd86111 100644 --- a/unittests/concatenation.cpp +++ b/unittests/concatenation.cpp @@ -74,12 +74,12 @@ BOOST_AUTO_TEST_CASE( portable_binary_string ) test_concatenation(); } -#if 0 BOOST_AUTO_TEST_CASE( xml_string_basic ) { test_concatenation(); } +#if 0 BOOST_AUTO_TEST_CASE( json_string_basic ) { test_concatenation();