Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Since PHP 8.1.7 strtotime translates a date-time with DST/non-DST hour differently based on default timezone #9165

Closed
ashiful-kyos opened this issue Jul 27, 2022 · 10 comments

Comments

@ashiful-kyos
Copy link

ashiful-kyos commented Jul 27, 2022

Description

For the timezone Europe/London on date 2018-10-28 two hour 01:00:00 exist - one DST and another non-DST. Following code shows that it gives two different timestamp based on the set default timezone. Which should not be the case as the string date-time provided to strtotime already contains the timezone.
Prior to PHP 8.1.7 strtotime was translating it into 1540688400 regardless set timezone.

<?php
date_default_timezone_set('America/Lower_Princes');
echo strtotime("2018-10-28 01:00:00 Europe/London") . PHP_EOL;

date_default_timezone_set('Europe/London');
echo strtotime("2018-10-28 01:00:00 Europe/London"). PHP_EOL;

Resulted in this output:

1540688400
1540684800

But I expected this output instead:

1540688400
1540688400

PHP Version

PHP 8.1.7

Operating System

No response

@cmb69
Copy link
Member

cmb69 commented Jul 27, 2022

Confirmed: https://3v4l.org/NEBOD. I'm not sure, though, whether the new behavior is intended, since the documentation states:

Each parameter of this function uses the default time zone unless a time zone is specified in that parameter. Be careful not to use different time zones in each parameter unless that is intended.

Since $baseTimestamp is not given, it may be interpreted using the default timezone.

@ashiful-kyos
Copy link
Author

Confirmed: https://3v4l.org/NEBOD. I'm not sure, though, whether the new behavior is intended, since the documentation states:

Each parameter of this function uses the default time zone unless a time zone is specified in that parameter. Be careful not to use different time zones in each parameter unless that is intended.

Since $baseTimestamp is not given, it may be interpreted using the default timezone.

As per my test this issue is only with the date-time that has duplicate hour ie. hour in dst and non-dst. If $baseTimestamp has a role in this case it would result in different timestamp for other date-times, which is not the case. For example following code results in the same timestamp regardless default timezone.

<?php
date_default_timezone_set('America/Lower_Princes');
echo strtotime("2018-10-28 04:00:00 Europe/London") . PHP_EOL;

date_default_timezone_set('Europe/London');
echo strtotime("2018-10-28 04:00:00 Europe/London"). PHP_EOL;

output

1540699200
1540699200

@derickr
Copy link
Member

derickr commented Aug 10, 2022

I'm pretty sure this change has been introduced through derickr/timelib#121. These calculations are very magic, and changing them again will likely cause other issues.

Unless https://wiki.php.net/rfc/datetime_and_daylight_saving_time is implemented, both answers are "not wrong", although a little confusing. I do plan to tackle this issue, but not likely for PHP 8.2.

@ashiful-kyos
Copy link
Author

ashiful-kyos commented Aug 11, 2022

I'm pretty sure this change has been introduced through derickr/timelib#121. These calculations are very magic, and changing them again will likely cause other issues.

Unless https://wiki.php.net/rfc/datetime_and_daylight_saving_time is implemented, both answers are "not wrong", although a little confusing. I do plan to tackle this issue, but not likely for PHP 8.2.

Let me give more example. Consider the following code

date_default_timezone_set('Europe/London');
echo strtotime("2018-10-28 01:00:00 Europe/London"). PHP_EOL;

echo (new DateTime('2018-10-28 01:00:00', new DateTimeZone('Europe/London')))->getTimestamp();

Before Php 8.1.7 both resulted in the same timestamp

1540688400
1540688400

However since PHP 8.1.7 they are different

1540684800
1540688400

If two methods are translating one date-time to two different timestamp, I don't think it is merely confusing, it is wrong.

@mitchdesign
Copy link

As someone who works a lot with hourly datasets, I am sympathetic to the complexity of these issues.

But, also I was very happy with the change made in 8.1.7, where a strtotime() on a string containing a duplicate hour will now return the timestamp of the first instance. This is in my opinion much better than the previous behaviour of returning the second one, because now a value can go back and forth using strtotime() and date() and not change. It makes many workflows a lot safer because we do not have to write special code for handling the case of our timestamp changing on us during manipulations.

Before:

v $timestamp $str = date('Y-m-d H:i', $timestamp) strtotime($str) .
8.1.6 1540684800 2018-10-28 01:00 1540688400 DIFFERENT
8.1.7 1540684800 2018-10-28 01:00 1540684800 SAME ✅

Now, the downside is due to the current issue, we still cannot rely on this behaviour.. Even though I agree that officially it is "not wrong" to give either value, the recently introduced behaviour is in practical use a lot better. So it is really inconvenient that there still is this unexpected variance.

And of course, because the whole point of passing timezones is that the default zone should not matter, I think it is a real bug that this difference exists, even if one accepts that the two values are both "not wrong".

athos-ribeiro added a commit to athos-ribeiro/cron-expression that referenced this issue Sep 1, 2022
As discussed in dragonmantank#133, the PHP 8.1's date extension daylight saving
APIs have been suffering with instabilities. Let's skip the affected
tests until php/php-src#9165 is resolved.

Signed-off-by: Athos Ribeiro <[email protected]>
dragonmantank pushed a commit to dragonmantank/cron-expression that referenced this issue Sep 6, 2022
As discussed in #133, the PHP 8.1's date extension daylight saving
APIs have been suffering with instabilities. Let's skip the affected
tests until php/php-src#9165 is resolved.

Signed-off-by: Athos Ribeiro <[email protected]>

Signed-off-by: Athos Ribeiro <[email protected]>
@derickr
Copy link
Member

derickr commented Sep 14, 2022

PR at #9538 — for PHP 8.1.11.

derickr added a commit that referenced this issue Sep 14, 2022
- Fixed #124: Can't parse INT_MIN
- Added a new API, timelib_get_time_zone_offset_info, which reduces allocation
  speeding up algorithms (Alberto Massari)
- Accelerate the do_range_limit_days algorythm by advancing multiple months in
  a single call (Alberto Massari)

Including fixes from 2021.17:

- Fixed 'const' and other compiler warnings
- Use new 'PACKRAT' format to prevent old timestamps from becoming incorrect
- New 2022b data file
- Fixed PHP GH-9165: strtotime translates a date-time with DST/non-DST hour
  differently
@tomandersen
Copy link

https://onlinephp.io?s=s7EvyCjg5dLXV3DNK0ktUqjMLy1SSM5PSVXISC1K1VFIzcvKr1Tk5UpNzshXUIrJC_AIULDQM1IoK1Yw1zNRKM8syVAoLikqyS_JzE1VSCpNt7dXiMlTsublQujRVTDNABpbDBEHi8K1aChZWhmaaOtCVChpWiO0EdSEpIeXCwA%2C&v=7.4.33%2C8.2.10

`<?php
// Enter your code here, enjoy!
echo "\nPHP 8.2 vs 7.4 with strtotime bug?? \n";

echo "\n- 5hours\n";
echo strtotime("9:14+-5hours");
echo "\n5hours\n";
echo strtotime("9:14+5hours");
`

`PHP7.4 with strtotime bug??

  • 5hours
    1700540040
    5hours
    1700576040`

`PHP 8.2

  • 5hours
    1700576040
    5hours
    1700576040`

@tomandersen
Copy link

Should we all just stop calling strtotime() ?

@ranvis
Copy link
Contributor

ranvis commented Nov 21, 2023

@tomandersen
Looks like OT, but "+-" doesn't work on 8.2.

number symbols in relative formats no longer accept multiple signs, e.g. +-2.
https://www.php.net/manual/en/migration82.incompatible.php

@tomandersen
Copy link

Thanks, I should have thought about that, some new string behaviour. I swear it wasn’t my code!

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

No branches or pull requests

7 participants