WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 18 months ago

Last modified 15 months ago

#16504 closed enhancement (invalid)

Faster maybe_unserialize

Reported by: hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

In #14429 many have taken care to optimize the is_serialized speed.

The function is of good general value and most often used to "maybe" unserialize.

It was originally smabauers who reminded that it's only about to check what it's about to check.

Option values, if serialized are only serialized Array, Objects or Strings. No exceptions. And they are always trimmed strings.

As is_serialized() is of general use and might be used by plugin authors, maybe_unserialize can get a new playmate.

Attachments (1)

16504.patch (1.7 KB) - added by hakre 4 years ago.
introducing is_serialized_maybe

Download all attachments as: .zip

Change History (13)

@hakre4 years ago

introducing is_serialized_maybe

comment:1 @hakre4 years ago

  • Type changed from defect (bug) to enhancement

comment:2 follow-up: @westi4 years ago

Option values, if serialized are only serialized Array, Objects or Strings. No exceptions. And they are always trimmed strings.

This is an incorrect assumption - the API allows you to provide already serialised data if you want.

comment:3 in reply to: ↑ 2 ; follow-up: @hakre4 years ago

Replying to westi:

Option values, if serialized are only serialized Array, Objects or Strings. No exceptions. And they are always trimmed strings.

This is an incorrect assumption - the API allows you to provide already serialised data if you want.

Well that's what for the "or Strings" part of my sentence is about. You must have just missed it, my sentence is very correct indeed.

comment:4 in reply to: ↑ 3 ; follow-up: @westi4 years ago

Replying to hakre:

Replying to westi:

Option values, if serialized are only serialized Array, Objects or Strings. No exceptions. And they are always trimmed strings.

This is an incorrect assumption - the API allows you to provide already serialised data if you want.

Well that's what for the "or Strings" part of my sentence is about. You must have just missed it, my sentence is very correct indeed.

No it's not - what about serialized integers?

comment:5 in reply to: ↑ 4 ; follow-up: @hakre4 years ago

Replying to westi:

Replying to hakre:

Option values, if serialized are only serialized Array, Objects or Strings. No exceptions. [...]

No it's not - what about serialized integers?

I've created a table in my post Serialization in Options: Cant see the Wood for the Trees that handles each data case and tells whether it is serialized in options or not. It's the first table.

The "Option Values" I talk about are those that come from the database, not those that are returned by get_option(). That function will always return the unserialized value, in case of your serialized integer, the serialized integer.

If you look into the code of the patch, it becomes clear about what functions this is about.

Does this helps to better understand?

comment:6 in reply to: ↑ 5 ; follow-up: @westi4 years ago

Replying to hakre:

Replying to westi:

Replying to hakre:

Option values, if serialized are only serialized Array, Objects or Strings. No exceptions. [...]

No it's not - what about serialized integers?

I've created a table in my post Serialization in Options: Cant see the Wood for the Trees that handles each data case and tells whether it is serialized in options or not. It's the first table.

The "Option Values" I talk about are those that come from the database, not those that are returned by get_option(). That function will always return the unserialized value, in case of your serialized integer, the serialized integer.

If you look into the code of the patch, it becomes clear about what functions this is about.

Does this helps to better understand?

Not really.

You are still making an assumption: You are assuming that the only place maybe_unserialize is ever used is for unserializing things which have been maybe_serialized - you are changing it's behaviour by not supporting all types of serialized data.

It has had this behaviour since WordPress 2.0 - we are not going to break it!

comment:7 in reply to: ↑ 6 @hakre4 years ago

Replying to westi:

You are still making an assumption: You are assuming that the only place maybe_unserialize is ever used is for unserializing things which have been maybe_serialized - you are changing it's behaviour by not supporting all types of serialized data.

That's no argument against the enhancement itself. If maybe_unserialize is not the right place for that enhancement, it's easy to find that right place.

It has had this behaviour since WordPress 2.0 - we are not going to break it!

I never said I want to break it, keep cool.

comment:8 @hakre4 years ago

Related: #17375

comment:9 @c3mdigital18 months ago

  • Resolution set to wontfix
  • Status changed from new to closed

If this is still an issue please reopen and be prepared with unit tests to support. See comment:6

comment:10 @nacin18 months ago

  • Resolution changed from wontfix to invalid

This also has security implications. So, no.

comment:11 @SergeyBiryukov18 months ago

  • Milestone Awaiting Review deleted

comment:12 @SergeyBiryukov15 months ago

#26521 was marked as a duplicate.

Note: See TracTickets for help on using tickets.