Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#20629 closed defect (bug) (fixed)

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

Reported by: redsweater's profile redsweater Owned by: ryan's profile ryan
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: XML-RPC Keywords: has-patch
Focuses: Cc:


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:

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

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.

Attachments (3)

FixGetPages.diff (681 bytes) - added by redsweater 12 years ago.
Patch to omit pages from wp_getPages when wp_getPage has returned an error for the given page
20629.patch (5.4 KB) - added by maxcutler 12 years ago.
20629.2.patch (5.6 KB) - added by maxcutler 12 years ago.
Tabs for indent, spaces for alignment.

Download all attachments as: .zip

Change History (12)

12 years ago

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

#1 @redsweater
12 years ago

  • Keywords has-patch added

#2 @maxcutler
12 years ago

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).

#3 @maxcutler
12 years ago

  • Cc maxcutler added

#4 @nacin
12 years ago

  • Milestone changed from Awaiting Review to 3.4

#5 @maxcutler
12 years ago

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.

#6 @redsweater
12 years ago

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.

#7 @nacin
12 years ago

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

12 years ago

#8 @maxcutler
12 years ago

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.

12 years ago

Tabs for indent, spaces for alignment.

#9 @ryan
12 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [20807]:

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

Note: See TracTickets for help on using tickets.