WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 2 weeks ago

#52958 new defect (bug)

The XML-RPC endpoint returns 404 rather than 200

Reported by: ariskataoka Owned by:
Milestone: 5.7.2 Priority: normal
Severity: normal Version: 5.7
Component: XML-RPC Keywords: has-patch has-unit-tests
Focuses: Cc:

Description (last modified by SergeyBiryukov)

The [XML-RPC specs](http://xmlrpc.com/spec.md) say:

Unless there's a lower-level error, always return 200, OK.

For example: for an attachment not found in the WP db, it should return a body with a faultCode 404, AND the HTTP status code should be 200.

That's how WordPress XML-RPC used to work until the 5.7 version was released.

After that release, it started returning the error code as the status code header. It stopped following the specs and broke eventual 3rd party integrations that rely on that status code to validate/handle the responses. (I saw some integrations experiencing that behavior).

I did a quick research on trac and could find this ticket: #48213

The issue:

Apparently, the ticket changes should only affect the status code when the XML-RPC is disabled or the user/pass is incorrect. However, I could notice it also affects the application when the XML-RPC is enabled.

How to test it?

Request command:

curl --location --request POST 'http://test57.local/xmlrpc.php' \
--header 'Content-Type: application/xml' \
--data-raw '<?xml version="1.0"?>
<methodCall>
     <methodName>wp.getMediaItem</methodName>
     <params>
          <param><value>ANY</value></param>
          <param> <value>valid user</value> </param>
          <param> <value>correct password</value></param>
          <param><value>non existent attachment id </value></param>
     </params>
</methodCall>
'
'

Response on WordPress 5.7:

HTTP/1.1 404 Not Found
Server: nginx/1.16.0
Date: Thu, 01 Apr 2021 09:58:28 GMT
Content-Type: text/xml; charset=UTF-8
Transfer-Encoding: chunked
Connection: keep-alive
Vary: Accept-Encoding
X-Powered-By: PHP/7.4.1

<?xml version="1.0" encoding="UTF-8"?>
<methodResponse>
  <fault>
    <value>
      <struct>
        <member>
          <name>faultCode</name>
          <value><int>404</int></value>
        </member>
        <member>
          <name>faultString</name>
          <value><string>Invalid attachment ID.</string></value>
        </member>
      </struct>
    </value>
  </fault>
</methodResponse>

The status code should still be a 200.

Reponse on WordPress 5.6:

HTTP/1.1 200 OK
Date: Thu, 01 Apr 2021 13:18:40 GMT
Server: Apache/2.4.38 (Debian)
X-Powered-By: PHP/7.4.15
Connection: close
Vary: Accept-Encoding
Content-Length: 394
Content-Type: text/xml; charset=UTF-8

<?xml version="1.0" encoding="UTF-8"?>
<methodResponse>
  <fault>
    <value>
      <struct>
        <member>
          <name>faultCode</name>
          <value><int>404</int></value>
        </member>
        <member>
          <name>faultString</name>
          <value><string>Invalid attachment ID.</string></value>
        </member>
      </struct>
    </value>
  </fault>
</methodResponse>

The following code is responsible for the relevant changes:

https://github.com/WordPress/wordpress-develop/pull/774/files#diff-c6872009630e677e111a2e9294070f161d1e90377389ecbe46e66cd4dcf1668eR133-R135

In the code, it's possible to see that the IXR_Error instances would return the HTTP status code header sent as a parameter to the constructor method. It doesn't take into consideration the XML-RPC is enabled or disabled.

E.g.:
https://github.com/WordPress/WordPress/blob/dc4de0d5a5360d6799985d54c8ad12f0ffe7da43/wp-includes/class-wp-xmlrpc-server.php#L1881

https://github.com/WordPress/WordPress/blob/dc4de0d5a5360d6799985d54c8ad12f0ffe7da43/wp-includes/class-wp-xmlrpc-server.php#L1885

Change History (6)

#1 @SergeyBiryukov
3 weeks ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 5.7.1

Hi there, welcome to WordPress Trac! Thanks for the report.

Moving to 5.7.1 for investigation, as this was introduced in [49862] / #48213.

#2 @johnbillion
3 weeks ago

  • Keywords needs-patch added

Thanks for the report. Yeah we expanded the implementation of HTTP response codes beyond the original scope of #48213, which was originally just for a better response code when XMLRPC is disabled.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


2 weeks ago

#4 @audrasjb
2 weeks ago

  • Milestone changed from 5.7.1 to 5.7.2

Moving to Milestone 5.7.2 as WordPress 5.7.1 Release Candidate 1 is planned for today.

This ticket was mentioned in PR #1184 on WordPress/wordpress-develop by ariskataoka.


2 weeks ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

When the XML-RPC is disabled, the HTTP status code should be according to the error code,
when it's enabled, it should return 200 as per the XML-RPC specs.

This PR:

  • Overrides the IXR-server error method, implementing the HTTP

status code logic based on the XML-RPC $enabled attribute;

  • Moves the logic that verifies if the XML-RPC is enabled to its own method;

Fixes: https://core.trac.wordpress.org/ticket/52958

#6 @ariskataoka
2 weeks ago

Hi there!

Thanks for accepting this report. I've added a patch on Github (link above).

When the XML-RPC is disabled, the HTTP status code should be according to the error code,
when it's enabled, it should return 200 as per the XML-RPC specs.

This PR:

Overrides the IXR-server error method, implementing the HTTP
status code logic based on the XML-RPC $enabled attribute;

Moves the logic that verifies if the XML-RPC is enabled to its own method;

Note: See TracTickets for help on using tickets.