Merge pull request #58 from jeking3/issue-56

issue-56: fix binary serialization compatibility problem with time_duration

.gitignore

@@ -1 +1,3 @@
-/test/test_facet_file.out
+/test/time_duration_serialization.v0
+/test/time_duration_serialization.v1
+/test/test_facet_file.out

include/boost/date_time/posix_time/time_serialize.hpp

@@ -11,9 +11,10 @@
 
 #include "boost/date_time/posix_time/posix_time.hpp"
 #include "boost/date_time/gregorian/greg_serialize.hpp"
+#include "boost/numeric/conversion/cast.hpp"
 #include "boost/serialization/split_free.hpp"
 #include "boost/serialization/nvp.hpp"
-
+#include "boost/serialization/version.hpp"
 
 // macros to split serialize functions into save & load functions
 // NOTE: these macros define template functions in the boost::serialization namespace.
@@ -22,6 +23,14 @@ BOOST_SERIALIZATION_SPLIT_FREE(boost::posix_time::ptime)
 BOOST_SERIALIZATION_SPLIT_FREE(boost::posix_time::time_duration)
 BOOST_SERIALIZATION_SPLIT_FREE(boost::posix_time::time_period)
 
+// Define versions for serialization compatibility
+// alows the unit tests to make an older version to check compatibility
+#ifndef BOOST_DATE_TIME_POSIX_TIME_DURATION_VERSION
+#define BOOST_DATE_TIME_POSIX_TIME_DURATION_VERSION 1
+#endif
+
+BOOST_CLASS_VERSION(boost::posix_time::time_duration, BOOST_DATE_TIME_POSIX_TIME_DURATION_VERSION)
+
 namespace boost {
 namespace serialization {
 
@@ -33,10 +42,23 @@ namespace serialization {
  * types are hour_type, min_type, sec_type, and fractional_seconds_type
  * as defined in the time_duration class
  */
+template<class TimeResTraitsSize, class Archive>
+void save_td(Archive& ar, const posix_time::time_duration& td)
+{
+    TimeResTraitsSize h = boost::numeric_cast<TimeResTraitsSize>(td.hours());
+    TimeResTraitsSize m = boost::numeric_cast<TimeResTraitsSize>(td.minutes());
+    TimeResTraitsSize s = boost::numeric_cast<TimeResTraitsSize>(td.seconds());
+    posix_time::time_duration::fractional_seconds_type fs = td.fractional_seconds();
+    ar & make_nvp("time_duration_hours", h);
+    ar & make_nvp("time_duration_minutes", m);
+    ar & make_nvp("time_duration_seconds", s);
+    ar & make_nvp("time_duration_fractional_seconds", fs);
+}
+
 template<class Archive>
 void save(Archive & ar, 
           const posix_time::time_duration& td, 
-          unsigned int /*version*/)
+          unsigned int version)
 {
   // serialize a bool so we know how to read this back in later
   bool is_special = td.is_special();
@@ -46,14 +68,13 @@ void save(Archive & ar,
     ar & make_nvp("sv_time_duration", s);
   }
   else {
-    posix_time::time_duration::hour_type h = td.hours();
-    posix_time::time_duration::min_type m = td.minutes();
-    posix_time::time_duration::sec_type s = td.seconds();
-    posix_time::time_duration::fractional_seconds_type fs = td.fractional_seconds();
-    ar & make_nvp("time_duration_hours", h);
-    ar & make_nvp("time_duration_minutes", m);
-    ar & make_nvp("time_duration_seconds", s);
-    ar & make_nvp("time_duration_fractional_seconds", fs);
+    // Write support for earlier versions allows for upgrade compatibility testing
+    // See load comments for version information
+    if (version == 0) {
+        save_td<int32_t>(ar, td);
+    } else {
+        save_td<int64_t>(ar, td);
+    }
   }
 }
 
@@ -62,10 +83,24 @@ void save(Archive & ar,
  * types are hour_type, min_type, sec_type, and fractional_seconds_type
  * as defined in the time_duration class
  */
+template<class TimeResTraitsSize, class Archive>
+void load_td(Archive& ar, posix_time::time_duration& td)
+{
+    TimeResTraitsSize h(0);
+    TimeResTraitsSize m(0);
+    TimeResTraitsSize s(0);
+    posix_time::time_duration::fractional_seconds_type fs(0);
+    ar & make_nvp("time_duration_hours", h);
+    ar & make_nvp("time_duration_minutes", m);
+    ar & make_nvp("time_duration_seconds", s);
+    ar & make_nvp("time_duration_fractional_seconds", fs);
+    td = posix_time::time_duration(h, m, s, fs);
+}
+
 template<class Archive>
 void load(Archive & ar, 
           posix_time::time_duration & td, 
-          unsigned int /*version*/)
+          unsigned int version)
 {
   bool is_special = false;
   ar & make_nvp("is_special", is_special);
@@ -76,15 +111,25 @@ void load(Archive & ar,
     td = posix_time::time_duration(sv);
   }
   else {
-    posix_time::time_duration::hour_type h(0);
-    posix_time::time_duration::min_type m(0);
-    posix_time::time_duration::sec_type s(0);
-    posix_time::time_duration::fractional_seconds_type fs(0);
-    ar & make_nvp("time_duration_hours", h);
-    ar & make_nvp("time_duration_minutes", m);
-    ar & make_nvp("time_duration_seconds", s);
-    ar & make_nvp("time_duration_fractional_seconds", fs);
-    td = posix_time::time_duration(h,m,s,fs);
+    // Version "0"   (Boost 1.65.1 or earlier, which used int32_t for day/hour/minute/second and
+    //                therefore suffered from the year 2038 issue.)
+    // Version "0.5" (Boost 1.66.0 changed to std::time_t but did not increase the version;
+    //                it was missed in the original change, all code reviews, and there were no
+    //                static assertions to protect the code; further std::time_t can be 32-bit
+    //                or 64-bit so it reduced portability.  This makes 1.66.0 hard to handle...)
+    // Version "1"   (Boost 1.67.0 or later uses int64_t and is properly versioned)
+
+    // If the size of any of these items changes, a new version is needed.
+    BOOST_STATIC_ASSERT(sizeof(posix_time::time_duration::hour_type) == sizeof(boost::int64_t));
+    BOOST_STATIC_ASSERT(sizeof(posix_time::time_duration::min_type) == sizeof(boost::int64_t));
+    BOOST_STATIC_ASSERT(sizeof(posix_time::time_duration::sec_type) == sizeof(boost::int64_t));
+    BOOST_STATIC_ASSERT(sizeof(posix_time::time_duration::fractional_seconds_type) == sizeof(boost::int64_t));
+
+    if (version == 0) {
+        load_td<int32_t>(ar, td);
+    } else {
+        load_td<int64_t>(ar, td);
+    }
   }
 }
 

include/boost/date_time/time_resolution_traits.hpp

@@ -60,6 +60,29 @@ namespace date_time {
     static bool is_adapted() { return true;}
   };
 
+  //
+  // Note about var_type, which is used to define the variable that
+  // stores hours, minutes, and seconds values:
+  //
+  // In Boost 1.65.1 and earlier var_type was boost::int32_t which suffers
+  // the year 2038 problem.  Binary serialization of posix_time uses
+  // 32-bit values, and uses serialization version 0.
+  //
+  // In Boost 1.66.0 the var_type changed to std::time_t, however
+  // binary serialization was not properly versioned, so on platforms
+  // where std::time_t is 32-bits, it remains compatible, however on
+  // platforms where std::time_t is 64-bits, binary serialization ingest
+  // will be incompatible with previous versions.  Furthermore, binary
+  // serialized output from 1.66.0 will not be compatible with future
+  // versions.  Yes, it's a mess.  Static assertions were not present
+  // in the serialization code to protect against this possibility.
+  //
+  // In Boost 1.67.0 the var_type was changed to boost::int64_t, 
+  // ensuring the output size is 64 bits, and the serialization version
+  // was bumped.  Static assertions were added as well, protecting
+  // future changes in this area.
+  //
+
   template<typename frac_sec_type,
            time_resolutions res,
 #if (defined(BOOST_MSVC) && (_MSC_VER < 1300))
@@ -68,7 +91,7 @@ namespace date_time {
            typename frac_sec_type::int_type resolution_adjust,
 #endif
            unsigned short frac_digits,
-           typename var_type = std::time_t >
+           typename var_type = boost::int64_t >     // see note above
   class time_resolution_traits {
   public:
     typedef typename frac_sec_type::int_type fractional_seconds_type;

test/Jamfile.v2

@@ -198,6 +198,17 @@ local DATE_TIME_PROPERTIES = <define>BOOST_DATE_TIME_POSIX_TIME_STD_CONFIG <defi
    ../../serialization/build//boost_serialization
    : : : $(DATE_TIME_PROPERTIES)  <define>DATE_TIME_XML_SERIALIZE
    : testtime_serialize_xml ]
+ [ run posix_time/testtime_serialize_versioning.cpp
+   ../build//boost_date_time/<link>static
+   ../../serialization/build//boost_serialization
+   : : : $(DATE_TIME_PROPERTIES)  <define>BOOST_DATE_TIME_POSIX_TIME_DURATION_VERSION=0
+   : testtime_serialize_versioning_prev ]
+ [ run posix_time/testtime_serialize_versioning.cpp
+   ../build//boost_date_time/<link>static
+   ../../serialization/build//boost_serialization
+   : : testtime_serialize_versioning_prev 
+   : $(DATE_TIME_PROPERTIES)
+   : testtime_serialize_versioning_curr ]
 
  # text archive tests
  [ run gregorian/testgreg_serialize.cpp

test/posix_time/testtime_serialize_versioning.cpp

@@ -0,0 +1,56 @@
+/* Copyright (c) 2017 James E. King III
+ * Use, modification and distribution is subject to the
+ * Boost Software License, Version 1.0. (See accompanying
+ * file LICENSE_1_0.txt or http://www.boost.org/LICENSE_1_0.txt)
+ */
+
+#include <boost/archive/binary_oarchive.hpp>
+#include <boost/archive/binary_iarchive.hpp>
+#include <boost/date_time/posix_time/posix_time.hpp>
+#include <boost/date_time/posix_time/time_serialize.hpp>
+#include <boost/lexical_cast.hpp>
+#include "../testfrmwk.hpp"
+#include <fstream>
+
+using namespace boost;
+using namespace posix_time;
+
+void check_filesize(const char* filename, std::ifstream::pos_type expectedSize)
+{
+    std::ifstream in(filename, std::ifstream::ate | std::ifstream::binary);
+    check_equal("check file size is " + boost::lexical_cast<std::string>(expectedSize), in.tellg(), expectedSize);
+}
+
+int main() {
+    const char *fname = "time_duration_serialization.v0";
+    time_duration td(12, 13, 52, 123456);
+
+#if BOOST_DATE_TIME_POSIX_TIME_DURATION_VERSION == 0
+    std::ofstream ofs(fname, std::ios_base::binary | std::ios_base::out | std::ios_base::trunc);
+    boost::archive::binary_oarchive oa(ofs);
+    oa << td;
+    ofs.close();
+
+#if defined(_MSC_VER)
+    check_filesize(fname, 62);
+#endif
+#else
+    std::ifstream ifs(fname, std::ios_base::binary | std::ios_base::in);
+    boost::archive::binary_iarchive ia(ifs);
+    time_duration tmp;
+    ia >> tmp;
+    ifs.close();
+    check_equal("read older version structure ok", td, tmp);
+
+    std::ofstream ofs("time_duration_serialization.v1", std::ios_base::binary | std::ios_base::out | std::ios_base::trunc);
+    boost::archive::binary_oarchive oa(ofs);
+    oa << td;
+    ofs.close();
+
+#if defined(_MSC_VER)
+    check_filesize("time_duration_serialization.v1", 74);
+#endif
+#endif
+
+    return printTestStats();
+}

xmldoc/time_duration.xml

@@ -234,7 +234,7 @@ time_duration td(duration_from_string(ts));</screen>
 	</thead>
 	<tbody>
           <row>
-	    <entry valign="top" morerows="1"><screen>long hours()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t hours()</screen></entry>
             <entry>Get the number of normalized hours (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>
@@ -245,7 +245,7 @@ neg_td.hours(); // --> -1</screen></entry>
           </row>
 
 	  <row>
-	    <entry valign="top" morerows="1"><screen>long minutes()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t minutes()</screen></entry>
             <entry>Get the number of minutes normalized +/-(0..59) (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>
@@ -256,7 +256,7 @@ neg_td.minutes(); // --> -2</screen></entry>
         </row>
 
 	  <row>
-	    <entry valign="top" morerows="1"><screen>long seconds()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t seconds()</screen></entry>
             <entry>Get the normalized number of second +/-(0..59) (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>
@@ -267,7 +267,7 @@ neg_td.seconds(); // --> -3</screen></entry>
         </row>
 
 	  <row>
-	    <entry valign="top" morerows="1"><screen>long total_seconds()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t total_seconds()</screen></entry>
 	    <entry>Get the total number of seconds truncating any fractional seconds (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>
@@ -278,7 +278,7 @@ td.total_seconds();
           </row>
 
 	  <row>
-	    <entry valign="top" morerows="1"><screen>long total_milliseconds()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t total_milliseconds()</screen></entry>
 	    <entry>Get the total number of milliseconds truncating any remaining digits (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>
@@ -291,7 +291,7 @@ td.total_milliseconds();
           </row>
 
 	  <row>
-	    <entry valign="top" morerows="1"><screen>long total_microseconds()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t total_microseconds()</screen></entry>
 	    <entry>Get the total number of microseconds truncating any remaining digits (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>
@@ -304,7 +304,7 @@ td.total_microseconds();
           </row>
 
 	  <row>
-	    <entry valign="top" morerows="1"><screen>long total_nanoseconds()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t total_nanoseconds()</screen></entry>
 	    <entry>Get the total number of nanoseconds truncating any remaining digits (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>
@@ -318,7 +318,7 @@ td.total_nanoseconds();
           </row>
 
 	  <row>
-	    <entry valign="top" morerows="1"><screen>long fractional_seconds()</screen></entry>
+	    <entry valign="top" morerows="1"><screen>boost::int64_t fractional_seconds()</screen></entry>
 	    <entry>Get the number of fractional seconds (will give unpredictable results if calling <code>time_duration</code> is a <code>special_value</code>).</entry>
 	  </row>
 	  <row>