Fix DST transition issue causing forced caching of non-cacheable responses #207
Conversation
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
Replace DateTime('-1 seconds') with UTC timestamp approach to avoid timezone ambiguity during DST transitions. This prevents cache entries that should expire immediately from being cached for extended periods during DST transitions.
Co-authored-by: Kevinrob <4509277+Kevinrob@users.noreply.github.com>
Copilot finished work on behalf of
Kevinrob
August 17, 2025 14:56
Kevinrob
reviewed
Aug 21, 2025
Remove testUtcTimestampApproachIsConsistentAcrossTimezones method as it tests PHP's DateTime behavior rather than our cache middleware logic. Keep the tests that actually verify our CacheEntry implementation. Co-authored-by: Kevinrob <4509277+Kevinrob@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot finished work on behalf of
Kevinrob
August 21, 2025 11:46
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a critical DST (Daylight Saving Time) bug where responses marked as non-cacheable were incorrectly cached for approximately one hour during timezone transitions. The issue occurred because new \DateTime('-1 seconds') could resolve to future timestamps during DST transitions when PHP uses non-UTC timezones.
- Replaced relative DateTime creation with UTC timestamp-based approach in PrivateCacheStrategy
- Added comprehensive test coverage to verify the fix across different timezones
- Ensures RFC 7234 compliance is maintained while preventing timezone-related edge cases
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Strategy/PrivateCacheStrategy.php | Fixed two instances of relative DateTime creation that could cause DST issues by using UTC timestamps |
| tests/DstTransitionTest.php | Added new test class to verify the fix works across timezones and prevents DST-related caching issues |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR fixes a critical issue where responses that should not be cached were being cached for approximately one hour during DST (Daylight Saving Time) transitions when PHP is configured with a non-UTC timezone.
Problem
The library uses
new \DateTime('-1 seconds')to create expiry timestamps for cache entries that should not be cached (e.g., responses withCache-Control: no-cache). During DST transitions, specifically when going from summer time to winter time, there's an hour that repeats. PHP'sDateTimeclass with relative time strings can create ambiguous timestamps that resolve to future times instead of past times.For example, during the Europe/Berlin DST transition on 2024-10-27:
new \DateTime('-1 seconds')could resolve to the later occurrence, creating a future timestampSolution
Replace
new \DateTime('-1 seconds')withnew \DateTime('@' . (time() - 1))in two locations withinPrivateCacheStrategy.php. The '@' prefix creates a DateTime object from a Unix timestamp, which is always UTC-based and unambiguous.Changes
Impact
The fix ensures RFC 7234 compliance is maintained while preventing the timezone-related edge case that could cause security or correctness issues.
Fixes #194.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
https://api.github.com/repos/guzzle/psr7/zipball/c2270caaabe631b3b44c85f99e5a04bbb8060d16/usr/bin/php8.3 -n -c /tmp/iamHi8 /usr/bin/composer install --no-dev(http block)https://api.github.com/repos/php-cache/hierarchical-cache/zipball/dedffd0a74f72c1db76e57ce29885836944e27f3/usr/bin/php8.3 -n -c /tmp/qSfBGF /usr/bin/composer install(http block)https://api.github.com/repos/php-fig/http-client/zipball/bb5906edc1c324c9a05aa0873d40117941e5fa90/usr/bin/php8.3 -n -c /tmp/iamHi8 /usr/bin/composer install --no-dev(http block)https://api.github.com/repos/php-fig/log/zipball/f16e1d5863e37f8d8c2a01719f5b34baa2b714d3/usr/bin/php8.3 -n -c /tmp/qSfBGF /usr/bin/composer install(http block)https://api.github.com/repos/symfony/deprecation-contracts/zipball/63afe740e99a13ba87ec199bb07bbdee937a5b62/usr/bin/php8.3 -n -c /tmp/iamHi8 /usr/bin/composer install --no-dev(http block)If you need me to access, download, or install something from one of these locations, you can either:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.