WordPress.org

Make WordPress Core

Opened 4 weeks ago

Last modified 12 days ago

#46026 assigned defect (bug)

WSODs protection returns incorrect content type for XML Requests

Reported by: spacedmonkey Owned by: spacedmonkey
Milestone: 5.2 Priority: normal
Severity: normal Version: trunk
Component: Bootstrap/Load Keywords: has-patch needs-testing servehappy
Focuses: multisite Cc:

Description

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.

Attachments (3)

46026.diff (3.8 KB) - added by spacedmonkey 4 weeks ago.
46026.2.diff (3.2 KB) - added by spacedmonkey 4 weeks ago.
46026.3.diff (3.3 KB) - added by spacedmonkey 4 weeks ago.

Download all attachments as: .zip

Change History (17)

@spacedmonkey
4 weeks ago

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


4 weeks ago

#2 @spacedmonkey
4 weeks ago

@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.

#3 @afragen
4 weeks ago

Does the output need to be escaped in some fashion?

#4 @birgire
4 weeks ago

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().

#5 @pento
4 weeks ago

  • Milestone changed from Awaiting Review to 5.1

Setting Milestone to 5.1 for review.

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


4 weeks ago

#7 @flixos90
4 weeks ago

  • Owner set to spacedmonkey
  • Status changed from new to assigned

@spacedmonkey
4 weeks ago

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


4 weeks ago

#9 @birgire
4 weeks ago

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.

#10 @spacedmonkey
4 weeks ago

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.


@spacedmonkey
4 weeks ago

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


3 weeks ago

#13 @flixos90
2 weeks ago

  • Milestone changed from 5.1 to 5.2

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


12 days ago

Note: See TracTickets for help on using tickets.