WordPress.org

Make WordPress Core

Opened 23 months ago

Last modified 7 weeks ago

#41782 reviewing defect (bug)

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 (2)

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

Download all attachments as: .zip

Change History (15)

#1 @birgire
23 months 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 23 months ago by birgire (previous) (diff)

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

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

#5 @birgire
23 months 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
23 months 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
23 months ago

Glad to hear that worked for you.

Yes this was just a way around the problem.

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

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.

I think we should at least patch it to support the Y:m:d H:i:s match.

Version 0, edited 23 months ago by birgire (next)

#8 @birgire
23 months 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 23 months ago by birgire (previous) (diff)

#9 @jave.web
9 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
9 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
7 weeks ago

Needs wp_timezone() merged.

#11 @Rarst
7 weeks 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.


7 weeks ago

#13 @SergeyBiryukov
7 weeks ago

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

@Rarst
7 weeks ago

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

Note: See TracTickets for help on using tickets.