Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#53599 closed defect (bug) (fixed)

WP_Theme::get_files() returns unexpected entry if parent theme not available.

Reported by: dd32's profile dd32 Owned by: audrasjb's profile audrasjb
Milestone: 6.0 Priority: normal
Severity: minor Version:
Component: Themes Keywords: good-first-bug has-patch has-unit-tests commit assigned-for-commit
Focuses: Cc:

Description

Given the following code, you'll get a slightly unexpected output if the parent theme is not available.

$theme = new WP_Theme( ... );
$files = $theme->get_files(
	null /* all file types */,
	-1 /* infinite recursion */,
	true /* include parent theme files */
);
var_dump( $files );

Expected output:

wp-content/plugins/example/example.php:4:
array (size=1)
  'index.php' => string '/tmp/test-theme/index.php' (length=25)

Actual output:

wp-content/plugins/example/example.php:4:
array (size=2)
  'index.php' => string '/tmp/test-theme/index.php' (length=25)
   0 => boolean false

I believe the inclusion of 0 as the filename, and false as the path is rather unexpected

Attachments (1)

53599.patch (650 bytes) - added by opurockey 3 years ago.
Added patch.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
3 years ago

  • Milestone changed from Awaiting Review to 5.9

@opurockey
3 years ago

Added patch.

#2 @karpstrucking
3 years ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


2 years ago

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


2 years ago

#5 @hellofromTonya
2 years ago

  • Milestone changed from 5.9 to 6.0
  • Owner set to hellofromTonya
  • Status changed from new to assigned

I wasn't able to reproduce the problem. As this also needs tests, assigning ownership to me to investigate and get the tests done. Moving to 6.0 as 5.9 Beta 1 is tomorrow and this needs more time to get it ready.

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


2 years ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


2 years ago

This ticket was mentioned in PR #2552 on WordPress/wordpress-develop by Rahmon.


2 years ago
#8

  • Keywords has-unit-tests added; needs-unit-tests removed

This PR changes the get_files method to not return false values in the array of files and adds tests for this case.

Trac ticket: https://core.trac.wordpress.org/ticket/53599

#9 @Rahmohn
2 years ago

WP_Theme::get_files() will return an entry with false value when the directory of the theme doesn't exist. So it's possible to reproduce this issue with the code below:

<?php
$theme = wp_get_theme( 'non-existent-directory-name' );

var_dump( $theme->get_files() );

/**
 * Output:
 *
 * array (size=1)
 *   0 => boolean false
 */

This is happening because WP_Theme::get_files() uses the method WP_Theme::scandir() to retrieve the files and this method can return false if the path doesn't exist or isn't a directory.

Although the patch added by opurockey can fix the issue when the parent theme is not available, it doesn't for the case I described above.

So, I've changed the WP_Theme::get_files() to filter out empty entries before returning the array of files. Other than that, I've added tests for this scenario.

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


2 years ago

#11 @audrasjb
2 years ago

  • Owner changed from hellofromTonya to audrasjb
  • Status changed from assigned to reviewing

The PR looks good to me.
Reassigning this ticket for final review.
I updated the above PR to add a small change on Tests docblocks.

#12 @audrasjb
2 years ago

  • Keywords commit assigned-for-commit added

#13 @audrasjb
2 years ago

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

In 53253:

Themes: Ensure WP_Theme::get_files() doesn't return unexpected values.

This change filters out empty entries from WP_Theme::get_files() before returning the array of files. This avoid returning an entry with false value when the directory of the theme does not exist.

Props dd32, opurockey, Rahmohn, audrasjb.
Fixes #53599.

Note: See TracTickets for help on using tickets.