wp.getPages response includes bogus content for pages lacking edit permission

[redsweater]

In the event that the authenticated user for wp.getPages does not have editing permissions for a given page, instead of that page being completely omitted from the resulting array, the array is filled with bogus entries. The practical implication of this bug is that for XMLRPC clients that do not expect the bogus values to appear, they will strain to interpret the error blocks as actual page entries.

The bogus entries appear like this:

<value><struct>
  <member><name>code</name><value><int>401</int></value></member>
  <member><name>message</name><value><string>Sorry, you cannot edit this page.</string></value></member>
</struct></value>

This doesn't match the documented behavior that the result should be an array of page structures.

I believe the expected behavior would be for the resulting array to simply omit any pages the user doesn't have permission to edit.

The crux of this issue is in the implementation of wp.getPages, and that it reuses the wp_getPage() method to fulfill the getPages request:

for ( $i = 0; $i < $num_pages; $i++ ) {
	$page = wp_xmlrpc_server::wp_getPage(array(
		$blog_id, $pages[$i]->ID, $username, $password
	));
	$pages_struct[] = $page;
}

It is within this wp_getPage() method that the page-specific permission check is made, and a bogus error is returned instead of the page.

Probably the wp_getPages() method should check for this bogus error result and, if present, omit the page from the return $pages_struct.

[redsweater]

Patch to omit pages from wp_getPages when wp_getPage has returned an error for the given page

[maxcutler]

This should really be fixed. While the current patch is the easy way, I wonder if a better solution would be to extract most of wp_getPage into a _prepare_page method like we've done in #20409 and the new 3.4 methods. That way the nested calls to wp_getPage from wp_getPages don't have to re-query for the page (even though it's a likely cache hit).

[maxcutler]

Added unit test in ​[UT715]. The attached patch is broken because $page is an array and get_class will fail on arrays. I'll work on a new patch.

[redsweater]

Aha! Great catch. I only tested the behavior from the other side of the API, where it did work, presumably because whatever value is returned by get_class(array()) is still not equal to IXR_Error.

[nacin]

is_a() or instanceof are both better in this case.

[maxcutler]

Added a new patch that adds a _prepare_page method as mentioned in comment:2. Cuts out double queries in wp.getPages and allows for filtering of the output array for both methods.

[maxcutler]

Tabs for indent, spaces for alignment.

[ryan]

In [20807]:

Fix wp.getPages response when fetching pages the current user does not have caps for. Props maxcutler, redsweater. fixes #20629