From de171954fe45e4d1338a641ebe70a84a181bd258 Mon Sep 17 00:00:00 2001 From: "James E. King III" Date: Sat, 23 Dec 2017 13:05:44 -0500 Subject: [PATCH] Fix serialization problem with time_duration This fixes #56 --- .gitignore | 4 +- .../date_time/posix_time/time_serialize.hpp | 85 ++++++++++++++----- .../date_time/time_resolution_traits.hpp | 25 +++++- test/Jamfile.v2 | 11 +++ .../testtime_serialize_versioning.cpp | 56 ++++++++++++ xmldoc/time_duration.xml | 16 ++-- 6 files changed, 167 insertions(+), 30 deletions(-) create mode 100644 test/posix_time/testtime_serialize_versioning.cpp diff --git a/.gitignore b/.gitignore index 548c7471d..5054abf40 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ -/test/test_facet_file.out \ No newline at end of file +/test/time_duration_serialization.v0 +/test/time_duration_serialization.v1 +/test/test_facet_file.out diff --git a/include/boost/date_time/posix_time/time_serialize.hpp b/include/boost/date_time/posix_time/time_serialize.hpp index 8650ae1da..71f60b374 100644 --- a/include/boost/date_time/posix_time/time_serialize.hpp +++ b/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 +void save_td(Archive& ar, const posix_time::time_duration& td) +{ + TimeResTraitsSize h = boost::numeric_cast(td.hours()); + TimeResTraitsSize m = boost::numeric_cast(td.minutes()); + TimeResTraitsSize s = boost::numeric_cast(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 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(ar, td); + } else { + save_td(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 +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 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(ar, td); + } else { + load_td(ar, td); + } } } diff --git a/include/boost/date_time/time_resolution_traits.hpp b/include/boost/date_time/time_resolution_traits.hpp index 7ba42c20b..b62248800 100644 --- a/include/boost/date_time/time_resolution_traits.hpp +++ b/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 var_type = boost::int64_t > // see note above class time_resolution_traits { public: typedef typename frac_sec_type::int_type fractional_seconds_type; diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 0bd240671..c803c8e11 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -198,6 +198,17 @@ local DATE_TIME_PROPERTIES = BOOST_DATE_TIME_POSIX_TIME_STD_CONFIG DATE_TIME_XML_SERIALIZE : testtime_serialize_xml ] + [ run posix_time/testtime_serialize_versioning.cpp + ../build//boost_date_time/static + ../../serialization/build//boost_serialization + : : : $(DATE_TIME_PROPERTIES) BOOST_DATE_TIME_POSIX_TIME_DURATION_VERSION=0 + : testtime_serialize_versioning_prev ] + [ run posix_time/testtime_serialize_versioning.cpp + ../build//boost_date_time/static + ../../serialization/build//boost_serialization + : : testtime_serialize_versioning_prev + : $(DATE_TIME_PROPERTIES) + : testtime_serialize_versioning_curr ] # text archive tests [ run gregorian/testgreg_serialize.cpp diff --git a/test/posix_time/testtime_serialize_versioning.cpp b/test/posix_time/testtime_serialize_versioning.cpp new file mode 100644 index 000000000..a1dcae666 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include "../testfrmwk.hpp" +#include + +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(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(); +} diff --git a/xmldoc/time_duration.xml b/xmldoc/time_duration.xml index fc8f176bb..4ae53fe53 100644 --- a/xmldoc/time_duration.xml +++ b/xmldoc/time_duration.xml @@ -234,7 +234,7 @@ time_duration td(duration_from_string(ts)); - long hours() + boost::int64_t hours() Get the number of normalized hours (will give unpredictable results if calling time_duration is a special_value). @@ -245,7 +245,7 @@ neg_td.hours(); // --> -1 - long minutes() + boost::int64_t minutes() Get the number of minutes normalized +/-(0..59) (will give unpredictable results if calling time_duration is a special_value). @@ -256,7 +256,7 @@ neg_td.minutes(); // --> -2 - long seconds() + boost::int64_t seconds() Get the normalized number of second +/-(0..59) (will give unpredictable results if calling time_duration is a special_value). @@ -267,7 +267,7 @@ neg_td.seconds(); // --> -3 - long total_seconds() + boost::int64_t total_seconds() Get the total number of seconds truncating any fractional seconds (will give unpredictable results if calling time_duration is a special_value). @@ -278,7 +278,7 @@ td.total_seconds(); - long total_milliseconds() + boost::int64_t total_milliseconds() Get the total number of milliseconds truncating any remaining digits (will give unpredictable results if calling time_duration is a special_value). @@ -291,7 +291,7 @@ td.total_milliseconds(); - long total_microseconds() + boost::int64_t total_microseconds() Get the total number of microseconds truncating any remaining digits (will give unpredictable results if calling time_duration is a special_value). @@ -304,7 +304,7 @@ td.total_microseconds(); - long total_nanoseconds() + boost::int64_t total_nanoseconds() Get the total number of nanoseconds truncating any remaining digits (will give unpredictable results if calling time_duration is a special_value). @@ -318,7 +318,7 @@ td.total_nanoseconds(); - long fractional_seconds() + boost::int64_t fractional_seconds() Get the number of fractional seconds (will give unpredictable results if calling time_duration is a special_value).