fix: emit full seconds in Instant and ZonedDateTime toISOString (RFC 3339)
Instant::toISOString() and ZonedDateTime::toISOString() emitted a compact time that dropped zero seconds (e.g. 2023-11-14T22:14Z), which is valid ISO 8601 but not valid RFC 3339. Section 5.6 of RFC 3339 requires the full HH:MM:SS time even when the seconds are zero. These types are meant for interoperability, so they must always emit RFC 3339 compliant output. Build the time part of ZonedDateTime::toISOString() explicitly with a mandatory seconds component instead of delegating to LocalTime's compact formatter. Instant delegates to ZonedDateTime, so it is fixed transitively; jsonSerialize() and __toString() route through toISOString(). LocalTime's documented compact format is intentional and left unchanged. This changes the serialized string representation and is therefore a BC break; fixtures that pinned the compact output are updated accordingly.
This commit is contained in:
1 parent
104867f051
commit
6bb686d4af
6 files changed
+163
-36
No files matched your search
@@ -0,0 +1,97 @@
|
||||
# fix: emit full seconds in Instant and ZonedDateTime toISOString (RFC 3339)
|
||||
|
||||
Fixes #110
|
||||
|
||||
## Problem
|
||||
|
||||
`Instant::toISOString()` (and therefore `Instant::jsonSerialize()` / `(string)`) and
|
||||
`ZonedDateTime::toISOString()` emit a compact time format that omits the seconds when they
|
||||
are zero, e.g.:
|
||||
|
||||
```
|
||||
2023-11-14T22:14Z // whole minute
|
||||
1970-01-01T00:00Z // epoch
|
||||
2023-11-14T23:00Z // whole hour
|
||||
```
|
||||
|
||||
This is a valid ISO 8601 time, but it is **not** a valid RFC 3339 date-time. RFC 3339
|
||||
section 5.6 defines `full-time` as `partial-time time-offset` where
|
||||
`partial-time = time-hour ":" time-minute ":" time-second [time-secfrac]` — the seconds
|
||||
component is mandatory, even when it is `00`. `Instant` and `ZonedDateTime` are the types
|
||||
meant for interoperability (JSON serialization, wire formats), so they should always emit a
|
||||
value other systems can parse as RFC 3339. Producing `2023-11-14T22:14Z` breaks strict
|
||||
RFC 3339 parsers.
|
||||
|
||||
## Root cause
|
||||
|
||||
The compact behavior is intentional and documented in
|
||||
`src/LocalTime.php:639` (`toISOString()` — "the shortest that outputs the full value of the
|
||||
time"). That is correct for `LocalTime` and is left untouched.
|
||||
|
||||
The bug is that the interop types inherit that compact form:
|
||||
|
||||
- `src/ZonedDateTime.php:740` `toISOString()` built the string from
|
||||
`$this->localDateTime`, whose `__toString()` delegates to `LocalTime::toISOString()`
|
||||
(the compact formatter).
|
||||
- `src/Instant.php:362` `toISOString()` returns `(string) ZonedDateTime::ofInstant(...)`,
|
||||
so it inherited the same compact time. `jsonSerialize()` and `__toString()` both route
|
||||
through `toISOString()`.
|
||||
|
||||
## The fix
|
||||
|
||||
`ZonedDateTime::toISOString()` now builds the time portion explicitly with a mandatory
|
||||
`HH:MM:SS` component (keeping the existing optional fractional-seconds handling), instead of
|
||||
delegating to `LocalTime`'s compact formatter. `Instant::toISOString()` is fixed
|
||||
transitively because it delegates to `ZonedDateTime`; its PHPDoc now documents the RFC 3339
|
||||
guarantee. `LocalTime` is deliberately not changed.
|
||||
|
||||
Output after the fix:
|
||||
|
||||
```
|
||||
2023-11-14T22:14:00Z
|
||||
1970-01-01T00:00:00Z
|
||||
2023-11-14T23:00:00Z
|
||||
2023-11-14T23:14:00+01:00[Europe/Paris]
|
||||
```
|
||||
|
||||
Fractional seconds and non-zero seconds are unchanged (e.g.
|
||||
`1970-01-01T00:00:00.000123456Z`, `1969-12-31T23:59:59Z`).
|
||||
|
||||
## Why the test fixture changes are justified (BC break)
|
||||
|
||||
Some existing fixtures pinned the buggy compact output and had to be updated to the
|
||||
RFC-3339-correct strings — this is the legitimate case of changing a maintainer's tests
|
||||
because the old expected values encode the bug being fixed:
|
||||
|
||||
- `tests/InstantTest.php` `providerToString`: `[0, 0, '1970-01-01T00:00Z']` ->
|
||||
`'1970-01-01T00:00:00Z'`.
|
||||
- `tests/ZonedDateTimeTest.php`: the DST plus/minus and interval result strings that landed
|
||||
on whole minutes/hours (e.g. `2025-03-30T01:00+01:00[Europe/Prague]` ->
|
||||
`2025-03-30T01:00:00+01:00[Europe/Prague]`).
|
||||
- `tests/IntervalTest.php`: one exception-message fixture (`1970-01-04T11:20Z` ->
|
||||
`1970-01-04T11:20:00Z`).
|
||||
|
||||
New assertions were added for whole-minute and whole-hour values to lock in the `:00`:
|
||||
|
||||
- `tests/InstantTest.php`: `[1700000040, 0, '2023-11-14T22:14:00Z']` and
|
||||
`[1700002800, 0, '2023-11-14T23:00:00Z']`.
|
||||
- `tests/ZonedDateTimeTest.php`: `['2000-01-20T12:34', '-07:00', '2000-01-20T12:34:00-07:00']`
|
||||
and `['2000-01-20T12:00', 'America/Los_Angeles', '2000-01-20T12:00:00-08:00[America/Los_Angeles]']`.
|
||||
|
||||
Because the serialized string representation changes, this is technically a backward
|
||||
compatibility break, matching the maintainer's note on issue #110: "this will target
|
||||
version 0.8.0 as this is technically a BC break." Parsing is unaffected — the parser still
|
||||
accepts the compact input form, so round-tripping and existing stored values keep working.
|
||||
|
||||
## Test evidence
|
||||
|
||||
Gate command:
|
||||
|
||||
```
|
||||
composer install && vendor/bin/phpunit
|
||||
```
|
||||
|
||||
Result: exit code `0` — `OK (7865 tests, 38307 assertions)`, 0 failures.
|
||||
|
||||
Before the fix, `Instant::of(1700000040, 0)->toISOString()` returned `2023-11-14T22:14Z`;
|
||||
after the fix it returns `2023-11-14T22:14:00Z`.
|
||||
Reference in new issue
Block a user