Skip to content

Add a file existence check in class-wp-theme-json-resolver to avoid… #10898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pbearne wants to merge 10 commits into
base: trunk
Choose a base branch
from
Open

Add a file existence check in class-wp-theme-json-resolver to avoid… #10898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3665a3f
Add a file existence check in `class-wp-theme-json-resolver` to avoid…
Feb 10, 2026
838b1dd
Update src/wp-includes/class-wp-theme-json-resolver.php
pbearne Feb 10, 2026
bbd7613
Change file existence check to readability check
pbearne Feb 18, 2026
7f38aa0
Merge branch 'trunk' into 64620-read_json_file
pbearne Jul 17, 2026
2b96383
Merge branch 'WordPress:trunk' into 64620-read_json_file
pbearne Jul 22, 2026
89e39fe
Add unit tests for `WP_Theme_JSON_Resolver::read_json_file()`
Jul 22, 2026
a68be02
Change group annotation from 'theme' to 'themes'
pbearne Oct 8, 2026
9867d5a
Merge branch 'trunk' into 64620-read_json_file
pbearne Oct 8, 2026
4dafbae
Refine `read_json_file` logic and improve unit tests for handling unr…
Oct 8, 2026
a585ce6
Improve file validation in `WP_Theme_JSON_Resolver::read_json_file()`…
Oct 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/wp-includes/class-wp-theme-json-resolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,12 @@ protected static function read_json_file( $file_path ) {
if ( array_key_exists( $file_path, static::$theme_json_file_cache ) ) {
return static::$theme_json_file_cache[ $file_path ];
}

$decoded_file = wp_json_file_decode( $file_path, array( 'associative' => true ) );
if ( is_array( $decoded_file ) ) {
static::$theme_json_file_cache[ $file_path ] = $decoded_file;
return static::$theme_json_file_cache[ $file_path ];
if ( is_file( $file_path ) && is_readable( $file_path ) ) {

@mukeshpanchal27 mukeshpanchal27 Oct 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check returns array() without any message. Before this change, wp_json_file_decode() reported the problem through wp_trigger_error(). Now a theme author with a broken or unreadable variation file gets no hint on a WP_DEBUG site.

I think the fix belongs inside wp_json_file_decode() instead. That function has two related bugs that other callers also hit, such as block.json registration:

  1. It overwrites $filename with wp_normalize_path( realpath( $filename ) ). realpath() returns false for a missing file, so the notice prints an empty path: File doesn't exist!. You can see this with wp_json_file_decode( '/does/not/exist.json' );.
  2. It calls file_get_contents() without checking is_file() and is_readable(). An unreadable file or a directory raises a PHP warning.

A check there, with a wp_trigger_error() call that keeps the original path, would fix every caller and keep the diagnostic.

$decoded_file = wp_json_file_decode( $file_path, array( 'associative' => true ) );
if ( is_array( $decoded_file ) ) {
static::$theme_json_file_cache[ $file_path ] = $decoded_file;
return static::$theme_json_file_cache[ $file_path ];
}
}
}

Expand Down
106 changes: 106 additions & 0 deletions tests/phpunit/tests/theme/wpThemeJsonResolver/readJsonFile.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
<?php

/**
* Test WP_Theme_JSON_Resolver::read_json_file().
*
* @package WordPress
* @subpackage Theme
*
* @group themes
*
* @covers WP_Theme_JSON_Resolver::read_json_file
*/
class Tests_Theme_wpThemeJsonResolver_readJsonFile extends WP_UnitTestCase {

/**
* WP_Theme_JSON_Resolver::$theme_json_file_cache property.
*
* @var ReflectionProperty
*/
private static $property_theme_json_file_cache;

/**
* Original value of the WP_Theme_JSON_Resolver::$theme_json_file_cache property.
*
* @var array
*/
private static $property_theme_json_file_cache_orig_value;

public static function set_up_before_class() {
parent::set_up_before_class();

static::$property_theme_json_file_cache = new ReflectionProperty( WP_Theme_JSON_Resolver::class, 'theme_json_file_cache' );
if ( PHP_VERSION_ID < 80100 ) {
static::$property_theme_json_file_cache->setAccessible( true );
}
static::$property_theme_json_file_cache_orig_value = static::$property_theme_json_file_cache->getValue();
}

public static function tear_down_after_class() {
static::$property_theme_json_file_cache->setValue( null, static::$property_theme_json_file_cache_orig_value );
parent::tear_down_after_class();
}

public function tear_down() {
// Reset data between tests.
static::$property_theme_json_file_cache->setValue( null, array() );
parent::tear_down();
}

/**
* @ticket 64620
*/
public function test_read_json_file() {

@mukeshpanchal27 mukeshpanchal27 Oct 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one method covers six scenarios. The first failing assertion hides the rest. Could you split it into separate test methods or use a data provider?

The unreadable-file case also skips itself without telling PHPUnit. On Windows, without the posix extension, or when tests run as root, it reports a pass for code it never ran. $this->markTestSkipped() in its own method would show this.

$read_json_file = new ReflectionMethod( WP_Theme_JSON_Resolver::class, 'read_json_file' );
if ( PHP_VERSION_ID < 80100 ) {
$read_json_file->setAccessible( true );
}

// Test reading a valid JSON file.
$valid_file = DIR_TESTDATA . '/themedir1/block-theme/theme.json';
$result = $read_json_file->invoke( null, $valid_file );
$this->assertIsArray( $result );
$this->assertArrayHasKey( 'version', $result );
$this->assertSame( 3, $result['version'] );

// Test that the result is cached.
$cache = static::$property_theme_json_file_cache->getValue();
$this->assertArrayHasKey( $valid_file, $cache );
$this->assertSame( $result, $cache[ $valid_file ] );

// Test cache hit: modify cache and verify read_json_file returns cached value.
$cache[ $valid_file ] = array(
'version' => 3,
'cached' => true,
);
static::$property_theme_json_file_cache->setValue( null, $cache );
$result = $read_json_file->invoke( null, $valid_file );
$this->assertSame( 3, $result['version'] );
$this->assertTrue( $result['cached'] );

// Test non-existent file.
$non_existent_file = DIR_TESTDATA . '/non-existent.json';
$result = $read_json_file->invoke( null, $non_existent_file );
$this->assertSame( array(), $result );

// Test unreadable file.
if ( function_exists( 'posix_getpwuid' ) && function_exists( 'posix_geteuid' ) ) {
$pwuid = posix_getpwuid( posix_geteuid() );
if ( is_array( $pwuid ) && isset( $pwuid['name'] ) && 'root' !== $pwuid['name'] ) {
$unreadable_file = tempnam( sys_get_temp_dir(), 'unreadable-json' );

@mukeshpanchal27 mukeshpanchal27 Oct 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the assertion below fails, chmod() and unlink() never run. A file with mode 0000 then stays in the system temp folder. The invalid-JSON temp file has the same problem. Moving cleanup into tear_down() or a try/finally block would fix this.

chmod( $unreadable_file, 0000 );
$result = $read_json_file->invoke( null, $unreadable_file );
$this->assertSame( array(), $result );
chmod( $unreadable_file, 0644 );
unlink( $unreadable_file );
}
}

// Test invalid JSON.
$invalid_json_file = tempnam( sys_get_temp_dir(), 'invalid-json' );
file_put_contents( $invalid_json_file, '{ invalid json }' );
$result = @$read_json_file->invoke( null, $invalid_json_file );

Check warning on line 102 in tests/phpunit/tests/theme/wpThemeJsonResolver/readJsonFile.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Silencing errors is strongly discouraged. Use proper error checking instead. Found: @$read_json_file->invoke( ...

@mukeshpanchal27 mukeshpanchal27 Oct 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @ hides the decode notice instead of checking that it fires. If the error reporting broke later, this test would still pass. Could you assert the notice with expectNotice() or the wp_trigger_error_run action? A matching assertion for the missing and unreadable cases would also cover the behavior change in the resolver.

$this->assertSame( array(), $result );
unlink( $invalid_json_file );
}
}
Loading