WordPress.org

Make WordPress Core

Opened 4 weeks ago

Last modified 4 weeks ago

#44231 new defect (bug)

XML oembed discovery is not implemented correctly

Reported by: rivalitaet Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Embeds Keywords:
Focuses: Cc:

Description

When using XML in oembed discovery, WordPress still tries to encode it as JSON and fails. It doesn't retry to check if XML would work.

How to reproduce this:

  • Add a link tag for an XML oembed: this is clearly tagged as text/xml+oembed and also has the file extension .xml: `<link rel="alternate" type="text/xml+oembed" href="http://example.com/oembed.xml?url=http://example.com/blah">
  • WordPress will now correctly resolve the provider as http://example.com/oembed.xml?url=http://example.com/blah
  • Wordpress will send a request to: http://example.com/oembed.xml?url=http://example.com/blah&maxwidth=525&maxheight=788&dnt=1&format=json. Note how it adds format=json but still uses the .xml file extension
  • The provider correctly returns an XML response, with HTTP status code 200
  • Wordpress tries to parse it as JSON and fails
  • Wordpress doesn't retry with format=xml since it only tries XML if the JSON endpoint failed with HTTP status code 501

Background info from the spec:

  • If the format of the request is given in the file extension (e.g. /oembed.xml), the provider should ignore the format parameter. (See https://oembed.com/#section2.2).
  • For discovery the provider should specify the full oembed endpoint, including the format paramter. It's therefore wrong to override it in Wordpress. (See https://oembed.com/#section4)

Change History (3)

#1 @joyously
4 weeks ago

So the expected result is?

  • WordPress should not put the json format on a xml request ?
  • WordPress should treat the 200 status differently ?
  • WordPress should try the request with different formats until it gets one right ?

#2 @rivalitaet
4 weeks ago

Well, there are a couple of options:

  • WordPress should not put any format on a discovered endpoint, since the format is already set via the type parameter in HTML input. This might be difficult to implement, since currently an endpoint is only a string (with no information about the needed format).
  • As you pointed out, WordPress should not put the json format on a xml request. To not break any existing (wrong) implementations, we could also continue to send a request with each format, but try xml first for XML endpoints.
  • WordPress could try to parse results in both of the formats: JSON parsing always fails for XML documents so this would be easy to implement.

Especially the second and/or third option seem simple to do.

If wanted, I could also provide a patch for this. You'd have to be a bit patient a bit with me though, I have no experience in using the WordPress trac system so far.

#3 @joyously
4 weeks ago

A patch would be awesome.

I'll just leave this link here in case you need it.

https://make.wordpress.org/core/handbook/contribute/

Note: See TracTickets for help on using tickets.