WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 9 months ago

#34435 new defect (bug)

get_gmt_from_date() can return incorrect value when input during fall's DST switch

Reported by: danielbachhuber Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Date/Time Keywords: needs-patch needs-docs
Focuses: Cc:

Description

When I use wp_insert_post() to create a post when timezone_tz is Europe/London ref, the post gets scheduled for the future.

$post_date is established here: https://core.trac.wordpress.org/browser/tags/4.3/src/wp-includes/post.php#L3272

Because $post_date_gmt isn't supplied, it's set here: https://core.trac.wordpress.org/browser/tags/4.3/src/wp-includes/post.php#L3292

However, get_gmt_from_date() returns the same value as its supplied, when it should return an hour less (as Europe/London respects DST). gmdate() returns the proper GMT date (one hour less) so the post gets forced to future ref

Here's debug of get_gmt_from_date():

<?php
$format = 'Y-m-d H:i:s';
$string = '2015-10-25 01:23:08';
$tz = 'Europe/London';
$datetime = date_create( $string, new DateTimeZone( $tz ) );
echo $datetime->format( $format ) . PHP_EOL;
// 2015-10-25 01:23:08
$datetime->setTimezone( new DateTimeZone( 'UTC' ) );
echo $datetime->format( $format ) . PHP_EOL;
// 2015-10-25 01:23:08, but should be 2015-10-25 00:23:08

Is this a bug in PHP, or how we're using DateTime? Tested on PHP 5.6.14, 5.5.9, and 5.5.27

Change History (7)

#1 @danielbachhuber
3 years ago

Oh. London apparently falls back on the last Sunday of October (e.g. today in Europe/London).

I have no idea what this bug actually is, but apparently get_gmt_from_date() produces a different value than gmdate() in a maybe twice per year edge case.

#2 follow-up: @danielbachhuber
3 years ago

After talking this through time and time again, @viper007bond and I feel reasonably confident that DST should be abolished.

The bug is most likely that 1:23 am occurs twice in London on 2015-10-25. DateTime doesn't know how to distinguish between them when doing relative time conversions so it assumes the later one. It could pick the correct one if it knew the UTC time.

#3 follow-up: @Viper007Bond
3 years ago

This bug should be avoidable if we stop determining GMT (UTC) date/time based on local date/time when creating a post without a provided date/time. Since those values will default to now, we don't need to convert timezones because we know what now is in UTC. We can determine now in UTC and local time independently of each other.

#4 @Viper007Bond
3 years ago

  • Summary changed from get_gmt_from_date() returns incorrect value when 'timezone_tz' is 'Europe/London' to get_gmt_from_date() can return incorrect value when input during fall's DST switch

#5 in reply to: ↑ 3 @Viper007Bond
3 years ago

Replying to Viper007Bond:

This bug should be avoidable if we stop determining GMT (UTC) date/time based on local date/time when creating a post without a provided date/time. Since those values will default to now, we don't need to convert timezones because we know what now is in UTC. We can determine now in UTC and local time independently of each other.

Note that this doesn't fix get_gmt_from_date() (I'm not sure that's possible), but it does fix trying to publish a post now and having it end up being scheduled an hour in the future.

#6 @danielbachhuber
3 years ago

  • Keywords needs-patch needs-docs added
  • Milestone changed from Awaiting Review to Future Release

Doing a search of the codebase, there are a few key places get_gmt_from_date() is used:

  • _wp_translate_postdata() ref
  • wp_insert_post() ref
  • wp_insert_comment() ref
  • Throughout wp_xmlrpc_server

I have two practical recommendations:

  • In the wp_insert_*() functions, when neither *_date nor *_date_gmt values are passed, *_date should be set from *_date_gmt
  • Include a concise note in get_gmt_from_date()'s PHPdoc mentioning this scenario where the function return value is ambiguous.

#7 in reply to: ↑ 2 @Rarst
9 months ago

Replying to danielbachhuber:

After talking this through time and time again, @viper007bond and I feel reasonably confident that DST should be abolished.

The bug is most likely that 1:23 am occurs twice in London on 2015-10-25. DateTime doesn't know how to distinguish between them when doing relative time conversions so it assumes the later one. It could pick the correct one if it knew the UTC time.

To be more precise what happens isn't just random assumption. PHP has a registered transition for Europe/London from BST to GMT at 2015-10-25T01:00:00+0000. So I think input before 01:00 is considered still in BST and after 01:00 is considered in GMT.

And yeah, on WP side it should ideally all just operate in UTC as much as possible. Concept of a "local" time for a post is deeply broken in many places.

Note: See TracTickets for help on using tickets.