WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#14509 closed defect (bug) (duplicate)

WordPress Import broken by maybe_serialze

Reported by: shawnparker Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Import Keywords:
Focuses: Cc:

Description

The function maybe_serialize breaks the WordPress Import Plugin. Any serialized post_meta data that is sent over the importer is double serialized by the maybe_serialize function. The root of the problem is that within maybe_serialize there is an explicit check to see if the data is already serialized and if it is, the data is double serialized. This logic is confusing to me.

Fixing maybe_serialize or changing the way the importer imports data is what is needed. Some might think that unserializing the data in the importer is the way to go but it is not. If a plugin is not installed at the time of import and the serialized data is a custom object that requires a class provided by a plugin then the data will be destroyed.

So I guess it boils down to a question of why does maybe_serialize explicitly double serialize data. Is there a valid use case for this? I've seen the tickets for when maybe_unserialize did an is_scalar() check but that broke plugins that were "doing it wrong". Now I'm "doing it right" and breaking. I've noticed that some imported WordPress data is double serialized as well (when that data is not explicitly excluded from import by the importer) which I'm curious about the implications elsewhere.

I think a patch for this would be pretty simple.

Attachments (1)

maybe_unserialize_no_double.diff (444 bytes) - added by shawnparker 4 years ago.
patch for maybe_unserialize to not double-serialize

Download all attachments as: .zip

Change History (7)

shawnparker4 years ago

patch for maybe_unserialize to not double-serialize

comment:1 nacin4 years ago

  • Component changed from General to Import
  • Keywords import maybe_serialize removed
  • Severity changed from blocker to normal

See #12930 for the explanation of why we double-serialize.

comment:2 duck_3 years ago

We should just maybe_unserialize everything coming out of post meta tags. I believe things like nav menu item classes are currently broken.

comment:3 nacin3 years ago

Related #15030 I imagine?

comment:4 duck_3 years ago

Related: #10619 #12860

Very strange...

Changeset [12495] introduced a call to maybe_unserialize for all imported postmeta values before they are added to the database. Then [14755] removed this, but then that was reverted in [14763]. However it looks like this last reintroduction never made it into the plugin version.

comment:5 westi3 years ago

[14755] broke things.

TestImportWP_PostMeta in the unit tests has the test cases

comment:6 duck_3 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed
  • Version 3.0 deleted

http://plugins.trac.wordpress.org/changeset/306039 reintroduces maybe_unserialize. Passes updated Unit Tests (more on those soon...)

Closing as duplicate of #10619

Note: See TracTickets for help on using tickets.