#26798 closed defect (bug) (fixed)
While inserting a post some values for 'post_date' throw a PHP exception
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.9 | Priority: | normal |
| Severity: | normal | Version: | 3.5 |
| Component: | Date/Time | Keywords: | has-patch has-unit-tests |
| 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:
- 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.
- 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)
Change History (32)
#2
@
12 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
@
12 years ago
- Keywords has-patch needs-testing added
- Version changed from trunk to 3.5
Introduced in r21921
#4
@
11 years ago
- Keywords needs-testing removed
Patch works as charm (WP 4.1), no errors or notices where thrown.
#5
@
5 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.
#6
follow-up:
↓ 7
@
3 years 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
@
3 years 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.
#8
@
3 years 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()"
#10
@
3 years 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.
This ticket was mentioned in PR #8400 on WordPress/wordpress-develop by @pbearne.
10 months ago
#12
- Scope of
$checkdate: The original patch had a subtle bug. The$checkdatevariable was declared *inside* theifstatement. This meant that if theifcondition was false (e.g., non-numeric inputs),$checkdatewould never be assigned a value, leading to a PHP warning about an undefined variable.- The fix is to initialize
$checkdateoutside theifstatement so that it is always defined. - added some comments about the original bug and what was causing it.
- The fix is to initialize
-
test_wp_insert_post_handle_malformed_post_date()inpost.php:- Clarity and Comments: Added comments to clarify the purpose of the test, what it's checking, and how it works.
- Added comments that the array of data is to be sent to
wp_insert_post() - added comments about what the result should be and how the test works.
- Correct Logic: The original test was correctly designed to check for failed inserts when dates were invalid.
- added a comment about what it being zero indicates, and that it being not zero indicates something else.
- added comments indicating the true/false for result, and what is being checked.
-
data_wp_insert_post_handle_malformed_post_date()inpost.php:- Comprehensive Test Cases: This data provider is excellent. It covers a wide range of valid and invalid date formats, including:
- Basic
YYYY-MM-DD -
YYYY-MM-DD HH:MM:SS - ISO 8601 with and without timezone offsets
- RFC 3339
- RSS
- Leap years (valid and invalid)
- Malformed dates with incorrect separators, missing parts, invalid months/days, etc.
- Basic
- Clear Organization: The test cases are well-organized and clearly labeled.
- added a comment explaining why 2012-31-08 00:00:00 is failing.
- added comment to the data provider to say that it is a data provider.
- Comprehensive Test Cases: This data provider is excellent. It covers a wide range of valid and invalid date formats, including:
-
test_wp_resolve_post_date_regex()anddata_wp_resolve_post_date_regex():- Focused Testing: These are specifically designed to test the date format *regex* used in
wp_resolve_post_date(). This is a great way to isolate and verify this part of the code. - Comprehensive Regex Tests: This data provider thoroughly checks the regex against valid and invalid date formats.
- added a comment to clarify what the emphasis of the test is, and what the regex is checking.
- added comments to clarify that this is the test for
wp_resolve_post_date() - added comment clarifying that the regex checks the format
- added comments to the data provider about what it is.
- Focused Testing: These are specifically designed to test the date format *regex* used in
-
wp_resolve_post_date()inpost.php:- Regex-Based Validation: The original code used
substr()and type casting, which was less robust. The change to usepreg_match()with a specific regular expression is much better. - Clearer Logic: The
if (empty...check combined with the count makes sure the date matches what is expected, and nothing else. - Early Exit: The
return falseif the date is invalid makes the code efficient. -
wp_checkdate()call:wp_checkdate()now gets the values from the $matches array, for better handling. - added comments to clarify the original logic, the improvements and what the code is doing.
- added comments to explain that the month check is now a regex.
- added comments to clarify that the count being less than 4, is when it should fail.
- Regex-Based Validation: The original code used
Overall:
The improved code is much more robust, readable, and testable. The use of regular expressions is a significant improvement, and the test suite is now much more comprehensive. The added comments make the code easier to understand and maintain. The changes also solve the problem and dont cause new ones.
#13
@
10 months ago
- Keywords changes-requested removed
- Milestone changed from Future Release to 6.8
- Owner set to pbearne
- Status changed from new to assigned
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
9 months ago
This ticket was mentioned in Slack in #core by wildworks. View the logs.
6 weeks ago
#17
@
6 weeks ago
This ticket was brought up in today's 6.9 bug scrub. Could someone please review the linked PR?
#20
@
5 weeks ago
- Resolution fixed deleted
- Status changed from closed to reopened
The commit [61172] is causing the unit tests to fail on the trunk branch. Let's investigate the cause.
This ticket was mentioned in PR #10479 on WordPress/wordpress-develop by @wildworks.
5 weeks ago
#21
Trac ticket:
@wildworks commented on PR #10479:
5 weeks ago
#22
If this regular expression is truly correct, then months and days with leading zeros missing should be acceptable. Therefore, I will correct the tests.
It may look like there are many differences, but in reality, I've only changed three tests from false to true and grouped them together.
#23
@
5 weeks ago
I believe PR 10479 fixes the unit test failures, but I only just saw this ticket today, so I would appreciate it if someone could review it.
cc @peterwilsoncc @westonruter
This ticket was mentioned in Slack in #core by wildworks. View the logs.
5 weeks ago
This ticket was mentioned in PR #10481 on WordPress/wordpress-develop by @mukesh27.
5 weeks ago
#26
Trac ticket: https://core.trac.wordpress.org/ticket/26798
This PR improves the documentation and structure of test cases related to post date handling in tests/phpunit/tests/post.php. The main focus is on updating docblocks for clarity and converting data provider methods to static methods, which is a best practice in PHPUnit.
- Updated the docblocks for
test_wp_insert_post_handle_malformed_post_date()andtest_wp_resolve_post_date_regex()to clearly describe the parameters and expected behavior of each test, making the intent and usage of the tests more understandable. - Converted the data provider methods
data_wp_insert_post_handle_malformed_post_date()anddata_wp_resolve_post_date_regex()to static methods and enhanced their docblocks to specify the structure of the returned data, aligning with PHPUnit best practices.
Diff for suggested changes to fix post.php