WSODs protection returns incorrect content type for XML Requests

[spacedmonkey]

Similar to #45933, calls to wp_die do not return the correct content type, when the requests resource in xml, such RSS feeds, trackbacks and other xml requests such as sitemaps (via a plugin like yoast). Returning the correct content type can have serious issues for third party tools that request say a feed and get a html page in return.

[spacedmonkey]

@birgire Thanks for the feedback, I have fixed those issues in my latest patch here.

I have also added, the following changes.

1. Now checks to see if is_feed and is_trackback return true.
2. Added more content type to accepted list for xml + oembed.

[afragen]

Does the output need to be escaped in some fashion?

[birgire]

Thanks @spacedmonkey

@afragen What about using <![CDATA[...]]> for e.g. the external input of $message?

Some minor things I noticed in 46026.diff​:

• It would be nice to add more information to the parameter's description of _wp_wrap_xml(), like the type and description.

• Missing dot at the end of the _wp_wrap_xml() description.

• Extra new lines at the end of wp_is_xml_request() and the indentation might need some adjustments.

• Should we format the wp_die in the description of _xml_wp_die_handler(), e.g. add a link for the doc parsing, like {@see wp_die} ?

• The function wp_is_xml_request() is not complicated, but it will be a part of the public API, so should unit tests be added for it?

I would also have suggested using e.g. "halt" instead of "kill", but I see that the latter is already used by core :-)

Not related to this ticket, but I don't see any is_callable() on the filterable callbacks in wp_die().

[pento]

Setting Milestone to 5.1 for review.

[birgire]

In 46026.2.diff​ there's a typo in _xml_wp_die_handler() regarding the content type. Should be e.g. text/xml rather than application/json.

The optional error title input is not used in the output of _xml_wp_die_handler(), so I wonder if the error title input should be handled in a similar way as the error message, i.e. with it's own element:

<error>
    <title>...</title>
    <message>...</message>
    ...
</error>

I also wonder if _json_wp_die_handler() should add the error title to the output.

I like the simplification in 46026.2.diff​ compared to 46026.diff​ regarding skipping the array2xml processing and just construct the xml directly.

[spacedmonkey]

Thanks for your feedback @birgire

In 46026.3.diff are the following changes

• Now returns <title> node.
• Now returns content type xml/text
• Also checks is_comment_feed
• Load.php now has a empty line at the end.

[flixos90]

This ticket is based on the old fatal error recovery mode implementation and will be covered as part of #46130.

[spacedmonkey]

Reopening as still valid and not part of #46130.

[SergeyBiryukov]

Restoring the milestone.

[SergeyBiryukov]

Moving to 5.2, per the ​latest #core-php chat.

[SergeyBiryukov]

In 45016:

Bootstrap/Load: Add support for XML requests to wp_die().

In addition to AJAX, XML-RPC, JSON, and JSONP requests, wp_die() now handles XML requests correctly, returning information in the expected content type.

Props spacedmonkey, birgire.
Fixes #46026. See #44458.

[SergeyBiryukov]

In 45017:

Docs: Improve documentation for wp_die() handlers after [45015] and [45016].

See #46543, #46025, #46026.

[SergeyBiryukov]

In 45018:

Docs: Fix typo in wp_is_xml_request() description.

See #46026.

[SergeyBiryukov]

Follow-up: #46813