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

DateTime modify with tz pattern should not update linked timezone #10583

Closed
marc-mabe opened this issue Feb 14, 2023 · 7 comments
Closed

DateTime modify with tz pattern should not update linked timezone #10583

marc-mabe opened this issue Feb 14, 2023 · 7 comments

Comments

@marc-mabe
Copy link
Contributor

Description

The following code: https://3v4l.org/EMU1K

<?php

$dt = new DateTime('2015-01-01 00:00:00+00:00');
var_dump($dt->format('c'));
var_dump($dt->modify('+1 s')->format('c'));

$dt = new DateTimeImmutable('2015-01-01 00:00:00+00:00');
var_dump($dt->format('c'));
var_dump($dt->modify('+1 s')->format('c'));

Resulted in this output:

string(25) "2015-01-01T00:00:00+00:00"
string(25) "2015-01-01T00:00:00+01:00"
string(25) "2015-01-01T00:00:00+00:00"
string(25) "2015-01-01T00:00:00+01:00"

But I expected this output instead:

string(25) "2015-01-01T00:00:00+00:00"
string(25) "2015-01-01T00:00:00+00:00"
string(25) "2015-01-01T00:00:00+00:00"
string(25) "2015-01-01T00:00:00+00:00"

As you can see it's modifying the time zone offset from +00:00 to +01:00 instead of doing nothing as s is not a valid unit according to https://www.php.net/manual/en/datetime.formats.relative.php.
It's interesting here because s is a common mistake as abbr. of second & sec where also ms and µs would be valid (which is another topic) but there is also no documented way of modifying the time zone using modify and in this particular case it's effectively moving backward instead of forward.

PHP Version

PHP 8.1.15 / PHP 8.2.2

Operating System

Linux

@hormus
Copy link

hormus commented Feb 14, 2023

PHP 8.1.15, 8.2.2 and later this fix change offset of modify function #9891 (comment)

If unknow format new DateTime('2015-01-01 00:00:00' . '+1 s') is timezone offset 01:00 from php all.
tzcorrection "GMT"? [+-] hh valid syntax https://www.php.net/manual/en/datetime.formats.time.php

@marc-mabe
Copy link
Contributor Author

@hormus You are right that the invalid unit is still ignored. Just tried it without any unit ->modify('+1') and now it's effectively setting the time zone.

Since 8.1.5 the following sets the time zone to +12:34 no matter what was set before:

$dt = new DateTime('2015-01-01 00:00:00+01:00');
var_dump($dt->format('c'));
var_dump($dt->modify('+12:34')->format('c'));

No sure if this is intended.

@hormus
Copy link

hormus commented Feb 14, 2023

hello @marc-mabe , if you read the previous fix for the modify function for php 8.1.15, 8.2.2 and later it is now possible to change timezone.

@marc-mabe
Copy link
Contributor Author

Hi @hormus,

The fix you linked is for setting the unix timestamp using @\d+ format where it's defined to be UTC. SO yes this will now set the timezone to UTC but I'm failing how this implies supporting [+-]\d+ format to set the time zone.

Please don't understand me wrong. I don't want to say this is definitely a bug but it's for sure a behavior change that I don't see documented anywhere.

@hormus
Copy link

hormus commented Feb 16, 2023

If you open the link of the fix written in C++ code in that thread you can see the change from immutable to mutable "tmp_time" for timezone
Yes, the documentation needs improvement

@derickr
Copy link
Contributor

derickr commented Mar 23, 2023

This doesn't look right indeed. modify isn't meant to be able to set the timezone like this. I'll have a good look.

@derickr
Copy link
Contributor

derickr commented Mar 23, 2023

This was introduced in the fix for #9891: d19a70c

@derickr derickr changed the title Regression in DateTime[Immutable]->modify with invalid unit since 8.1.15 and 8.2.2 DateTime modify with tz pattern should not update linked timezone Mar 23, 2023
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

5 participants
@derickr @marc-mabe @Girgias @hormus and others