WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#12860 closed defect (bug) (fixed)

Stripslashes_deep doesn't handle arrays of objects well.

Reported by: Otto42 Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version: 3.0
Component: Formatting Keywords:
Focuses: Cc:

Description

Installed 3.0 beta 1. Tried to import a fresh WXR XML file from another blog (running 2.9.2). Importer fails after the first post with the following error:

Catchable fatal error: Object of class stdClass could not be converted to string in /.../wp-includes/formatting.php on line 1228

Change History (13)

comment:1 @Otto425 years ago

  • Severity changed from critical to major

Update: After some debugging, I determined the cause to be this data in postmeta:

<wp:postmeta>
<wp:meta_key>_utw_tags_0</wp:meta_key>
<wp:meta_value>a:13:{i:0;O:8:"stdClass":1:{s:3:"tag";s:5:"album";}i:1;O:8:"stdClass":1:{s:3:"tag";s:5:"apple";}i:2;O:8:"stdClass":1:{s:3:"tag";s:3:"art";}i:3;O:8:"stdClass":1:{s:3:"tag";s:7:"artwork";}i:4;O:8:"stdClass":1:{s:3:"tag";s:11:"dead-tracks";}i:5;O:8:"stdClass":1:{s:3:"tag";s:4:"ipod";}i:6;O:8:"stdClass":1:{s:3:"tag";s:6:"itunes";}i:7;O:8:"stdClass":1:{s:3:"tag";s:10:"javascript";}i:8;O:8:"stdClass":1:{s:3:"tag";s:6:"lyrics";}i:9;O:8:"stdClass":1:{s:3:"tag";s:6:"script";}i:10;O:8:"stdClass":1:{s:3:"tag";s:6:"tracks";}i:11;O:8:"stdClass":1:{s:3:"tag";s:22:"windows-scripting-host";}i:12;O:8:"stdClass":1:{s:3:"tag";s:7:"wscript";}}</wp:meta_value>
</wp:postmeta>

That's an old Ultimate Tag Warrior entry I had never cleaned out. Removing all those allowed the importer to continue, albeit not without minor errors here and there.

Still, this should get some error handling, I think.

comment:2 @mrickert815 years ago

I came across this error as well. I just went into my database and deleted all the entries from the wp_postmeta table that had the meta_key of "_utw_tags_0"

After that, I exported my XML file again from the master blog and ran the import process again on a fresh WP 3.0beta1 install. Worked great that time.

comment:3 @yoavf5 years ago

Related: #13263

comment:4 @automattor5 years ago

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

(In [14755]) add_metadata() already stripslashes & unserializes metadata, fixes #12860

comment:5 @westi5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Re-opening.

That code was introduced on purpose because otherwise post meta didn't import and I have tests for that which no appear to be broken:

comment:6 @westi5 years ago

Basically this breaks all the tests I wrote when fixing - #10619

TestImportWP_PostMeta and TestImportWP_PostMetaCDATA

comment:7 @westi5 years ago

(In [14763]) Revert [14755] as this breaks post_meta importing. See #12860.

comment:8 follow-up: @westi5 years ago

This looks like possibly a general add_post_meta with array of objects issue.

comment:9 in reply to: ↑ 8 @westi5 years ago

Replying to westi:

This looks like possibly a general add_post_meta with array of objects issue.

Confirmed.

And I get the same php notice in 2.9 as well:

http://svn.automattic.com/wordpress-tests/wp-testcase/test_includes_post.php
WPTestPostMeta::test_funky_post_meta

comment:10 @westi5 years ago

  • Component changed from Import to Formatting
  • Priority changed from high to normal
  • Severity changed from major to normal
  • Summary changed from WordPress importer failure to Stripslashes_deep doesn't handle arrays of objects well.

Re-titling.

comment:11 @automattor5 years ago

(In [14766]) Attempt to make stripslashes_deep object safe. See #12860

comment:12 follow-up: @nacin5 years ago

Generally we see failures in stripslashes_deep for WP_Error objects that aren't properly checked for earlier and end up getting passed in. Wondering if we should account for that.

comment:13 in reply to: ↑ 12 @westi5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to nacin:

Generally we see failures in stripslashes_deep for WP_Error objects that aren't properly checked for earlier and end up getting passed in. Wondering if we should account for that.

We could do - but what would we do - return another WP_Error / wp_die.

I think we have to take a pragmatic approach and expect plugin devs to check things for WP_Error and not have lots of checks all over the place in utility functions for it.

I think we can safely close this down now as I've not seen any fallout from the change.

We can revisit it in the future if necessary but education may be a better solution.

Note: See TracTickets for help on using tickets.