Bug in the maybe_serialize() function - it serializing also serailize objects.

[maorb]

Hi,

I have discovered this bug by accident, when trying to import posts using the csv importer plugin.
One of my lines contained a serialized array to be saved.
The serialized data was ok, but saved to the db with a prefix of s:196:" and suffix of ";.
The reason is that the add_post_meta() function uses the add_metadata() , which uses the maybe_serialize() function to determine if need to serialize or not.
The thing is that it is serializing anyway!

Look at wp-includes/functions.php, lines 1005-1013 -

function maybe_serialize( $data ) {
	if ( is_array( $data ) || is_object( $data ) )
		return serialize( $data );

	if ( is_serialized( $data ) )
		return serialize( $data );

	return $data;
}

It says to serialize also if the data is_serialized....

if ( is_serialized( $data ) )
		return serialize( $data );

The correct should be -

function maybe_serialize( $data ) {
	if ( is_array( $data ) || is_object( $data ) )
		return serialize( $data );

	if ( is_serialized( $data ) )
		return $data ;

	return $data;
}

Otherwise, the data is being serialized twice and then WP can't show the real unserialized data ..

After I changed it and rechecked passing serialized data to the add_meta_data() function - it all went ok.

This should be fixed in core.

Here is a reference also in the forums
​http://wordpress.org/support/topic/plugin-csv-importer-escaping-quotation-marks

Thanks,
Maor

[greuben]

Dupe of #15057
see [14074]

[nacin]

Double-serialize is intentional.

In 3.2 I will add a quick comment there letting people know it is intentional and I'll cross-reference r14074. Let's just leave this open for a few days before I forget.

[maorb]

Why it should be double serialized?
This means that if one will pass serialized data to the add_metadata() function, it will never be parsed ok, since it will add the second serialization.
This causes breaking of the data when pulling out from the db..
If double serialize is a must, maybe should have addition to the get_metadata() to handle it better?

Thanks

[nacin]

If you serialize something then pass it to add_metadata(), then you should be expected to unserialize it when you fetch it from get_metadata(). Whatever goes in, comes out.

In turn, the functions already serialize -- that's the point of maybe_serialize(), it gets used in the transient, option, site option, and metadata APIs. Just pass an array or object and it'll take care of it. In a nutshell, you're thinking too hard. :-)

When this last came up, I wrote a blog post on it: ​http://andrewnacin.com/2010/04/18/wordpress-serializing-data/.

[maorb]

I agree with you that what goes in should come out.
Anyway, there are situations of different methods to insert data and thus it sometimes will go as an array and sometimes as a serialized data.
For this example I encountered - there are some meta keys that are indeed being passed as an array, where the end user just saves data from the admin panel.
But, in this case for the same site, there is a way of importing csv file to the database.
All these custom fields that are being saved into one meta_key and the meta_value is the serialized array, cannot be passed as an object when it comes to a csv file, since you cannot pass an array in a csv, thus when data comes from csv it must be serialized first.

I thought that the maybe_serialize() should check if the data passes it alreay serialized and not to serialize it again..

[hakre]

If you import a CSV, the import script should make use of the is_serailized() function on each field. In case this function returns true, unserialize it. Then give only unserialized values to the add_post_meta()/add_metadata() functions and you should be fine.

Edit: Or use maybe_unserialize() on CSV data fields which might be more straight forward (and does what I wrote).

[SergeyBiryukov]

#15057

[SergeyBiryukov]

Added patch with a comment, as per nacin's suggestion (just in case).

[markjaquith]

In [18807]:

Add note about why double-serialization is done. props SergeyBiryukov. fixes #16597