WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 3 months ago

#16495 new defect (bug)

Make iso8601_to_datetime a bit more compliant

Reported by: chrisscott Owned by:
Milestone: Future Release Priority: low
Severity: minor Version: 3.0.5
Component: Formatting Keywords: needs-patch
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 3 years ago.
16495.test.diff (2.1 KB) - added by ericmann 6 months ago.
Add some tests listing iso8601 dates and what they should be parsed to.

Download all attachments as: .zip

Change History (7)

chrisscott3 years ago

comment:1 solarissmoke3 years ago

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

comment:2 dd328 months 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.

comment:3 rmccue8 months 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.

ericmann6 months ago

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

comment:4 ericmann6 months 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).

comment:5 nacin3 months 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.

Note: See TracTickets for help on using tickets.