Opened 11 years ago
Closed 7 years ago
#25365 closed defect (bug) (fixed)
timeAgo attribute for revisions produces the wrong result due to missing UTC in strtotime
Reported by: | biranit | Owned by: | adamsilverstein |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 3.6.1 |
Component: | Revisions | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
When looking at revisions, the date for each revision appears correctly, but the human_time_diff is wrong.
For example, on a server with timezone UTC+3, the time is now 13:28 (UTC+3), and looking a revision from 20 Sep 09:41 (UTC+3) shows:
7 hours ago (20 Sep @ 9:41)
Where in fact it should be
4 hours ago (20 Sep @ 9:41)
The post_date for this revision is: 2013-09-20 09:41:08
The post_date_gmt for this revision is: 2013-09-20 06:41:08
(The server timezone is UTC+3, as is the Wordpress option and the php.ini)
The reason for this bug is in wp-admin/includes/revision.php line 117:
$modified_gmt = strtotime( $revision->post_modified_gmt );
strtotime uses the server's timezone and therefore assumes the strong $revision->post_modified_gmt is actually in UTC+3 (the server's timezone). To avoid this and ensure the unix timestap is indeed GMT, the line should change to:
$modified_gmt = strtotime( $revision->post_modified_gmt . " UTC");
Thank you,
Biranit
Attachments (5)
Change History (32)
#3
@
11 years ago
- Summary changed from timeAgo attribute for revisions produces the wrong result due to misuse of strtotime to timeAgo attribute for revisions produces the wrong result due to missing UTC in strtotime
#4
@
11 years ago
We specifically do date_default_timezone_set( 'UTC' );
in wp-settings.php, which means I'm pretty sure strtotime() should never be GMT+3.
#5
in reply to:
↑ 2
@
11 years ago
- Resolution set to worksforme
- Status changed from new to closed
Based on @nacin's comment, I'm guessing you were experiencing some sort of server configuration issue - possibly the server time was not set correctly? In all of our testing of this feature we never saw the time difference issue you mention.
I'm going to close this ticket for now because I am unable to reproduce the problem, and it looks like a configuration error. If you are still seeing the issue, feel free to re-open the ticket and please provide some more details - steps to reproduce the issue and screenshots would be helpful.
Thanks for the report, happy to work on fixing this if I can reproduce it.
Replying to biranit:
I went through ALL instances where strtotime is used in Wordpress 3.6.1 and found two - possibly three - instances where this error should also be fixed:
1) In /wp-includes/comment.php line 2047:
if ( time() - strtotime( $posts[0]->post_date_gmt ) > ( $days_old * DAY_IN_SECONDS ) ) {should be changed to
if ( time() - strtotime( $posts[0]->post_date_gmt . " UTC" ) > ( $days_old * DAY_IN_SECONDS ) ) {2) In /wp-includes/media.php lines 1661-1662:
'date' => strtotime( $attachment->post_date_gmt ) * 1000, 'modified' => strtotime( $attachment->post_modified_gmt ) * 1000,Should be changed to
'date' => strtotime( $attachment->post_date_gmt . " UTC") * 1000, 'modified' => strtotime( $attachment->post_modified_gmt . " UTC") * 1000,2) In /wp-content/plugins/akismet/akismet.php line 385:
$last_updated = strtotime( $post->post_modified_gmt );Should be changed to
$last_updated = strtotime( $post->post_modified_gmt . " UTC");Thank you
#7
@
10 years ago
- Resolution worksforme deleted
- Status changed from closed to reopened
This issue seems unduly dismissed.
In the core code above and on trac https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/revision.php#L172 WordPress is shown to use $modified
to calculate the pretty date (dateShort
) but $modified_gmt
to show the human_time_diff()
(timeAgo
). This causes apparent time travel.
I'm guessing you were experiencing some sort of server configuration issue
This does not seem to be possible to me given the equivalence between the dates, but I'm interested in hearing how my servers might be misconfigured. Looking for answers here because I see no other way to filter these values, as they are templated on the client side.
#8
follow-up:
↓ 10
@
10 years ago
I think in my case, the issue is down to the fact that our theme's function file has the line:
date_default_timezone_set('Asia/Jerusalem');
For us, timezone functions are crucial. Everything on the server is set up for this timezone -- including php.ini, OS timezone + clock AND the hardware clock.
Yet the revisions are all off, as they appear in UTC+0 instead of UTC+3 (Asia/Jerusalem).
Personally, I still think this is a bug - or just an oversight - that should be fixed...
#9
@
10 years ago
I can confirm for you that your date_default_timezone_set('Asia/Jerusalem');
is in fact the configuration "issue" that causes this behavior. For me of course it's my own America/New_York
, needed for a 3rd party Google Analytics library on which we rely.
In a setup where WordPress is a component in a larger system, using date_default_timezone_set()
is commonplace. In fact if you don't set this you'll get the "It is not safe to rely on the system's timezone settings" warning telling you that you should use this, or "the date.timezone setting" which I presume will have a similar effect.
I've found no filters or other way around this that doesn't require core modification or client-side hackery. A proposed "fix" is can be as simple as using $modified
for the human_time_diff()
.
Removing the date_default_timezone_set()
call seems necessary but that's unacceptable in my configuration, and unfortunately the difference is somewhat undermining our user confidence.
#10
in reply to:
↑ 8
@
10 years ago
Biranit - thanks for the feedback.
In order to simplify timezone issues, WordPress always keeps track of time internally using UTC. Then, when the time is displayed it's converted the the local time that's defined in the Timezone setting on the General Settings page. The proper place to set your timezone within WordPress is under Settings->General (see http://cl.ly/image/3O3N170m2C3R).
Can you try setting your timezone there and commenting out out or deleting the date_default_timezone_set call in your functions.php file? When I tested this locally, the revision time_ago called returned the proper value.
The UTC/timezone issue has a long history that relates in part to legacy support of PHP4 - see Otto's comments here - http://wordpress.org/support/topic/why-does-wordpress-set-timezone-to-utc?replies=5#post-3749681
There is a long term effort to improve timezone handling in WordPress - if you are interested in helping out with this effort, take a look at these related tickets: #28636, #25555, #20973, #23384
Replying to Biranit:
I think in my case, the issue is down to the fact that our theme's function file has the line:
date_default_timezone_set('Asia/Jerusalem');
For us, timezone functions are crucial. Everything on the server is set up for this timezone -- including php.ini, OS timezone + clock AND the hardware clock.
Yet the revisions are all off, as they appear in UTC+0 instead of UTC+3 (Asia/Jerusalem).
Personally, I still think this is a bug - or just an oversight - that should be fixed...
#11
follow-up:
↓ 12
@
10 years ago
I can confirm that in my case, timezone is set correctly in WordPress under Settings->General. It's also set correctly in PHP using date_default_timezone_set()
.
To repeat, the problem is not that WordPress uses UTC to represent dates internally. The problem is that in one single instance feeds human_time_diff()
a UTC date for external display of a localized time.
#12
in reply to:
↑ 11
@
10 years ago
Thanks for the feedback -
When using WordPress the timezone should not be set in PHP. WordPress explicitly sets the timezone to UTC in wp-settings.php and calculates all offsets from there - see here -- setting the timezone elsewhere in PHP is confusing WordPress internally.
I tried changing the code from using modified_gmt to modified as suggested above and that does fix the revisions page if I set the timezone in PHP - otherwise the times are all off. Since setting the timezone in PHP is against recommended practice, making this change would likely have a negative affect for everyone who doesn't have this set.
Where and why are you setting the timezone in your PHP with date_default_timezone_set? can you try removing that set statement to see if the issue is resolved? Setting the timezone in WordPress should be sufficient.
If you absolutely have to set the timezone for a block of PHP, try setting it back to UTC at the end of the block, so WordPress internal calculations are not affected.
Hope that helps.
Replying to buley:
I can confirm that in my case timezone is set correctly in WordPress under Settings->General. It's also set correctly in PHP using
date_default_timezone_set()
.
To repeat, the problem is not that WordPress uses UTC to represent dates internally. The problem is that in one single instance feeds
human_time_diff()
a UTC date for external display of a localized time.
#14
@
10 years ago
I'm really sorry I've not replied before -- it's been a little difficult around here, recently...
I just wanted to say: it's not realistic to prevent PHP settings of timezone. In fact, on a PHP install without setting timezone, PHP now issues a warning. So most sysadmins will likely set up a timezone.
Furthermore, on shared servers, the local directive for PHP timezone is very important: it affects error log files, for example; and other non-WordPress scripts.
WordPress should be self-contained, in my opinion, and this is really a very simple thing to resolve to ensure there is no discrepancy in times. Fact is, this issue does NOT occur anywhere other than revisions. So why not just solve this simply by adding that . " UTC" bit to those functions?
Many thanks,
Bira
#15
@
10 years ago
Those are my exact feelings as well, Bira. And I also apologize for my delay in response.
We use WordPress as the heart of a larger PHP-based platform. It seems like a shame to "force" what seems like WordPress-specific need on the rest of our codebase -- especially since we're talking about a single parameter on a single function. But, following adamsilveerstin's very kind advice, that's what we had to do in order to prevent our revision dates from time-traveling because we are unwilling to hack core even for a revision time.
#16
@
10 years ago
- Keywords has-patch dev-feedback added
25365.diff includes the suggested fixes from Bira. Bira, can you see if this patch resolves your issue?
I only applied the UTC fix to the revisions code so we stay within the scope of this ticket; The issue deserves more attention in a new ticket: scanning core I see several other uses of strtotime on gmt values, some with no appended timezone, some with ' GMT', some with '+0000'
#17
@
10 years ago
correction, 25365.2.diff is limited to revisions
#19
in reply to:
↑ 18
@
10 years ago
You are welcome. I will defer to the lead developers to decide if this is a fix that belongs in core.
Thanks for testing and raising the issue!
Replying to Biranit:
Works perfect, adamsilverstein :)
Many thanks!
#20
@
10 years ago
In 25365.3.diff: refresh against trunk.
#21
@
8 years ago
- Owner set to adamsilverstein
- Status changed from reopened to assigned
25365.4.diff refresh against trunk
#23
follow-up:
↓ 24
@
7 years ago
I would like to revive this ticket. On a completely clean install, with no intervention in WordPress's timezone management, the revisions continue to show the wrong time (adding 3 hours to them).
Time on server is UTC+3
Timezone set in WordPress options is Jerusalem.
No other time is set in functions.php or elsehwhere.
And yet a revision from 15 minutes ago displays as being from 3 hours and 15 minutes ago; a revision from 1 hour ago shows as being from 4 hours ago, etc.
Can't this be fixed? it's such a simple fix and it's been this way for four years now.
#24
in reply to:
↑ 23
@
7 years ago
- Milestone changed from Awaiting Review to 4.9
Replying to Biranit:
Can't this be fixed? it's such a simple fix and it's been this way for four years now.
Let's do it!
#25
@
7 years ago
- Keywords commit added
Checking in core, there are several places where we handle gmt dates and add ' +0000'. 25365.5.diff switches to this app[roach, which should be the same as using UTC.
I went through ALL instances where strtotime is used in Wordpress 3.6.1 and found two - possibly three - instances where this error should also be fixed:
1) In /wp-includes/comment.php line 2047:
should be changed to
2) In /wp-includes/media.php lines 1661-1662:
Should be changed to
2) In /wp-content/plugins/akismet/akismet.php line 385:
Should be changed to
Thank you