checks against array('draft', 'pending') should be changed

[Denis-de-Bernardy]

With the introduction of the trash status, checks for a post_status in array('draft', 'pending') should probably be changed into either of = 'publish' or != 'publish'.

As things currently stand, a function like wp_link_pages() with a paginated trashed post probably ends up returning a fancy permalink.

[nacin]

I looked through those instances, and it looks like wp_list_pages() is the only function that may be affected. Best I can tell, all other instances already filter out trashed posts or have their own trash handlers.

But the post itself can't be viewed -- you'll get a 404 in the frontend, and a wp_die in the backend, so is it really that bad that wp_link_pages() doesn't check for it?

That said, there is an instance that does need to be patched -- while wp-comments-posts.php checks for trash status and bails, admin-ajax.php doesn't check when replying to comments in wp-admin. I was able to reply to a comment on a post that was trashed.

Two patches forthcoming. One tramples over the string freeze, one doesn't but then returns an error message that isn't 100% correct.

[nacin]

Adds a new string

[nacin]

No new strings, but an inaccurate response

[Denis-de-Bernardy]

scanning further at the same time. there also are various occurrences of:

array('pending', 'draft', 'future')
array('pending', 'draft')
array('publish', 'future', 'private')
array('publish', 'future')

and probably

statements that aren't immediately obvious when doing a search in project.

[Denis-de-Bernardy]

continuing scans... post.dev.js has:

			if ( $('option:selected', postStatus).val() == 'private' || $('option:selected', postStatus).val() == 'publish' ) {
				$('#save-post').hide();
			} else {
				$('#save-post').show();
				if ( $('option:selected', postStatus).val() == 'pending' ) {
					$('#save-post').show().val( postL10n.savePending );
				} else {
					$('#save-post').show().val( postL10n.saveDraft );
				}
			}

and:

				if ( $('#original_post_status').val() == 'future' || $('#original_post_status').val() == 'draft' ) {
					if ( optPublish.length ) {
						optPublish.remove();
						postStatus.val($('#hidden_post_status').val());
					}

I'm supposing that most of these are covered indirectly, but it would be good to ultimately harden these checks as needed.

[nacin]

Yeah, my initial search string was in_array.*(pending|draft|future), which found 23 matches in 10 files. I also found one instance of an or/and check, ​in _get_page_link().

I looked through those matches quickly and saw nothing glaring other than the two I've pointed out, though a second look couldn't hurt.

Good catch in the JS.

This all said, while I agree with hardening, the indirect coverage is largely this: many of these instances are inaccessible as a trashed post/page returns 404 in the front and wp_die in the admin area.

[westi]

_get_page_link() looks fine to me - this check is whether or not the post has a public permalink yet or should just be referred by id - Trashed posts should be fine returning the real permalink.

Can you rephrase this ticket as a list of actual bugs - it is hard to investigate/fix without a list of actual issues.

[nacin]

All I found is that while wp-comments-posts.php checks for trash status and bails, admin-ajax.php doesn't check when replying to comments in wp-admin. I was able to reply to a comment on a post that was trashed.

Two patches correct this -- one adheres to string freeze, the other doesn't.

[Denis-de-Bernardy]

Replying to westi:

Can you rephrase this ticket as a list of actual bugs - it is hard to investigate/fix without a list of actual issues.

I'm afraid I cannot: I merely noted the issue while working on my caching plugin, I thought it looked very fishy, and so I opened a ticket in case anyone felt looking into it.

nacin did, checked that most potential issues were already covered indirectly, except the one that he mentions in his previous comment.

[azaozz]

(In [12394]) Discard replies to comments on a trashed post, props nacin, fixes #11401

[nacin]

Re-opening and moving to 3.0 to get a proper string in there specific to a trashed post, like there is in wp-comments-post.php. Feel free to re-close if you think that's unnecessary.

[azaozz]

Not sure that's needed as this is just a double check. Comments on trashed posts are not displayed in the admin so the user cannot reply in the first place.

[nacin]

Right, I had to trash it in another tab to reproduce.

[hakre]

References: #11236, ​overall issue blog post, ​2.9.2 release blog post