Make WordPress Core

Opened 13 years ago

Last modified 5 years ago

#16495 new defect (bug)

Make iso8601_to_datetime a bit more compliant

Reported by: chrisscott's profile chrisscott Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0.5
Component: Formatting Keywords: needs-patch needs-refresh needs-testing
Focuses: Cc:

Description

With a valid ISO 8601 date string with dashes in the date or decimal fractions for seconds, this function will fail to parse it correctly. The attached patch updates the regex to optionally accept dashes and decimal fractions for seconds (which are ignored). It also uses a variable for the regex since it was duplicated.

Attachments (2)

formatting.php.patch (1.1 KB) - added by chrisscott 13 years ago.
16495.test.diff (2.1 KB) - added by ericmann 10 years ago.
Add some tests listing iso8601 dates and what they should be parsed to.

Download all attachments as: .zip

Change History (8)

#1 @solarissmoke
13 years ago

  • Component changed from General to Formatting
  • Keywords has-patch added

#2 @dd32
11 years ago

  • Keywords needs-unit-tests added

This function doesn't handle 90% of the iso8601 formats it seems, for example, it chokes on something as basic as '2013-08-09T00:31:38Z' (even with the patch).

This needs serious unit tests and a patch that handles it as such.

#3 @rmccue
11 years ago

Whoa, the hell is this function? Here's my take from the REST API, which handles RFC3339 (a subset of ISO8601 that doesn't include all the unnecessary bits) which is usually used on the 'net. I'd be happy to build off that, but this giant regex seems unnecessary.

@ericmann
10 years ago

Add some tests listing iso8601 dates and what they should be parsed to.

#4 @ericmann
10 years ago

  • Keywords needs-patch 2nd-opinion added; has-patch needs-unit-tests removed

Added a test function that lists several valid iso8601 dates and the mysql date they should be parsed to. Any patches submitted to this ticket will need to address these formats and pass the test (current implementation and existing patch don't quite meet that criteria).

#5 @nacin
10 years ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to Future Release
  • Priority changed from normal to low
  • Severity changed from normal to minor

Certainly would be nice to fix this. Thanks for the tests, ericmann.

#6 @chriscct7
8 years ago

  • Keywords needs-refresh needs-testing added
  • Priority changed from low to normal
  • Severity changed from minor to normal
Note: See TracTickets for help on using tickets.