Ticket #4029 (closed defect: invalid)

Opened 2 years ago

Last modified 5 months ago

maybe_serialize() can do double-serialize

Reported by: takayukister Assigned to: anonymous
Priority: normal Milestone:
Component: General Version: 2.2
Severity: major Keywords: has-patch dev-feedback
Cc:

Description

I'm not sure if this is a problem, but I'm little confused. When a string argument is passed to maybe_serialize(), it serialize the string even if the string is an object which is already serialized. Is it necessary??

Attachments

maybe_serialize.diff (426 bytes) - added by takayukister on 03/25/07 13:52:37.
Serialize if target is *not* serialized.
4029.testcases.diff (2.7 kB) - added by jacobsantos on 06/28/08 18:29:12.
Test cases for maybe_serialize()

Change History

03/25/07 13:52:37 changed by takayukister

  • attachment maybe_serialize.diff added.

Serialize if target is *not* serialized.

03/25/07 16:08:08 changed by rob1n

  • keywords changed from has-patch serialize to has-patch dev-feedback.
  • priority changed from low to normal.
  • severity changed from normal to major.
  • milestone changed from 2.3 to 2.2.

Haha, I'm guessing we don't use maybe_serialize at all in the code.

But yes, I'm puzzled too and your patch makes sense.

03/25/07 23:22:56 changed by ryan

Looks like it's been broken since it was introduce in [4382]. Fix looks good to me.

03/25/07 23:23:54 changed by ryan

  • status changed from new to closed.
  • resolution set to fixed.

(In [5102]) Correct reversed logic in maybe_serialize. Props takayukister. fixes #4029

03/26/07 00:21:13 changed by mdawaffe

  • status changed from closed to reopened.
  • resolution deleted.

I think the original logic was intentional.

If you want to store a serialized string, you store it as twice serialized so that when you retrieve it, it comes back once serialized.

MarkJaquith??

03/26/07 03:13:22 changed by masquerade

I believe its intentional also. This way, nobody can fake a serialized string that is an array with a bizzare amount of elements for the sole purpose of crashing PHP and the webserver. I remember this being a bug in the past because you could put a serialized string in some of the options fields and it would be deserialized, meaning you could inject objects/large arrays/etc.

03/26/07 04:04:15 changed by ryan

But using maybe_serialize() outside the context of options.php could be confusing. You could triple serialize if not careful, yes?

03/26/07 05:12:39 changed by takayukister

I am sorry but I found that my first patch may cause another problem.

Original maybe_serialize() does not apply serialization to plain (not serialized) string, so there are many plain string values existing in options, postmeta and usermeta tables. My patched maybe_serialize() reverse the behavior and serialize all the plain strings.

If there are codes or plugins that access directly to that tables and expect to get plain string value, they may fail.

I'm sorry for my carelessness. I think it would be good to make another patches if necessary.

03/26/07 06:13:45 changed by ryan

[5109] reverts.

03/26/07 06:23:25 changed by markjaquith

  • status changed from reopened to closed.
  • resolution set to invalid.
  • milestone deleted.

Intentional. Ensures that input == output (using maybe_unserialize()). Otherwise, as masquerade pointed out, you can enter a serialized array into a string input and get an array as the output. There were documented server-crash scenarios from this.

The other solution was serializing everything, but this makes the options and meta tables really unfriendly.

06/13/08 05:12:01 changed by hakre

Some questions:

  • who did invent that function maybe_serialize()?
  • where is documented when this function should be used?
  • where is documented why this function has been implemented?

06/14/08 00:28:13 changed by DD32

where is documented when this function should be used? where is documented why this function has been implemented?

I'm not sure on either account, However, The use of maybe_serialize()/ maybe_unserialize() functions are used when storing metadata(post, taxonomy, options), It means that a string can be saved as a pure string(unserialised) - which reduces on operations needed, It means that a integer gets straight into the db, it means that an array gets serialized before, etc. maybe_unserialize() does the same, if the input is serialised, it spits out the unserialised version, else lets it pass straight theough

06/28/08 18:29:12 changed by jacobsantos

  • attachment 4029.testcases.diff added.

Test cases for maybe_serialize()