WordPress.org

Make WordPress Core

#20942 closed defect (bug) (fixed)

get_gmt_from_date() throws exceptions

Reported by: mdawaffe Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: Date/Time Keywords: has-patch dev-feedback
Focuses: Cc:

Description

As of PHP 5.3, new DateTime( $string ) throws an exception if $string is unparseable. WordPress doesn't catch the exception.

We should either catch the exception or use date_create( $string ), which just returns false if $string is unparseable.

I don't know what we should return from get_gmt_from_date( $string ) if $string is unparseable. false? 1970? "0000-00-00 00:00:00"? In the branch that handles an empty $tz, WordPress usually returns something near "1999-11-30 00:00:00" when $string doesn't match the expected format.


$ php -r 'define( "WP_DEBUG_DISPLAY", true ); require "wp-load.php"; add_filter( "pre_option_timezone_string", function() { return "America/Los_Angeles"; } ); var_dump( get_gmt_from_date( "WordPress" ) );'
PHP Fatal error:  Uncaught exception 'Exception' with message 'DateTime::__construct(): Failed to parse time string (WordPress) at position 0 (W): The timezone could not be found in the database' in /Users/mdawaffe/Sites/wordpress/wp-includes/formatting.php:1872
Stack trace:
#0 /Users/mdawaffe/Sites/wordpress/wp-includes/formatting.php(1872): DateTime->__construct('WordPress')
#1 Command line code(1): get_gmt_from_date('WordPress')
#2 {main}
  thrown in /Users/mdawaffe/Sites/wordpress/wp-includes/formatting.php on line 1872

Attachments (3)

20942.patch (1020 bytes) - added by winterDev 21 months ago.
Adjusted and re-uploaded patch per suggestion from georgestephanis: "Do you need to account for a negative offset in line 1874 ? swap out '+' . $offset / 3600 . ' hours'); with ( $offset < 0 ? '-' : '+' ) . abs( $offset / 3600 ) . ' hours' ); -- perhaps? If it works as is with the + so be it, it's just my thoughts after a sixty second glance."
gmt-date-matches.diff (537 bytes) - added by wonderboymusic 20 months ago.
gmt-date-matches-refresh.diff (835 bytes) - added by wonderboymusic 19 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 scribu23 months ago

  • Keywords needs-patch added

We don't like exceptions, so date_create() it is. In my mind, returning false would make the most sense for an invalid date.

winterDev21 months ago

Adjusted and re-uploaded patch per suggestion from georgestephanis: "Do you need to account for a negative offset in line 1874 ? swap out '+' . $offset / 3600 . ' hours'); with ( $offset < 0 ? '-' : '+' ) . abs( $offset / 3600 ) . ' hours' ); -- perhaps? If it works as is with the + so be it, it's just my thoughts after a sixty second glance."

comment:2 wonderboymusic20 months ago

  • Keywords has-patch added; needs-patch removed

The RegEx already checks for proper date string, so all we need to do is wrap the if / else timezone code in if ( $matches ) { .... }. I am not in love with the procedural aliases PHP offers.

Attached a patch

Last edited 20 months ago by wonderboymusic (previous) (diff)

comment:3 wonderboymusic20 months ago

actually, it's way simpler than that - updated. Now just have to decide if null is cool - or false or some other time should be returned.

Last edited 20 months ago by wonderboymusic (previous) (diff)

comment:4 nacin20 months ago

  • Milestone changed from Awaiting Review to 3.5

comment:5 nacin19 months ago

If you pass an invalidate date (like month 13), the regular expression will be fine but the exception will be thrown.

There's no need for us to convert all of this to procedural aliases. Just the creation. So:

$datetime = date_create( $string );
if ( ! $datetime )
    return something;
 $datetime->setTimezone( new DateTimeZone('UTC') );

comment:6 nacin19 months ago

  • Keywords needs-patch added; has-patch removed

Of course, if the regex fails first, we can bail just the same.

comment:7 wonderboymusic19 months ago

  • Keywords has-patch dev-feedback added; needs-patch removed

Refreshed the patch - caveat: every place that uses this in core expects a date to be returned and possibly filtered. Also have to respect the 'format' arg. I accomplish this by returning date( $format, 0 ).

Last edited 19 months ago by wonderboymusic (previous) (diff)

comment:8 nacin18 months ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 22435:

Avoid an uncaught exception in get_gmt_from_date(). The return value is imperfect - date( $format, 0 ) - but better than a fatal error. props wonderboymusic. fixes #20942.

Note: See TracTickets for help on using tickets.