WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#20394 closed defect (bug) (fixed)

XMLRPC: notices for missing arguments

Reported by: markoheijnen Owned by: nacin
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: XML-RPC Keywords: has-patch dev-feedback
Focuses: Cc:

Description

When arguments aren't pass there is a big chance a notice get fired.
When someone uses WP_Debug this can mean the XMLRPC will break

Attachments (4)

20394.patch (21.2 KB) - added by markoheijnen 7 years ago.
$args checks changes for all the XMLRPC methods
20394.guard.diff (20.6 KB) - added by markoheijnen 7 years ago.
Alternative way
20394.guard.2.diff (4.3 KB) - added by maxcutler 7 years ago.
20394.3.diff (5.6 KB) - added by sivel 7 years ago.
Use tabs instead of spaces

Download all attachments as: .zip

Change History (15)

@markoheijnen
7 years ago

$args checks changes for all the XMLRPC methods

#1 @maxcutler
7 years ago

I wonder if that patch is really the right approach.

It might be better to have a guard condition at the top of each method that checks that $args is at least a minimum length. And if the guard fails, return a 400 status code error.

Sensible error messages help API consumers as well as prevent these PHP notices.

#2 @maxcutler
7 years ago

  • Cc max@… added

#3 @markoheijnen
7 years ago

Was first thinking about that. Looking into the code I think for all methods it should work. Probably some need discussion.

One issue need to be fixed and that is in wp_editPost. $post_more isn't set what causes a notice

@markoheijnen
7 years ago

Alternative way

#4 @markoheijnen
7 years ago

This is a first rough version of the guard condition version. A lot of unit test will fail since they also test if there are nog enough arguments passed.

#5 @nacin
7 years ago

I could go for 20394.guard.diff but we should enforce this for new methods only.

#6 @nacin
7 years ago

  • Milestone changed from Awaiting Review to 3.4

#7 @maxcutler
7 years ago

Added a refreshed patch that makes the method name a bit friendly and only applies to methods added in 3.4 for back-compat. I'm not sure whether we should make it cover the other wp.* methods, but at least we should avoid adding it to the legacy method namespaces.

#8 @nacin
7 years ago

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

In [20636]:

Introduce minimum_args() method in XML-RPC and leverage it to return errors for insufficient arguments for methods that are new in 3.4.

props maxcutler, markoheijnen.
fixes #20394.

#9 @sivel
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The patch, used spaces instead of tabs, and 4 spaces at that. I happened to notice that as a result the code looked poorly formatted with a config using an 8 space tab.

Upcoming patch switches to tabs instead of spaces.

@sivel
7 years ago

Use tabs instead of spaces

#10 @nacin
7 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [20703]:

We indent with spaces, not tabs. props sivel. fixes #20394.

#11 @nacin
7 years ago

Of course, we indent with tabs, not spaces.

Note: See TracTickets for help on using tickets.