WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 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 2 years ago.
$args checks changes for all the XMLRPC methods
20394.guard.diff (20.6 KB) - added by markoheijnen 2 years ago.
Alternative way
20394.guard.2.diff (4.3 KB) - added by maxcutler 2 years ago.
20394.3.diff (5.6 KB) - added by sivel 2 years ago.
Use tabs instead of spaces

Download all attachments as: .zip

Change History (15)

markoheijnen2 years ago

$args checks changes for all the XMLRPC methods

comment:1 maxcutler2 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.

comment:2 maxcutler2 years ago

  • Cc max@… added

comment:3 markoheijnen2 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

markoheijnen2 years ago

Alternative way

comment:4 markoheijnen2 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.

comment:5 nacin2 years ago

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

comment:6 nacin2 years ago

  • Milestone changed from Awaiting Review to 3.4

maxcutler2 years ago

comment:7 maxcutler2 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.

comment:8 nacin2 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.

comment:9 sivel2 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.

sivel2 years ago

Use tabs instead of spaces

comment:10 nacin2 years ago

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

In [20703]:

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

comment:11 nacin2 years ago

Of course, we indent with tabs, not spaces.

Note: See TracTickets for help on using tickets.