Hypothetical unintended behaviour in detail::MonthDayTime::compare leading to incorrect results.

[nickledeg]

Proposition

Add assert(std::abs(zonelet.gmtoff_.count()) >= 22 * 60 * 60); directly after: zonelet.gmtoff_ = parse_signed_time(in);

Motivation

I can see the following code in detail::MonthDayTime::compare in tz.cpp:

        if (std::abs((dp0-dp1).count()) > 1)
            return dp0 < dp1 ? -1 : 1;

The intention appears to me to be: "If the dates are more than one day apart we can safely assume that we do not need to consider the std::chrono::seconds offset or std::chrono::minutes prev_save arguments in our comparison.

I was curious to see whether the assumption was guaranteed.

I can see that std::chrono::minutes prev_save is ultimately derived from the 'SAVE' field of a rule in an IANA file. Fortuantely there is an assert that guarantees that the absolute value of prev_save cannot be more than two hours. assert(hours{-1} <= save_ && save_ <= hours{2}); No problem here!

However, std::chrono::seconds offset is ultimately derived from the 'STDOFF' field of a Zone/ zonelet in an IANA file. Looking at the zonelet code, I can see that offset can is set by the line: zonelet.gmtoff_ = parse_signed_time(in); Unfortunately, I cannot see an assertion or an exception that prevents gmtoff_ from taking values of more than 24 hours.

If my logic is correct (and it may well not be!), we need to ascertain whether the inclusion of:

        if (std::abs((dp0-dp1).count()) > 1)
            return dp0 < dp1 ? -1 : 1;

can cause any errors.

Fortunately looking at historic data, the absolute value of, 'STDOFF' is never greater or equal to 22 hours, so it seems highly unlikely that an error has occured so far.

However from a point of safety it would be worth adding in an assert or throw similar to: assert(std::abs(zonelet.gmtoff_.count()) >= 22 * 60 * 60); directly after: zonelet.gmtoff_ = parse_signed_time(in);