Skip to content

Fix GH-21616: DateTime::modify() wrong timestamp across DST transitions - #21669

Open
MarcusXavierr wants to merge 6 commits into
php:PHP-8.4from
MarcusXavierr:PHP-8.4
Open

MarcusXavierr wants to merge 6 commits into
php:PHP-8.4from
MarcusXavierr:PHP-8.4

Conversation

@MarcusXavierr

@MarcusXavierr MarcusXavierr commented Apr 8, 2026 •

Copy link
Copy Markdown
Contributor

DateTime::modify() with relative h/i/s strings produces wrong timestamps near DST transitions.

do_adjust_relative() adds hours, minutes, and seconds to wall-clock fields before timelib_update_ts() converts to a Unix timestamp. Near a spring-forward gap, that intermediate wall-clock time is ambiguous and resolves to the wrong side. add()/sub() don't have this problem — timelib_add_wall() adds h/i/s directly to SSE.

In php_date_modify(), strip h, i, s, and us from the relative struct before timelib_update_ts(), then apply them via sse += afterward. Full seconds are extracted from any microsecond overflow before the SSE step. timelib_update_ts() still handles y/m/d, weekdays, and special relatives, so calendar strings like '+3 days' and 'first day of next month' are unaffected.

Fixes GH-21616.
Fixes GH-15880.

MarcusXavierr and others added 6 commits April 8, 2026 00:44
do_adjust_relative() adds h/i/s to wall-clock fields before converting
to a Unix timestamp. Near a spring-forward gap that produces a
nonexistent time, which resolves to the wrong offset.

Fix: zero out relative h/i/s/us before timelib_update_ts(), apply them
to sse directly. Same technique as timelib_add_wall(). y/m/d, weekday
relatives, and first_last_day_of still go through timelib_update_ts.

Also fixes phpGH-15880.
Always fold microseconds into seconds before SSE arithmetic instead of
guarding with if-checks. Handle negative remainders correctly.
Remove section headers and redundant cases. Add -72h backward through
fall-back, +61 minutes across spring-forward gap, and DateTimeImmutable.
@anun333

anun333 commented Oct 29, 2026

Copy link
Copy Markdown

Quoted below: written with Claude, an LLM, as CONTRIBUTING.md asks us to mark. The builds and test runs behind it are on my machine; the commands to repeat them are under the fold.

Tested on today's PHP-8.4 (c7ea9b17582) and merged up onto PHP-8.5 (6de713d23b2), comparing modify() with add()/sub() and a DateInterval, which add elapsed time (GH-21616's reference). Seconds from the start:

Start modify() PHP-8.4 this PR interval
London 2025-03-30 02:00:00 BST -1 second +3599 −1 −1
Amsterdam 2005-10-30 01:59:59 CEST +1 hour 1 second +7201 +3601 +3601
London 2026-10-25 01:30:00 BST +1 hour +7200 +3600 +3600
Dublin 2026-10-25 01:30:00 IST +1 hour +7200 +7200 +3600
GH-21616's +1 second, -1 second round trip 02:59:59 BST 00:59:59 GMT

The results are the same on PHP-8.5 with this merged up, and ext/date/tests passes on both branches. Dublin is the one left: the tz database gives Irish time a negative DST offset (IST is standard time, GMT the "DST"), the same root as GH-24735.

Two things for landing:

How this was tested

Linux x86_64, gcc 13.3, debug builds of origin/PHP-8.4 and origin/PHP-8.5, each with and without this PR's commit cherry-picked:

./buildconf --force
./configure --enable-debug --disable-cgi --disable-phpdbg --disable-fileinfo --enable-pcntl --enable-zend-test --enable-fpm --with-curl --enable-mbstring --disable-mbregex
make -j4

For each row, $d->modify($m) against $d->add(new DateInterval(...)) (or sub()), printing getTimestamp() differences; the fall-back starts are built from UTC ((new DateTimeImmutable('2026-10-25 00:30:00 UTC'))->setTimezone(new DateTimeZone('Europe/Dublin'))) so the zone stays the named one. Tests: TEST_PHP_EXECUTABLE=$PWD/sapi/cli/php sapi/cli/php run-tests.php -n -q -j4 ext/date/tests. The conflict: git cherry-pick of #24738's commits onto PHP-8.5 with this PR's applied stops in ext/date/php_date.c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants