Make WordPress Core

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#48213 closed defect (bug) (fixed)

The XML-RPC endpoint should return a more appropriate HTTP status code when it's disabled

Reported by: johnbillion's profile johnbillion Owned by: johnbillion's profile johnbillion
Milestone: 5.7 Priority: low
Severity: normal Version:
Component: XML-RPC Keywords: has-patch
Focuses: Cc:

Description

When the XML-RPC endpoint is disabled via a false return value on the xmlrpc_enabled filter, a POST request to /xmlrpc.php receives an HTTP 200 response code along with the error message.

A more appropriate response code would be either 403 or 405, the latter of which mirrors the IXR_Error code used internally.

Change History (9)

This ticket was mentioned in PR #774 on WordPress/wordpress-develop by ericmann.


4 years ago
#1

  • Keywords has-patch added; needs-patch removed

Updates the XML-RPC system to return proper HTTP error codes when authentication fails, when XML-RPC is explicitly disabled, and when any other errors are thrown by the system.

Trac ticket: https://core.trac.wordpress.org/ticket/48213

github-actions[bot] commented on PR #774:


4 years ago
#2

Hi @ericmann! 👋

Thank you for your contribution to WordPress! 💖

It looks like this is your first pull request to wordpress-develop. Here are a few things to be aware of that may help you out!

No one monitors this repository for new pull requests. Pull requests must be attached to a Trac ticket to be considered for inclusion in WordPress Core. To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description.

Pull requests are never merged on GitHub. The WordPress codebase continues to be managed through the SVN repository that this GitHub repository mirrors. Please feel free to open pull requests to work on any contribution you are making.

More information about how GitHub pull requests can be used to contribute to WordPress can be found in this blog post.

Please include automated tests. Including tests in your pull request is one way to help your patch be considered faster. To learn about WordPress' test suites, visit the Automated Testing page in the handbook.

If you have not had a chance, please review the Contribute with Code page in the WordPress Core Handbook.

The Developer Hub also documents the various coding standards that are followed:

Thank you,
The WordPress Project

#3 @ericmann
4 years ago

Patch above created via GitHub does three things:

  1. It adds a 405 header when XML-RPC is disabled.
  2. It adds a 403 header when authentication fails.
  3. It adds whatever error is encoded into IXR_Error automatically when any other error is throw.

The last bullet might be unnecessary and is a little outside the scope of the original ask, but was a relatively straightforward add to the behavior of IXR_Server::error() so I threw it in for completeness.

#4 @johnbillion
4 years ago

  • Owner set to johnbillion
  • Status changed from new to reviewing

#5 @johnbillion
4 years ago

  • Milestone changed from Awaiting Review to 5.7

#6 follow-up: @johnbillion
4 years ago

Thanks for the PR @ericmann . Adding the status_header() call inside IXR_Server::error() makes sense as it allows for other errors to send a status code.

Unfortunately several places in the IXR library use non-standard error codes (eg. -32300) but these are safely ignored by status_header().

I pushed some commits to your PR to remove the duplicate status_header() calls.

#7 in reply to: ↑ 6 @ericmann
4 years ago

Replying to johnbillion:

I pushed some commits to your PR to remove the duplicate status_header() calls.

I saw the commits come through via email. Great additions, thanks!

#8 @johnbillion
4 years ago

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

In 49862:

XML-RPC: Emit an appropriate HTTP status code when an error is returned in response to an XML-RPC request.

This most notably affects the response when XML-RPC is disabled or when the supplied username and password is incorrect.

Props ericmann

Fixes #48213

Note: See TracTickets for help on using tickets.