Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#22883 closed defect (bug) (fixed)

get_page_uri() can produce PHP Warnings when non-WP_Post object instances are passed

Reported by: dd32's profile dd32 Owned by: nacin's profile nacin
Milestone: 3.5.1 Priority: normal
Severity: normal Version: 3.5
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

A common PHP Warning being posted on the forums includes the following:

Warning: Invalid argument supplied for foreach() in wp-includes/post.php on line 3588

This is being caused by the lazy-loading of the ancestor field, with non-WP_Post instances being passed in.

Although I haven't tracked down how themes are using it, using the following code in the loop to simulate a WP_Post object being converted to a standard StdClass instance duplicates the issue:

var_dump( get_page_uri( (object)(array)$post ) );

The change to fix this seems like an obvious change to me, so I've not yet dug into the reasonings behind themes triggering it.

I've seen the Warning in the wild in a few places, but an example forum thread: http://wordpress.org/support/topic/warning-invalid-argument-supplied-for-foreach-in-homeftpublic_htmlwp-incl?

Patch attached, milestone set for review purposes.

Attachments (1)

22883.diff (428 bytes) - added by dd32 11 years ago.
Upgrade objects to WP_Post objects if passed in

Download all attachments as: .zip

Change History (11)

@dd32
11 years ago

Upgrade objects to WP_Post objects if passed in

#1 @nacin
11 years ago

Definitely a good change. I imagine we'll find random code throughout WP that might need to be updated to call get_post() unconditionally.

#2 @ocean90
11 years ago

#22945 was marked as a duplicate.

#3 follow-up: @lukbarros
11 years ago

it is possible to track who placed this if and when? He can this ai for some other purpose correction.

#4 in reply to: ↑ 3 @SergeyBiryukov
11 years ago

Replying to lukbarros:

it is possible to track who placed this if and when?

Introduced in [14399].

#5 @nacin
11 years ago

  • Keywords commit needs-unit-tests added
  • Version set to 3.5

Seems good.

#6 @georgestephanis
11 years ago

Is there any chance that someone would pass in an associative array version of the post? I don't expect it, but I don't wear the tin foil hat for nothin'.

Apart from that, the change looks good and sane, +1.

#7 @nacin
11 years ago

In 1175/tests:

Test for get_page_uri() producing PHP warnings on non-WP_Post post objects. see #22883.

#8 @nacin
11 years ago

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

In 23208:

Always call get_post() in get_page_uri() to ensure we have a WP_Post object, which lazy-loads the ancestors this function requires.

props dd32
fixes #22883
Unit tests: [1175/tests]

#9 @nacin
11 years ago

In 23209:

Always call get_post() in get_page_uri() to ensure we have a WP_Post object, which lazy-loads the ancestors this function requires.

Merges [23208] to the 3.5 branch.
props dd32.
fixes #22883.

#10 @nacin
11 years ago

  • Keywords needs-unit-tests removed
Note: See TracTickets for help on using tickets.