Invalid published date insertion in posts

Description

To reproduce:

Set date on a post to Feb 29th in a non-leap year.

Click "Update".

The UI JS will prevent you from hitting OK, but you can bypass it and insert the bad datestamp as described above. I get other inconsistant oddness using the same procedure with other bad numerals (54, -2, etc).

Ah, problem indeed. Here's a tentative patch - it disables the submit button until you click OK on the timestamp editor and the date is valid (this may need UI feedback), and issues an error server-side if an invalid date is supplied (for when JS is disabled).

That one is tricky... edit_post() calls wp_die() when there is an error. In AJAX mode, wp_die() kills the error message and returns -1 (which list table JS doesn't handle too well right now, it just prints it). I don't think the behaviour of edit_post() can be changed because it is used in many places. We could:

Just print a generic error response when the response is -1 (not very helpful to the user IMO)

Add JS validation to the inline-edit form

Change the inline-save procedure to call _wp_translate_post_data() manually and return any errors before calling edit_post()

Check the dates via js whenever the post publishing form #post is submitted. If the dates are invalid, prevent publish, hide the ajax spinner, remove the disabled state from the publish button and slide down the #timestampdiv revealing the date error (highlited in red), giving the user a visual indication of the problem and an opportunity to fix it.

Check the dates with php's native chekdate() function before before saving a post and return a WP_Error if the date is wrong

Tested the patch with posts, pages and quick edit and it all works as expected.

I tested with this plugin active, but it was hard to tell whether things we're behaving properly or not. Partially because I have no knowledge of how this calendar work or of arabic (which the plugin's test is in) and also because the plugin spits out a bunch of php notices and warnings. Nonetheless, I seemed to be able to save posts properly with the plugin active.

Check the dates via js whenever the post publishing form #post is submitted. If the dates are invalid, prevent publish, hide the ajax spinner, remove the disabled state from the publish button and slide down the #timestampdiv revealing the date error (highlited in red), giving the user a visual indication of the problem and an opportunity to fix it.

Check the dates with php's native chekdate() function before saving a post and return a WP_Error if the date is wrong

Tested the patch with posts, pages and quick edit and it all works as expected.

This looks like a good first pass but I think it doesn't cover all possible post editing/insertion routes and so we could still have invalid dates coming in from other places because this is only really in the code flow for the admin ui.

We should protect against invalid dates from XML-RPC as well by adding a similar check lower down in the API inside wp_insert_post as this is the function that finally does the db writes.

I tested with this plugin active, but it was hard to tell whether things we're behaving properly or not. Partially because I have no knowledge of how this calendar works or of arabic (which the plugin's text is in) and also because the plugin spits out a bunch of php notices and warnings. Nonetheless, I seemed to be able to save posts properly with the plugin active.

I think that as checkdate is specifically a Gregorian function we should either have a wrapper function with a filter in it or we should filter the response from it so that plugins that want to implement different calendar systems can check the date in there own way.

We should protect against invalid dates from XML-RPC as well by adding a similar check lower down in the API inside wp_insert_post as this is the function that finally does the db writes.

I think that as checkdate is specifically a Gregorian function we should either have a wrapper function with a filter in it or we should filter the response from it so that plugins that want to implement different calendar systems can check the date in there own way.

Posting: Make it much harder to create posts with invalid dates by enforcing the post date tests in the UI and the backend code.

Previously you could quite easily send a new post into the back of beyond by specifying an invalid date like the 30th Feb and this was very confusing.
Sometimes it would seem to work and sometimes the post would end up very far in the past - depending on the mysql version and other factors.

These filters are two different names. They should probably be the same. I usually like filters over wrapper functions, but wp_checkdate() probably isn't a bad idea, here. I don't think the context (whether you came from wp_insert_post() or edit_post(), or what the post array is) is necessary, and if anything it is spurious. If you want to modify a date based on some context, there are other filters for that. I usually don't fight for less context, but the only context that should matter here is what calendar system is being used.

Also, wp_insert_post() can return 0 depending on $wp_error, so we need to avoid returning WP_Error in that case. And whoops has an "h". :-)

These filters are two different names. They should probably be the same. I usually like filters over wrapper functions, but wp_checkdate() probably isn't a bad idea, here. I don't think the context (whether you came from wp_insert_post() or edit_post(), or what the post array is) is necessary, and if anything it is spurious. If you want to modify a date based on some context, there are other filters for that. I usually don't fight for less context, but the only context that should matter here is what calendar system is being used.

Also, wp_insert_post() can return 0 depending on $wp_error, so we need to avoid returning WP_Error in that case. And whoops has an "h". :-)

@jkudish: I tested the latest patch and it looks like it works but has confusing behaviour in current trunk because it kicks in after the spinner has been enabled and so you have a spinner still spinning.

Curious why? The php part of this patch has been in for a while and seemingly works fine. The js part was working fine but broke due to some other js changes in core. It's now back to working fine and should be ok to commit. I think that if we are to punt this then we should revert the php changes. Because the experience of attempting a bad date and ending up on a wp_die screen (something the js otherwise prevents) is a poor experience. But of course, I would vote for not punting :)