Make WordPress Core

Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#7383 closed enhancement (wontfix)

maybe_serialize() should serialize all non-string values

Reported by: dd32's profile DD32 Owned by:
Milestone: Priority: normal
Severity: trivial Version: 2.6
Component: General Keywords: has-patch dev-feedback wont-fix not-a-bug invalid
Focuses: Cc:

Description

update_option('test', 45);
var_dump( get_option('test') );

Will output "int 45", However, on subsequent page loads (ie. Once the cache is cleared) get_option will return 45 as a string "string '45'"

The issue is that maybe_serialize only serialises arrays, objects, and allready serialized data.
Integers, Booleans, Floats, etc pass right through, (int)45 !== (string)'45' - Which causes problems for plugins which might store a boolean, or a integer in a option (And rely upon === for comparisons for example).

I've attached a patch which serializes all non-string values, while respecting that serialised strings need to be serialized again.

Attachments (1)

7383.diff (933 bytes) - added by DD32 14 years ago.

Download all attachments as: .zip

Change History (12)

@DD32
14 years ago

#1 @jacobsantos
14 years ago

Lets just hope no one passes a resource to the function.

#2 @DD32
14 years ago

Lets just hope no one passes a resource to the function.

If you serialize a resource, It gets saved as an integer :)

This also affects Post meta, So it might be worth looking into seeing if theres any effects of this change there.

I'm going to work around this in the specific plugin by storing the value in an array.. I was going to have to move it to an array at some stage anyway.

#3 @jacobsantos
14 years ago

That is because you can't serialize a resource. You can't unserialize the serialized resource and get the handle to the resource back, you just get a number which doesn't do you any good.

#4 follow-up: @DD32
14 years ago

Ah yeah, thats what i meant. I thought you were possibly refering to attempting to serialize a resource would cause something to break.

If someones passing a resource to maybe_serialize then IMHO, thats the least of the functions problem - why's it being passed in to start with.

#5 in reply to: ↑ 4 @santosj
14 years ago

Replying to DD32:

Ah yeah, thats what i meant. I thought you were possibly refering to attempting to serialize a resource would cause something to break.

If someones passing a resource to maybe_serialize then IMHO, thats the least of the functions problem - why's it being passed in to start with.


LOL. Okay, I agree.

#6 @jacobsantos
14 years ago

  • Type changed from defect to enhancement

#7 @DD32
14 years ago

  • Milestone changed from 2.7 to 2.9

#8 @Denis-de-Bernardy
13 years ago

  • Keywords dev-feedback added

personally, I think that checking for is_numeric() would get the job done in a more efficient manner. as in, store the numeric option as a string (no serialization required, i.e. fast). then, when instantiating the option, use is_numeric() (fast) and convert to float or string as necessary if it is numeric.

boolean is a bit different imo. if my memory serves me well, it currently gets serialized, and it probably should get converted to an integer before getting stored. that was get_option() would only return false when the option is undefined.

#9 @DD32
13 years ago

Yeah.. I'm tempted to close as wontfix and leave us people to work around it.

The default value of 'false' being returned is nothing to be worried about, its easy enough to work around with the default option value (get_option('non-exitant-option', null); === null)

As a plugin author, you should know what datatype is stored, so (float)get_option('my-option', 0.00); is easy enough.. And if you've got multiple items.. its easier to use a array.. which will have its types stored..

#10 @hakre
13 years ago

  • Cc hanskrentel@… added
  • Keywords wont-fix not-a-bug invalid added
  • Resolution set to wontfix
  • Severity changed from normal to trivial
  • Status changed from new to closed

There is no need for a fix because this is not a Bug. I would like to see it as invalid but set it to wontfix first.

The original reporter must have missed something. This is PHP (and WordPress) and there are no such variable-types like string or integer in PHP. It is called Scalar and it can be any of the types at any time like string, integer, bool, float but not resource, object or array.

Additionally serialization is defined in the PHP lanugage as well.

Please check PHP Docs if you are unfamilar with the concept:
http://php.net/manual/en/language.types.intro.php
http://php.net/manual/en/function.serialize.php
http://php.net/manual/en/function.unserialize.php

the function maybe_serialize does not clearly say what it does (I've critized this already) so therefore the wording "when it's needed" might be misleading. It only means, that if there is a value to store into the database as option which is of type array or object it should be serialized (so therefore it can be stored in there with an SQL query). Right?

#11 @jacobsantos
13 years ago

  • Milestone 2.9 deleted
Note: See TracTickets for help on using tickets.