brick / date-time

Date and time library for PHP
MIT License
323 stars 29 forks source link

`ZonedDateTime::of()` issue when year is less than 1892 #44

Closed dakur closed 1 year ago

dakur commented 2 years ago

Following code works well (no output):

ZonedDateTime::of(
    LocalDateTime::of(2022,1,1),
    TimeZoneRegion::parse('Europe/Prague')
);

while this one throws Brick\DateTime\DateTimeException: The time zone offset of 3464 seconds is not a multiple of 60:

ZonedDateTime::of(
    LocalDateTime::of(0,1,1),
    TimeZoneRegion::parse('Europe/Prague')
);

The only difference is the year passed into LocalDateTime::of(). If I try LocalDateTime::min() instead, it results in the exception as well.

I found out that the border where it stops working is 1891–1892 (which 130 years back from now). Is this some issue with DateTime and/or its getOffset() which is used inside of the ZonedDateTime::of()?

But it doesn't fail with UTC time zone actually..

jiripudil commented 2 years ago

Hi, for dates before time zones were invented, the timezonedb tends to observe the local mean time of the place and simply provides the UTC offset of the town's meridian. Consider the database entry for Europe/Prague:

# Zone  NAME            STDOFF  RULES   FORMAT  [UNTIL]
Zone    Europe/Prague   0:57:44 -       LMT     1850
                        0:57:44 -       PMT     1891 Oct    # Prague Mean Time
                        1:00    C-Eur   CE%sT   1945 May  9
                        ...

You can see that until October 1891, the UTC offset was 0:57:44. However, sub-minute offsets have been prohibited in 01c7e255. Perhaps @BenMorel can chime in with the reasoning behind the change?

BenMorel commented 2 years ago

@jiripudil Please check #35 for the rationale behind this. Could you please investigate the issue and tell me what you think the best move would be here? I'd happy to revert the commit, but PHP seems to always refuse seconds in offset now: https://3v4l.org/FG6nZ

(see the PHP bug in the linked issue)

Does that mean that we can't rely on, or have to hack around, DateTimeZone now?

jiripudil commented 2 years ago

Thanks for the context! It seems that this issue has been remediated in the fresh out of the oven PHP 8.1.7 by an update of the underlying timelib (php/php-src#8589).

Unfortunately, PHP's data structures do not support sub-minute offsets, so getName() truncates the seconds – this will be an issue in TimeZone::fromDateTimeZone, but I think it could be hacked around using getOffset() which appears to be computed correctly with seconds: https://3v4l.org/fFpgU

I don't know what your stance is on supported PHP versions, but even I would be hesitant to require a version that has just been tagged a few days ago. That being said, I see no reason why developers should be deprived of the feature if they're running a version of PHP that supports it. Perhaps we could only allow sub-minute offsets for PHP_VERSION_ID >= 80107? And add a hint about it to the exception message in previous versions?

BenMorel commented 2 years ago

That sounds good, do you want to open a PR?

jiripudil commented 2 years ago

Sure :)