WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#20246 closed defect (bug) (fixed)

When adding a url to video

Reported by: dimitrov.adrian Owned by: nacin
Milestone: 3.4.2 Priority: normal
Severity: normal Version: 3.3.1
Component: Media Keywords: has-patch commit
Focuses: Cc:

Description

It is on my 3.3.1 installation but i remmember the problem and from previous versions.

Here is and error log output:


[15-Mar-2012 20:04:45] PHP Fatal error:  Uncaught exception 'Exception' with message 'Serialization of 'SimpleXMLElement' is not allowed' in /home/scifibgr/public_html/wp-includes/functions.php:1038
Stack trace:
#0 /home/scifibgr/public_html/wp-includes/functions.php(1038): serialize(Object(SimpleXMLElement))
#1 /home/scifibgr/public_html/wp-includes/meta.php(60): maybe_serialize(Object(SimpleXMLElement))
#2 /home/scifibgr/public_html/wp-includes/meta.php(127): add_metadata('post', 11436, '_oembed_a45174d...', Object(SimpleXMLElement))
#3 /home/scifibgr/public_html/wp-includes/post.php(1492): update_metadata('post', 11436, '_oembed_a45174d...', Object(SimpleXMLElement), '')
#4 /home/scifibgr/public_html/wp-includes/media.php(1204): update_post_meta(11436, '_oembed_a45174d...', Object(SimpleXMLElement))
#5 /home/scifibgr/public_html/wp-includes/media.php(1278): WP_Embed->shortcode(Array, 'http://vbox7.co...')
#6 [internal function]: WP_Embed->autoembed_callback(Array)
#7 /home/scifibgr/public_html/wp-includes/media.php(1264): preg_replace_callback('| in /home/scifibgr/public_html/wp-includes/functions.php on line 1038

Attachments (4)

20246.docs.patch (584 bytes) - added by SergeyBiryukov 3 years ago.
20246.patch (460 bytes) - added by SergeyBiryukov 3 years ago.
20246.2.patch (1.6 KB) - added by SergeyBiryukov 3 years ago.
20246.diff (625 bytes) - added by nacin 3 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 @ocean903 years ago

  • Keywords reporter-feedback added

Are you using a plugin? Core doesn't handle vbox7.com videos.

comment:2 @dd323 years ago

The Bulgarian localised version does add vbox7 support: #19980 - it might be coming from there.

comment:3 @SergeyBiryukov3 years ago

Apparently wp_oembed_get() has returned a SimpleXMLElement object. However, looking at the core code, it should only return a string or false.

I've added the line from bg_BG version:

wp_oembed_add_provider( '#http://(www\.)?vbox7\.com/play:.+#', 'http://vbox7.com/etc/oembed/', true );

Then tried to embed a random video from vbox7.com. The video appeared without an error.

Have you tried with all plugins disabled? What are the steps to reproduce this on a clean install?

comment:4 @SergeyBiryukov3 years ago

Moreover, core doesn't seem to use SimpleXMLElement objects anywhere.

comment:5 @nacin3 years ago

We do, in WP_oEmbed::_fetch_xml().

@SergeyBiryukov3 years ago

comment:6 @SergeyBiryukov3 years ago

Ah, right. I guess it's WP_oEmbed::_parse_xml():
http://core.trac.wordpress.org/browser/tags/3.3.1/wp-includes/class-oembed.php#L204

Still, WP_oEmbed::data2html() should have converted it to an HTML string.

Also, seems that the docs for wp_oembed_get() are incorrect. It returns false on failure, not the original URL. 20246.docs.patch fixes that.

comment:7 @SergeyBiryukov3 years ago

Moved 20246.docs.patch to its own ticket: #20355.

comment:8 @nacin3 years ago

  • Keywords has-patch commit added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 3.5

@SergeyBiryukov3 years ago

@SergeyBiryukov3 years ago

comment:9 @SergeyBiryukov3 years ago

  • Milestone changed from 3.5 to 3.4.2

The wp_oembed_get() PHPDocs fix was already committed in [20562].

I've tried to reproduce the original issue and this time the result was different from comment:3.

The data was properly fetched, but then discarded due to is_string( $data->html ) returning false:
http://core.trac.wordpress.org/browser/tags/3.4.1/wp-includes/class-oembed.php#L248

is_string() check was introduced in [20539], two weeks after comment:3, which explains the different results. Seems that before [20539], the object was somehow automatically converted to string on my PHP install, so I didn't get the error.

I've disabled WP_Embed cache to get consistent results and make sure it's not a network glitch.

var_dump( $data->html ) returned this:

object(SimpleXMLElement)#266 (1) {
  [0]=>
  string(507) "<object classid="clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" codebase="http://download.macromedia.com/pub/shockwave/cabs/flash/swflash.cab#version=8,0,0,0" width="450" height="403"><param name="movie" value="http://i48.vbox7.com/player/ext.swf?vid=26c23d8d"><param name="quality" value="high"><embed src="http://i48.vbox7.com/player/ext.swf?vid=26c23d8d" quality="high" pluginspage="http://www.macromedia.com/go/getflashplayer" type="application/x-shockwave-flash" width="450" height="403"></embed></object>"
}

At first I thought that vbox7 returns an object for the html value (like Viddler in ticket:20322:2), but this doesn't seem to be the case, the response looks proper:
http://vbox7.com/etc/oembed/?maxwidth=625&maxheight=600&url=http%3A%2F%2Fvbox7.com%2Fplay%3Aa3dffea7&format=xml

Then I removed json from line 166 to force YouTube to use XML and got a similar response:
http://www.youtube.com/oembed?maxwidth=625&maxheight=600&url=http%3A%2F%2Fwww.youtube.com%2Fwatch%3Fv%3DD7vS4z6ngQo&format=xml

No video was displayed in the post. var_dump() showed that $data->html was a SimpleXMLElement object (like the one above). Looks like [20539] broke XML oEmbed responses, which is a regression.

According to the php.net manual, we should cast the elements to strings (see example 6):
http://www.php.net/manual/en/simplexml.examples-basic.php

20246.2.patch does that.

Version 1, edited 3 years ago by SergeyBiryukov (previous) (next) (diff)

comment:10 @nacin3 years ago

I think 20246.2.patch would in turn regress what [20539] was trying to fix. Should we try to detect if we're dealing with a SimpleXMLElement and then convert to string, otherwise reject if ! is_string()?

Kind of lame that the XML-specific stuff isn't contained in a method.

@nacin3 years ago

comment:11 @nacin3 years ago

In [21701]:

Fix oEmbed when the provider only supports XML responses.

[20539] removed string casts that would have taken place on SimpleXMLElement
objects, which implement toString. Instead, convert the SimpleXMLElement object
to a stdClass object before we leave _parse_xml(), for consistency with the
simple object returned from _parse_json().

see #20246.
for trunk.

comment:12 @nacin3 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21711]:

Fix oEmbed when the provider only supports XML responses.

[20539] removed string casts that would have taken place on SimpleXMLElement
objects, which implement toString. Instead, convert the SimpleXMLElement object
to a stdClass object before we leave _parse_xml(), for consistency with the
simple object returned from _parse_json().

fixes #20246.
for the 3.4 branch.

comment:13 @shushuglobal2 years ago

  • Cc shushuglobal added
Note: See TracTickets for help on using tickets.