[wp-trac] [WordPress Trac] #62483: maybe_serialize() does support double serialization, but does not inform the developer if doing so

[WordPress Trac]

#62483: maybe_serialize() does support double serialization, but does not inform
the developer if doing so
-------------------------+------------------------
 Reporter:  apermo       |       Owner:  audrasjb
     Type:  enhancement  |      Status:  reviewing
 Priority:  normal       |   Milestone:  6.8
Component:  General      |     Version:  3.6.1
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |     Focuses:
-------------------------+------------------------

Comment (by apermo):

 @TimothyBlynJacobs

 Sure I'm happy to.

 Well in the first place it was once disallowed 15 years ago, and only
 added back for backwards compatibility.

 Double serialization does:
 **Increase complexity of code and data structures**
 Adding manual serialization or worse mixed use of double and single
 serialization will decrease code quality and can be a source of bugs. A
 custom sniff will likely not be possible to find this.

 **Risk of data corruption**
 Single serialized code is already a pain to read and while you should not
 manually "fix" serialized data in the database, we know that people will
 occasionally do so. And having to deal with double serialization will
 increase the odds of breaking sites.

 **Decrease performance**
 While this is out of the scope of WordPress core, and serializing a string
 will not be as "expensive" as serializing a complex array, it's still
 overhead. If developers properly use the auto-serialization WordPress does
 provide, they will save one `serialize()` and `unserialize()` plus get a
 very cheap early exit on `is_serialized()` via `if ( ! is_string( $data )
 ) { return false; }`. This might not be much, but it still adds up.


 There might be more reasons, but these are those best to grasp in my
 opinion.


 ----


 In addition I've suggested a different approach to take care of this issue
 https://github.com/WordPress/wordpress-develop/pull/8434. Instead of
 adding a `_doing_it_wrong()` we could as well add a `add_action()` in the
 same spot.

 This will still allow professionals to easily monitor and improve their
 code base and projects. As well as taking @audrasjb's concern into
 account, that bystanders will not be annoyed by warnings and prevent
 another "loading text domain in time" discussion.

 The PR is green, if you guys think that is the better approach, I'm also
 happy with it. This will kind of fail the point of making less experienced
 developers aware, but still give professional teams a tool to increase
 their code quality.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/62483#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform