Opened 8 years ago
Closed 6 years ago
#38940 closed defect (bug) (duplicate)
current_time() expects that date_default_timezone_set is never used
Reported by: | marco.marsala | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 4.6.1 |
Component: | Date/Time | Keywords: | has-patch dev-feedback 2nd-opinion |
Focuses: | Cc: |
Description (last modified by )
Currently, the function current_time()
expects date_default_timezone_set()
is never used.
Inspecting the code of current_time()
confirmed this, because it formats the result using date()
function (that is affected by timezone).
In fact seems WordPress always resets the timezone to UTC; an echo date_default_timezone_get();
placed at top and at bottom of wp-config.php in a clean WordPress setup will prove this.
So using date_default_timezone_set()
anywhere in theme or plugin code will broke current_time()
.
Attachments (1)
Change History (12)
#3
@
8 years ago
- Keywords has-patch added
A possible code fix would be:
<?php function current_time( $type, $gmt = 0 ) { $oldgmt = date_default_timezone_get(); date_default_timezone_set('UTC'); switch ( $type ) { case 'mysql': return ( $gmt ) ? gmdate( 'Y-m-d H:i:s' ) : gmdate( 'Y-m-d H:i:s', ( time() + ( get_option( 'gmt_offset' ) * HOUR_IN_SECONDS ) ) ); case 'timestamp': return ( $gmt ) ? time() : time() + ( get_option( 'gmt_offset' ) * HOUR_IN_SECONDS ); default: return ( $gmt ) ? date( $type ) : date( $type, time() + ( get_option( 'gmt_offset' ) * HOUR_IN_SECONDS ) ); } date_default_timezone_set($oldgmt); }
#4
@
8 years ago
- Keywords dev-feedback 2nd-opinion added
I'd say it is more of a documentation problem than a code problem. The code has been this way for a long time, and I don't know if there have been any complaints about bugs arising from code outside WordPress core calling date_default_timezone_set()
. It would affect a lot of time-related stuff in WordPress, since date()
is used many other places where gmdate()
would need to be used instead. Ensuring that we always use gmdate()
when getting UTC time would be an extra precaution, but it wouldn't immediately make it OK for code to call date_default_timezone_set()
, because a lot of plugins would still be assuming date()
to be UTC. So, for backward-compatibility reasons, I doubt that we should change the code. Not unless there actually is a significant specific use-case for making WordPress core resilient against calls to date_default_timezone_set()
.
#5
follow-up:
↓ 6
@
8 years ago
I didn't suggest to change date()
into gmdate()
.
The proposed patch would affect only the bcurrent_time()
and has no side effect on date()
function outside current_time()
calls, look at the code above.
#6
in reply to:
↑ 5
;
follow-up:
↓ 7
@
8 years ago
Replying to marco.marsala:
I didn't suggest to change
date()
intogmdate()
.
The proposed patch would affect only the b
current_time()
and has no side effect ondate()
function outsidecurrent_time()
calls, look at the code above.
I understand that, but if some code outside of WordPress core uses date_default_timezone_set()
, it will affect every call to date()
in WordPress (and plugins), not just the current_time()
function. current_time()
is not used everywhere that WordPress retrieves the current date/time, so other places in the code will still be affected by the changed timezone, even though they are expected to be getting the UTC time. They would need to be updated to use current_time()
or gmdate()
in that case.
In fact, you could avoid changing the timezone in your patch for current_time()
, you could just update it to use gmdate()
instead of date()
, I think.
#7
in reply to:
↑ 6
@
8 years ago
Replying to jdgrimes:
Replying to marco.marsala:
I didn't suggest to change
date()
intogmdate()
.
The proposed patch would affect only the b
current_time()
and has no side effect ondate()
function outsidecurrent_time()
calls, look at the code above.
I understand that, but if some code outside of WordPress core uses
date_default_timezone_set()
, it will affect every call todate()
in WordPress (and plugins), not just thecurrent_time()
function.current_time()
is not used everywhere that WordPress retrieves the current date/time, so other places in the code will still be affected by the changed timezone, even though they are expected to be getting the UTC time. They would need to be updated to usecurrent_time()
orgmdate()
in that case.
It is what I'm saying. The code should use current_time()
, but current_time()
is bugged if date_default_timezone_set()
is used. The patch aim to fix current_time()
without adding side effects.
In fact, you could avoid changing the timezone in your patch for
current_time()
, you could just update it to usegmdate()
instead ofdate()
, I think.
You're completely right.
#8
@
8 years ago
Ticket description gets scope slightly wrong. current_time()
is not broken if PHP timezone is changed, it is just broken altogether in context
<?php var_dump( date_default_timezone_get() ); // "UTC" var_dump( date('D, j M Y H:i:s O') ); // "Tue, 2 May 2017 11:38:08 +0000" << correct UTC var_dump( current_time( 'D, j M Y H:i:s O' ) ); // "Tue, 2 May 2017 14:38:08 +0000" << local time, invalid time zone var_dump( current_time( 'D, j M Y H:i:s O', true ) ); // "Tue, 2 May 2017 11:38:08 +0000" << correct UTC
The logic of feeding "WP timestamp" into date()
is just inherently broken and cannot produce correct output.
Effectively any format with timezone will produce invalid results, unless gmt
flag is set.
I am against the suggested patch, this is not PHP API problem and shouldn't be approached as one.
#9
@
8 years ago
The more I work with dates and times in WordPress, the more I think that its date and time API needs to be completely reworked. It is usable to a certain degree when working with GMT times, but if you want to do something relative to the site's timezone, it does just about everything wrong. The method of converting between timezones in current_time()
, for example, always uses GMT offsets, instead of actual timezones (meaning that if a timezone observes DST, or has any other quirks, things won't come out correctly). What needs to really happen is using PHP's datetime API, instead of all of these hacks. Probably current_time()
needs to retain current behavior for back-compat, be deprecated, and a new helper function introduced. But that would really be for a separate ticket.
I'm unsure if this is a documentation fault or a code fault. This limitation should at least be mentioned in documentation (documentation fault) or the code fixed (code fault).