WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 6 months ago

Last modified 6 months ago

#39277 closed defect (bug) (fixed)

get_post_galleries() produces PHP Warning in PHP 7.1 when parsing empty shortcode

Reported by: DJPaul Owned by: dd32
Milestone: 4.7.3 Priority: normal
Severity: normal Version: 4.7
Component: Shortcodes Keywords: needs-patch needs-unit-tests
Focuses: Cc:

Description

With WordPress 4.7 and PHP 7.1, get_post_galleries() produces a PHP Warning when given an empty gallery shortcode:

Illegal string offset 'src'
/vagrant/wp/wp-includes/media.php:3696

This broke a probably-useless unit test in BuddyPress, but it will at least cause a PHP Warning if a certain common code path is triggered in production (and probably cause a regression in functionality).

Here's an example PHPUnit test you can copy/paste in to quickly see the issue. Compare it on PHP 7.1 against PHP < 7.1.

public function test_this_breaks_on_php71() {
	$post_id = $this->factory->post->create( array(
		'post_content' => 'blah [gallery] blah',
	) );
	$gallery = get_post_galleries( $post_id, false );
	$this->assertSame( array( array( 'src' => array() ) ), $gallery );
}

I think returning an empty array, rather than an pair of nested arrays with only one src key set, is actually better/more expected behaviour.

Change History (7)

#1 @pento
8 months ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.7.1

shortcode_parse_atts() can return an array or a string, this isn't being tested before treating the return value as an array.

I've audited all other usage of shortcode_parse_atts() in Core, nothing else suffers from a similar problem. There was one minor documentation issue, tracked in #39294.

#2 @dd32
8 months ago

While looking at this, I ran into #39304, 39304.diff includes a workaround for this case by assuming an empty array in the event a non-array is returned (Which seems like the expected behaviour here).

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 months ago

#4 @jbpaul17
7 months ago

  • Milestone changed from 4.7.1 to 4.7.2

Per today's bug scrub in #core: this will be punted to 4.7.2 as it won't be ready for commit in time for 4.7.1.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


6 months ago

#6 @dd32
6 months ago

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

In 40070:

Media: Avoid PHP Warnings in get_post_galleries() when processing empty [gallery] shortcodes and avoid returning the incorrect results when the global $post does not match the provided post ID.

Props dd32, joemcgill, seanchayes.
Fixes #39277, #39304.

#7 @dd32
6 months ago

In 40071:

Media: Avoid PHP Warnings in get_post_galleries() when processing empty [gallery] shortcodes and avoid returning the incorrect results when the global $post does not match the provided post ID.

Props dd32, joemcgill, seanchayes.
Merges [40070] to the 4.7 branch.
Fixes #39277, #39304.

Note: See TracTickets for help on using tickets.