[PATCH] Incorrect decode from 64bit tzfile

[Ken Murchison]

On 3/9/21 2:03 PM, Allen Winter wrote:
> On Tuesday, March 9, 2021 1:01:53 PM EST Milan Crha wrote:
>> 	Hello,
>> I've been looking onto a downstream bug
>> https://gitlab.gnome.org/GNOME/evolution/-/issues/1381
>> which exhibits as showing incorrect UTC offset for Asia/Kolkata time
>> zone.
>>
>> Diving into the code the problem is that the times returned from
>>
>>    $ zdump Asia/Kolkata -i
>>
>> are like this:
>>
>>     TZ="Asia/Kolkata"
>>     -	        -		+055328	LMT
>>     1854-06-27	23:59:52	+055320	HMT
>>     1869-12-31	23:27:50	+052110	MMT
>>     1906-01-01	00:08:50	+0530	IST
>>     1941-10-01	01		+0630		1
>>     1942-05-14	23		+0530	IST
>>     1942-09-01	01		+0630		1
>>     1945-10-14	23		+0530	IST
>>
>> thus back in 1854 / 1869. The later, when truncated, is 2006, which
>> causes the wrong UTC offset, due to the code picking this change as the
>> nearest, even though the corresponding RRULE has set UNTIL (but this
>> information is not propagated in the 'changes' array).
>>
>> I realized there's only a type cast error for little endian when
>> decoding 64bit values from the tzfile, which is easy to fix (and hard
>> to find). See the attached patch.
>>
>> Checking what libical does now, it misses the 1854 transition, it
>> starts from 1869. It makes sense, because the num_trans are processed
>> from 1, not from 0. Starting from 0 does not help much, also because
>> there's missing the starting offset, thus I did not include it in this
>> patch.
>>
>> This change does not fix an issue with reading tzfile in the slim
>> format, even I do not see much difference in the fat and slim formats
>> regarding the values read from it by the libical. It's still to be
>> investigated what fails there.
>>
>> Good news is that before the patch the 'timezones' test returns:
>>   *** Summary: 425 zones tested, 2920 days failed, 152205 okay => 1% failed ***
>> while after the patch it returns:
>>   *** Summary: 425 zones tested, 1825 days failed, 153300 okay => 1% failed ***
>> thus there is an improvement.
>>
>> Of course, the right thing would be to have 0 failed days there.
>> 	Bye,
>> 	Milan
>>
>>
> Grrr.  how could we have missed this?
> all the other return statements in decode64() cast to (const long long int) already.


Yeah, that's on me.


Milan, I'm looking into the other icaltz-util.c issue and I know what it 
is.  The slim tzdata doesn't contain explicit transitions past where the 
TZ string in the footer can take over.  We need to parse the TZ string 
and create a non-terminating RRULE from it.

We don't see an issue with the fat tzdata (yet) because it contains 
explicit transitions up to 2037.


-- 

Kenneth Murchison
Senior Software Developer
Fastmail US LLC