Make WordPress Core

Opened 10 years ago

Last modified 22 months ago

#26798 new defect (bug)

While inserting a post some values for 'post_date' throw a PHP exception

Reported by: mobius5150's profile mobius5150 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Date/Time Keywords: has-patch has-unit-tests changes-requested
Focuses: Cc:

Description

If you try and insert a post with a date that does not have leading zeros (eg 2012-01-8 or 2012-1-08 or 2012-1-8) a PHP error is thrown in trunk/src/wp-includes/functions.php#L4015: A non well formed numeric value encountered due to the checkdate() call.

Minimum code to reproduce:

$post_data = array(
    'post_title' => 'some title',
    'post_content' => 'some content',
    'post_status' => 'publish',
    'post_date' => '2012-01-8 12:00:00',
);

wp_insert_post( $post_data );

Two possible solutions:

  1. Get strict about date formatting: ie: require leading zeros. If a date comes in without leading zeros a WP_Error() should be returned, an error should not get thrown.
  2. Allow non-leading zeros.

Either way using substr() in trunk/src/wp-includes/post.php#L2805 is at fault.

I've attached a diff that uses a regex to parse the date using capture groups. Its much cleaner and will detect a badly formatted date and handle it gracefully. The regex does not require leading zeros. If leading zeros are required the regex should be changed to:
/^(?P<year>\d{4})-(?P<month>0[1-9]|1[012])-(?P<day>0[1-9]|[12][0-9]|3[01])/

Attachments (6)

post.php.diff (989 bytes) - added by mobius5150 10 years ago.
Diff for suggested changes to fix post.php
post.php.2.diff (990 bytes) - added by mobius5150 10 years ago.
26798.3.diff (2.8 KB) - added by johnregan3 22 months ago.
Refresh of the patch and Unit Tests
26798.4.diff (4.3 KB) - added by johnregan3 22 months ago.
Update to support ISO format and use data provider for testing
26798.5.diff (7.0 KB) - added by johnregan3 22 months ago.
Improved date validation & tests
26798.6.diff (7.0 KB) - added by johnregan3 22 months ago.
Minor improvement. Label regex groups as was originally implemented by reporter.

Download all attachments as: .zip

Change History (17)

@mobius5150
10 years ago

Diff for suggested changes to fix post.php

#1 @mobius5150
10 years ago

Fixed a typo. The match array should have at least 6 elements not exactly 6.

#2 @nacin
10 years ago

  • Component changed from Validation to Date/Time

Perhaps wp_checkdate() should be able to either:

  • take a string and break it up on its own
  • decline to pass obviously invalid values to checkdate()

#3 @johnbillion
10 years ago

  • Keywords has-patch needs-testing added
  • Version changed from trunk to 3.5

Introduced in r21921

#4 @alordiel
9 years ago

  • Keywords needs-testing removed

Patch works as charm (WP 4.1), no errors or notices where thrown.

#5 @desrosj
4 years ago

  • Keywords needs-refresh needs-unit-tests added
  • Milestone set to Future Release

Did some testing today on the latest trunk (currently 5.6-alpha) and this still occurs.

The patch needs a refresh, and the feedback in comment:2 needs to be explored. A unit test demonstrating the failure will also help validate any proposed fixes.

@johnregan3
22 months ago

Refresh of the patch and Unit Tests

#6 follow-up: @desrosj
22 months ago

  • Keywords has-unit-tests changes-requested added; needs-refresh needs-unit-tests removed

THanks @johnregan3!

Could you adjust the test_wp_resolve_post_date_regex() test to use a data provider instead?

#7 in reply to: ↑ 6 @johnregan3
22 months ago

Replying to desrosj:

THanks @johnregan3!

Could you adjust the test_wp_resolve_post_date_regex() test to use a data provider instead?

Will do. Also, I'm updating my patch. I just found a bug during further testing. Apologies for that.

@johnregan3
22 months ago

Update to support ISO format and use data provider for testing

#8 @johnregan3
22 months ago

@desrosj In testing, I found that the WP_REST tests use ISO formatted dates, so I added support for that. Additionally, this now handles YYYY-MM-DD dates differently from YYYY-MM-DD H:i:s so that malformed times (when included) do not cause issues. Finally, the tests are now using a data provider.

One last addition over the previous patch: I added intval() validation to wp_checkdate() to help prevent PHP Errors when invalid data is passed into it. This addresses comment:2 suggestion to "decline to pass invalid values to checkdate()"

#9 @johnregan3
22 months ago

To confirm: I've run the entire set of WP PHP Unit tests on this code.

@johnregan3
22 months ago

Improved date validation & tests

#10 @johnregan3
22 months ago

I discovered some date formats were, in fact, failing (Namely RFC3339 and RSS formats), so I simplified the regex being used, and improved the validation inside of wp_checkdate().

Part of the issue with the previous patch was it added validation for time, which isn't the emphasis of wp_resolve_post_date() nor wp_checkdate(), so I removed that time validation.

@johnregan3
22 months ago

Minor improvement. Label regex groups as was originally implemented by reporter.

#11 @johnregan3
22 months ago

I'm leaving this ticket alone now. Thanks for enduring the updates.

Note: See TracTickets for help on using tickets.