Repository navigation
oEmbed: Fall back to XML parsing when JSON parsing fails - #12657
shrivastavanolo wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves resilience of WordPress’s oEmbed fetching by adding a fallback parse path when a provider returns XML despite WordPress requesting JSON (e.g., when the discovered endpoint is effectively XML-only and ignores format=json).
Changes:
- Parse the response body using the requested format first, then fall back to XML parsing when JSON parsing fails.
- Return the parsed result from
_fetch_with_format()after attempting the fallback.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/wp-includes/class-wp-oembed.php:656
- This new JSON→XML fallback changes the observable behavior of
WP_oEmbed::fetch()in an important edge case (JSON request returning an XML oEmbed body). There doesn’t appear to be any PHPUnit coverage for this code path today, so it would be easy to regress.
Consider adding a unit test that mocks the HTTP response to return a valid XML oEmbed document when the request URL contains format=json, and assert that WP_oEmbed::fetch() returns the parsed object (and/or that data2html() produces expected output).
if ( false === $data && 'json' === $format && false !== stripos( $body, '<oembed' ) ) {
$data = $this->_parse_xml( $body );
}
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
When
WP_oEmbed::_fetch_with_format()requestsformat=jsonbut JSON parsing of the response fails, this falls back to attempting to parse the same response body as XML before giving up. Some oEmbed providers advertise an XML-only endpoint via a<link rel="alternate" type="text/xml+oembed" href="...">discovery tag.WordPress's
fetch()always appends&format=jsonto the request regardless of the discovered endpoint's declared type. A spec-compliant provider correctly ignores that parameter and returns a valid XML response with an HTTP 200 status.Trac ticket: https://core.trac.wordpress.org/ticket/44231
Use of AI Tools
AI assistance: Yes
Tool(s): Claude
Model(s): Claude Sonnet 5
Used for: Investigating the bug and drafting the initial fix. Reviewed, debugged, and verified end-to-end by me.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.