Fix timezone conversion skipped with DateTimeImmutable - #171
Merged
rlanvin merged 1 commit intoJul 29, 2026
Merged
Conversation
DateTimeImmutable::setTimezone() returns a new object instead of modifying the receiver, so four calls that discarded the return value never converted anything when dtstart, until, or the occursAt() argument was immutable. As a result occursAt() ran its BYHOUR/BYMINUTE/BYSECOND and BYMONTHDAY checks against the wall clock of the unconverted date, and rfcString() emitted local time labelled as UTC for DTSTART with an unsupported timezone such as "+02:00", plus an unconverted UNTIL. Assigning the return value is a no-op for DateTime, which mutates in place and returns itself, so existing callers are unaffected. PHP 8.5 surfaces the bug via #[\NoDiscard], but the wrong output occurs on every supported version.
Contributor
Author
|
Just saw, one of the cases was already requested as MR in #170 Explanation and test added in the commit. |
Owner
|
Thank you for adding explanation and tests, this actually helped me a lot to understand what the issue was. |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DateTimeImmutable::setTimezone()returns a new object instead of modifying the receiver, but four call sites discard the return value. Whendtstart,untilor theoccursAt()argument is aDateTimeImmutable, the conversion silently never happens.RRule.php:609—rfcString()sets$timezone_name = 'UTC'for an unsupported timezone like+02:00, but$dtstartkeeps its original offset, so local time is emitted labelled as UTC.RRule.php:643/RRule.php:649—UNTILends up neither in UTC nor on the DTSTART timezone.RRule.php:773—occursAt()carries the comment "convert timezone to dtstart timezone for comparison", but theBYHOUR/BYMINUTE/BYSECONDandBYMONTHDAYchecks that follow readformat()off the unconverted date, so the returned result can be wrong.