ZonedDateTime::of not working correctly with seconds part offset

[solodkiy]

Looks like \DateTimeZone with used in ZonedDateTime::of logic not working correctly with seconds in offset. It silently convert any offset with seconds to UTC timezone.

1) Brick\DateTime\Tests\ZonedDateTimeTest::testOf with data set #17 ('2016-05-17T12:34:56.123456789', '+00:15:01', '+00:15:01', 0, 1463487597, 123456789)
Failed asserting that 1463488496 is identical to 1463487597.

diff --git a/tests/ZonedDateTimeTest.php b/tests/ZonedDateTimeTest.php
index 151b3b4..2bb4edb 100644
--- a/tests/ZonedDateTimeTest.php
+++ b/tests/ZonedDateTimeTest.php
@@ -75,6 +75,7 @@ class ZonedDateTimeTest extends AbstractTestCase
             ['2014-03-15T12:34:56.123456789', '-00:15', '-00:15', 0, 1394887796, 123456789],
             ['2015-04-16T12:34:56.123456789', '+00:00', '+00:00', 0, 1429187696, 123456789],
             ['2016-05-17T12:34:56.123456789', '+00:15', '+00:15', 0, 1463487596, 123456789],
+            ['2016-05-17T12:34:56.123456789', '+00:15:01', '+00:15:01', 0, 1463487597, 123456789],
             ['2017-06-18T12:34:56.123456789', '+00:30', '+00:30', 0, 1497787496, 123456789],
             ['2018-07-19T12:34:56.123456789', '+00:45', '+00:45', 0, 1532000996, 123456789],
             ['2019-08-20T12:34:56.123456789', '+01:00', '+01:00', 0, 1566300896, 123456789],

1) Brick\DateTime\Tests\TimeZoneTest::testFromDateTimeZone with data set #2 ('+01:00:01')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'+01:00:01'
+'Z'

/home/doc/projects/_forks/date-time/tests/TimeZoneTest.php:81

FAILURES!
Tests: 16, Assertions: 23, Failures: 1.
[doc@doc-pc ~/projects/_forks/date-time (master)]# git diff tests/Time
TimeZoneOffsetTest.php  TimeZoneRegionTest.php  TimeZoneTest.php        
[doc@doc-pc ~/projects/_forks/date-time (master)]# git diff tests/TimeZoneTest.php
diff --git a/tests/TimeZoneTest.php b/tests/TimeZoneTest.php
index 4e4709f..60b0174 100644
--- a/tests/TimeZoneTest.php
+++ b/tests/TimeZoneTest.php
@@ -86,6 +86,7 @@ class TimeZoneTest extends AbstractTestCase
         return [
             ['Z'],
             ['+01:00'],
+            ['+01:00:01'],
             ['Europe/London'],
             ['America/Los_Angeles']
         ];

[solodkiy]

Looks like we should or start throw exception on any seconds part start from TimeZoneOffset object, or rewrite ZonedDateTime::of logic to not use \DateTimeZone. First solution looks more reasonable for me.

[BenMorel]

Good finding! I don't think there is a use case for a time-zone offset with seconds, so I agree that we should throw an exception.

Should we regexp it first, before giving it to DateTimeZone?

Just read the code again, and if I'm not mistaken, we should deny creating a TimeZoneOffset with seconds altogether:

• in TimeZoneOffset factory methods
• in IsoParsers::timeZoneOffset()

BTW, I think this should also be brought up on the internals mailing list, as it's a rather big annoyance.

[BenMorel]

This will be fixed in PHP 8.1: https://bugs.php.net/bug.php?id=81097

Do you think we should throw an exception in earlier PHP versions, when seconds is not zero?

[BenMorel]

Fixed in 01c7e255575be8dce95d95c57a380c47c360d985. We now prevent seconds altogether in TimeZoneOffset, so that we're always compatible with DateTimeZone.