WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 4 weeks ago

#41782 closed defect (bug) (fixed)

Using date_query with 'before' in WP_Query returns wrong timezone

Reported by: Biranit Owned by: 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 4 months ago.
Needs wp_timezone() merged.
build-mysql-datetime1.patch (3.9 KB) - added by Rarst 3 months 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 4 weeks ago.

Download all attachments as: .zip

Change History (18)

#1 @birgire
2 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 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'.

Version 0, edited 2 years ago by birgire (next)

#2 @Biranit
2 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
2 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
2 years ago

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

#5 @birgire
2 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
2 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
2 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 2 years ago by birgire (previous) (diff)

#8 @birgire
2 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 = 0 )
{
	$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 ) ): %s" 		. PHP_EOL, gmdate( 'Y-m-d H:i:s', strtotime( $datetime ) ) );
	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(): 1504457440
$now = time() + $gm_offset * 3600: 1504457440
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 ) ): 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(): 1504457440
$now = time() + $gm_offset * 3600: 1504461040
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 ) ): 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(): 1504457440
$now = time() + $gm_offset * 3600: 1504464640
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 ) ): 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(): 1504457440
$now = time() + $gm_offset * 3600: 1504457440
strtotime( $datetime ): 1504198240
strtotime( $datetime, $now ): 1504198240
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:50:40
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 16:50:40
gmdate( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:50:40
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 16:50:40

$datetime: -3 days
date_default_timezone_get(): UTC
$gmt_offset: 1
time(): 1504457440
$now = time() + $gm_offset * 3600: 1504461040
strtotime( $datetime ): 1504198240
strtotime( $datetime, $now ): 1504201840
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:50:40
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 17:50:40
gmdate( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:50:40
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 17:50:40

$datetime: -3 days
date_default_timezone_get(): UTC
$gmt_offset: 2
time(): 1504457440
$now = time() + $gm_offset * 3600: 1504464640
strtotime( $datetime ): 1504198240
strtotime( $datetime, $now ): 1504205440
date( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:50:40
date( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 18:50:40
gmdate( 'Y-m-d H:i:s', strtotime( $datetime ) ): 2017-08-31 16:50:40
gmdate( 'Y-m-d H:i:s', strtotime( $datetime, $now ) ): 2017-08-31 18:50:40

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

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

#9 @jave.web
10 months 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
10 months 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
4 months ago

Needs wp_timezone() merged.

#11 follow-up: @Rarst
4 months 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.


4 months ago

#13 @SergeyBiryukov
3 months ago

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

@Rarst
3 months ago

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

@SergeyBiryukov
4 weeks ago

#14 in reply to: ↑ 11 @SergeyBiryukov
4 weeks 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
4 weeks 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.