pavkam/tzdb

Test for invalid local times is incorrect

pavkam opened this issue · 3 comments

Consider this test function.

procedure TForm1.Button1Click(Sender: TObject);
var
tz: TBundledTimeZone;
d: TDateTime;
begin
tz := TBundledTimeZone.Create('America/Vancouver');
d := tz.ToUniversalTime(StrToDateTime('3/11/2012 1:06:32 AM'));
d := tz.ToUniversalTime(StrToDateTime('3/11/2012 2:06:32 AM'));
tz.Free;
end;

=== What is the expected output? What do you see instead? ===

The first line will produce an ELocalTimeInvalid exception; the second will not. This is the opposite of the correct behaviour. At 2 AM local time, the clock rolls forward to 3 AM, so local times >= 2 AM and < 3AM should be invalid.

=== What version of the product are you using? On what operating system? ===

Delphi 7
Windows 7
TZDB 1.8

=== Please provide any additional information below. ===

I was able to correct the problem by changing the following lines 681-683 in TZDB.pas in
function TCompiledRule.GetLocalTimeType(const ADateTime: TDateTime): TLocalTimeType :

< if (FNext <> nil) and (FNext.FOffset > FOffset) and
< (CompareDateTime(ADateTime, IncSecond(FNext.FStartsOn, FOffset - FNext.FOffset)) >= 0) then
< Result := lttInvalid

if (FPrev <> nil) and (FOffset > FPrev.FOffset) and
(CompareDateTime(ADateTime, FStartsOn) >= 0) and
(CompareDateTime(ADateTime, IncSecond(FStartsOn, FOffset - FPrev.FOffset)) < 0) then
Result := lttInvalid

I am not sure if this is the correct fix, but it does appear to correctly address the boundary cases and makes intuitive sense. By the time this edge case is tested, the contents of FPrev / Self / FNext have already advanced, for which the current code fails to account.

(As a side note, I was not able to add breakpoints to tzdb.pas -- despite checking for CRLF line endings, removing DCU files, enabling debug info, etc. Would be interested to know if there is something about tzdb that prevents this.)

For clarity, it is the first call to TBundledTimeZone.ToUniversalTime that produces the exception (not the first line).

I think I fixed this with my update 9028f34