Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#41782 closed defect (bug) (fixed)

Using date_query with 'before' in WP_Query returns wrong timezone

Reported by: biranit's profile Biranit Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.8.1
Component: Date/Time Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Hi,

When using the date_query for WP_Query, to retrieve posts before a given date, it appears the date is used as gmdate - which creates an unwanted gap.

Example:

My site's timezone settings is UTC+3

I do a query with 'date_query' => array('before' => '2017-01-01 10:00:00') then I get results from prior to 2017-01-01 07:00:00.

Looking at wp-includes/date.php, I can see you already suspected this may be an issue:

if ( ! is_array( $datetime ) ) {
    // @todo Timezone issues here possibly
    return gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) );
}

So, yes... there are timezone issues there for sure :D

I tried various ways to sort this out, with no luck. Both our server and wordpress options are correctly set up. So it's just these kind of queries -- and revisions suffer similarly -- from a distortion of the actual date to return.

Thank you,

Bira

Attachments (3)

build-mysql-datetime.patch (4.1 KB) - added by Rarst 5 years ago.
Needs wp_timezone() merged.
build-mysql-datetime1.patch (3.9 KB) - added by Rarst 5 years ago.
Hardcoded more of the unit test and bumped delta so less likely to fail when running slow.
41782.diff (5.4 KB) - added by SergeyBiryukov 5 years ago.

Download all attachments as: .zip

Change History (18)

#1 @birgire
7 years ago

What happens when you use an array of an array and skip the seconds:

'date_query' => array( array( 'before' => '2017-01-01 10:00' ) )

It's interesting to see that $datetime string in WP_Date_Query::build_mysql_datetime()

https://github.com/WordPress/WordPress/blob/bbb8d48086b7d10908f4fda673585ee122f2851d/wp-includes/date.php#L856

is matched for 'Y', 'Y-m', 'Y-m-d', 'Y-m-d H:i' but not 'Y-m-d H:i:s' before passed on to the gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ) part for non-matches.

Last edited 7 years ago by birgire (previous) (diff)

#2 @Biranit
7 years ago

@birgire 'before' either takes any strtotime string, or YEAR MONTH DAY variables. So for strtotime, there's no difference really between with/without seconds.

#3 @birgire
7 years ago

If the $datetime string has the format 'Y', 'Y-m', 'Y-m-d' or 'Y-m-d H:i' then it will skip:

gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) )

So '2017-01-01 10:00' should skip it too and get the seconds part from the

$datetime['second'] = ( $default_to_max ) ? 59 : 0;

But on the other hand '2017-01-01 10:00:00' does not skip it.

#4 @Biranit
7 years ago

@birgire interesting idea. Just tried it - unfortunately it doesn't make a difference :(

#5 @birgire
7 years ago

When I play with:

$obj = new WP_Date_Query();
echo $obj->build_mysql_datetime( $datetime = '2017-01-01 10:00' );

then $datetime becomes an array, within that method:

Array
(
    [year] => 2017
    [month] => 1
    [day] => 1
    [hour] => 10
    [minute] => 0
)

But with

$obj = new WP_Date_Query();
echo $obj->build_mysql_datetime( $datetime = '2017-01-01 10:00:00' );

then $datetime remains a string.

#6 @Biranit
7 years ago

@birgire actually you are absolutely right! I tinkered with the query and ensured it only uses Y-m-d H:i -- and indeed that works...

So many thanks for the easy fix to my code.

But surely this has to be addressed by WordPress anyway?

#7 @birgire
7 years ago

Glad to hear that worked for you.

After some tests I found out that the second input argument to strtotime() does not always have effect.

It was reported long time ago here:

https://bugs.php.net/bug.php?id=18177

but it's probably just a feature of that function.

I did some testing and found out that it doesn't have effect on these examples:

echo strtotime("1 January 2017" ) . PHP_EOL;
echo strtotime("1 January 2017", time() + 3600 ) . PHP_EOL;

echo strtotime("2017-01-01" ) . PHP_EOL;
echo strtotime("2017-01-01", time() + 3600 ) . PHP_EOL;

echo strtotime("2017-01-01 10:00:00" ) . PHP_EOL;
echo strtotime("2017-01-01 10:00:00", time() + 3600 ) . PHP_EOL;

as they all display all the same value, but it seems to have an effect for relative dates, e.g.

echo strtotime("last week" ) . PHP_EOL;
echo strtotime("last week", time() + 3600 ) . PHP_EOL;

echo strtotime("-3 days" ) . PHP_EOL;
echo strtotime("-3 days", time() + 3600 ) . PHP_EOL;

I didn't find any mention of this here:

http://php.net/manual/en/function.strtotime.php

We notice that the fallback in build_mysql_datetime() uses:

strtotime( $datetime, $now )

where $now is time() + get_option( 'gmt_offset' ) + HOUR_IN_SECONDS.

Maybe we should consider patching a support for the Y:m:d H:i:s match?

Last edited 7 years ago by birgire (previous) (diff)

#8 @birgire
7 years ago

If I understand it correctly WordPress sets the PHP timezone to UTC in the wp-settings.php file:

// WordPress calculates offsets from UTC.
date_default_timezone_set( 'UTC' );

and uses the gmt_offset option to calculate the offset from it.

I will have to look better into how the date_default_timezone_set() call in the /wp-admin/options-general.php file, affects it. But I it looks like it's only to display the localtime settings and is restored back to UTC soon after.

Trying to understand better how WP_Date_Query::build_mysql_datetime() handles string dates, I ran this test function:

function dq_test_info( $datetime, $gmt_offset )
{
	$time = time();
	$now = $time + $gmt_offset * 3600;

	print PHP_EOL;
	printf( "\$datetime: %s" 						. PHP_EOL, $datetime );
	printf( "date_default_timezone_get(): %s" 				. PHP_EOL, date_default_timezone_get() );
	printf( "\$gmt_offset: %s" 						. PHP_EOL, $gmt_offset );
	printf( "time(): %s" 							. PHP_EOL, $time );
	printf( "\$now = time() + \$gm_offset * 3600: %s" 			. PHP_EOL, $now );
	printf( "strtotime( \$datetime ): %s" 					. PHP_EOL, strtotime( $datetime ) );
	printf( "strtotime( \$datetime, \$now ): %s" 				. PHP_EOL, strtotime( $datetime, $now ) );
	printf( "date( 'Y-m-d H:i:s', strtotime( \$datetime ) ): %s" 		. PHP_EOL, date( 'Y-m-d H:i:s', strtotime( $datetime ) ) );
	printf( "date( 'Y-m-d H:i:s', strtotime( \$datetime, \$now ) ): %s" 	. PHP_EOL, date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ) );
	printf( "gmdate( 'Y-m-d H:i:s', strtotime( \$datetime, \$now ) ): %s" 	. PHP_EOL, gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ) );
}

Test 1

$datetime = '2017-01-01 10:00:00';

dq_test_info( $datetime, $gmt_offset = 0 );
dq_test_info( $datetime, $gmt_offset + 1 );
dq_test_info( $datetime, $gmt_offset + 2 );

with output:

$datetime: 2017-01-01 10:00:00
date_default_timezone_get(): UTC
$gmt_offset: 0
time(): 1504456206
$now = time() + $gm_offset * 3600: 1504456206
strtotime( $datetime ): 1483264800
strtotime( $datetime, $now ): 1483264800
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-01-01 10:00:00
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-01-01 10:00:00
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-01-01 10:00:00

$datetime: 2017-01-01 10:00:00
date_default_timezone_get(): UTC
$gmt_offset: 1
time(): 1504456206
$now = time() + $gm_offset * 3600: 1504459806
strtotime( $datetime ): 1483264800
strtotime( $datetime, $now ): 1483264800
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-01-01 10:00:00
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-01-01 10:00:00
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-01-01 10:00:00

$datetime: 2017-01-01 10:00:00
date_default_timezone_get(): UTC
$gmt_offset: 2
time(): 1504456206
$now = time() + $gm_offset * 3600: 1504463406
strtotime( $datetime ): 1483264800
strtotime( $datetime, $now ): 1483264800
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-01-01 10:00:00
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-01-01 10:00:00
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-01-01 10:00:00

Test 2

$datetime = '-3 days';

dq_test_info( $datetime, $gmt_offset = 0 );
dq_test_info( $datetime, $gmt_offset + 1 );
dq_test_info( $datetime, $gmt_offset + 2 );

with output:

$datetime: -3 days
date_default_timezone_get(): UTC
$gmt_offset: 0
time(): 1504456206
$now = time() + $gm_offset * 3600: 1504456206
strtotime( $datetime ): 1504197006
strtotime( $datetime, $now ): 1504197006
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:30:06
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 16:30:06
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 16:30:06

$datetime: -3 days
date_default_timezone_get(): UTC
$gmt_offset: 1
time(): 1504456206
$now = time() + $gm_offset * 3600: 1504459806
strtotime( $datetime ): 1504197006
strtotime( $datetime, $now ): 1504200606
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:30:06
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 17:30:06
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 17:30:06

$datetime: -3 days
date_default_timezone_get(): UTC
$gmt_offset: 2
time(): 1504456206
$now = time() + $gm_offset * 3600: 1504463406
strtotime( $datetime ): 1504197006
strtotime( $datetime, $now ): 1504204206
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:30:06
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 18:30:06
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 18:30:06

@Biranit, it would be interesting to see how it runs on your install.

Version 6, edited 7 years ago by birgire (previous) (next) (diff)

#9 @jave.web
6 years ago

The codex and reference states this

If you want a before date to be inclusive, include the time as well, such as
'before' => '2013-02-28 23:59:59'

You would think your provided time is final, but no, it actually shifts the time. This should NOT happen, your words should be the final words.

If you provide it separately year/month/day/hour/minute/second it is (correctly) not altered (probably because of how the query is created).

The design problem is in build_mysql_datetime() method:

if ( ! is_array( $datetime ) ) {
  // @todo Timezone issues here possibly
  return gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) );
}

Programmer rather added @todo instead of just replacing gmdate() with just date(), because gmdate() is affected by date_default_timezone_set(), date() is not...

#10 @jave.web
6 years ago

There is also another issue in this part:

DATE_FORMAT( POST_DATE, '%H.%i%s' ) <= CALCULATED

Because for e.g. '2018-11-05 20:56:00' it will produce something like

20.5600 <= 8.274500

Therefore, you would only get posts or whatever only in a part of the day...

@Rarst
5 years ago

Needs wp_timezone() merged.

#11 follow-up: @Rarst
5 years ago

  • Keywords has-patch has-unit-tests added

I couldn't reproduce timezone shift as described, but there are definitely time zone issues in there.

Attached a patch that improves unit tests coverage for different cases and handling of arbitrary input.

This ticket was mentioned in Slack in #core-datetime by rarst. View the logs.


5 years ago

#13 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 5.3
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

@Rarst
5 years ago

Hardcoded more of the unit test and bumped delta so less likely to fail when running slow.

@SergeyBiryukov
5 years ago

#14 in reply to: ↑ 11 @SergeyBiryukov
5 years ago

Replying to Rarst:

I couldn't reproduce timezone shift as described, but there are definitely time zone issues in there.

Attached a patch that improves unit tests coverage for different cases and handling of arbitrary input.

Thanks for the patch!

I couldn't get phpunit --group date to pass with build-mysql-datetime1.patch:

There were 2 failures:                                                                                                                                                                                                                                                                                                  

1) Tests_Date_I18n::test_should_format_date                                                                                                                 
The dates should be equal                                                                                                                                   
Failed asserting that 1566453248 matches expected 1566442448.                                                                                                                                                                                                                                                           

S:\home\wordpress.test\develop\tests\phpunit\tests\date\dateI18n.php:9                                                                                                                                                                                                                                                  

2) Tests_WP_Date_Query::test_build_mysql_datetime with data set #0 ('-1 day', '2019-08-21 05:53:57')                                                        
Expected 2019-08-21 05:53:57, got 2019-08-21 05:54:11                                                                                                       
Failed asserting that 1566366851 matches expected 1566366837.                                                                                                                                                                                                                                                           

S:\home\wordpress.test\develop\tests\phpunit\tests\date\query.php:526                            

Apparently setting timezone_string in a data provider doesn't work as expected: it affects other tests and happens far before the test is actually executed, so the minutes and seconds don't match.

Splitting the tests into several groups in 41782.diff (general, custom timezone, relative date) seems to work.

#15 @SergeyBiryukov
5 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 45876:

Date/Time: Use wp_timezone() in WP_Date_Query::build_mysql_datetime() to address timezone issues.

Improve unit test coverage.

Props Rarst, Biranit, birgire, jave.web, SergeyBiryukov.
Fixes #41782.

Note: See TracTickets for help on using tickets.