Skip to content

Commit

Permalink
PHP 8.1 | load_script_textdomain(): fix passing null to non-nullable
Browse files Browse the repository at this point in the history
The `$path` parameter has a default value of `null`, but would be passed onto the `untrailingslashit()` function without any input validation, even though the `untrailingslashit()` function explicitly only expects/supports a string input.

Note: I'm explicitly not changing the `untrailingslashit()` function, but fixing this at the point where the invalid input is incorrectly (not) validated.

Includes adding a dedicated unit test for this issue.

This fix also allows to remove a couple of calls to `expectDeprecation()` in unrelated tests.

Fixes:
```
4) Tests_Dependencies_Scripts::test_wp_external_wp_i18n_print_order
rtrim(): Passing null to parameter #1 ($string) of type string is deprecated

/var/www/src/wp-includes/formatting.php:2782
/var/www/src/wp-includes/l10n.php:1068
/var/www/src/wp-includes/class.wp-scripts.php:605
/var/www/src/wp-includes/class.wp-scripts.php:320
/var/www/src/wp-includes/class.wp-dependencies.php:136
/var/www/src/wp-includes/functions.wp-scripts.php:109
/var/www/tests/phpunit/tests/dependencies/scripts.php:1505
/var/www/tests/phpunit/includes/utils.php:436
/var/www/tests/phpunit/tests/dependencies/scripts.php:1507
/var/www/vendor/bin/phpunit:123
```
  • Loading branch information
jrfnl committed Sep 27, 2022
1 parent 85106c6 commit 46f50cc
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 21 deletions.
4 changes: 3 additions & 1 deletion src/wp-includes/l10n.php
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,9 @@ function load_script_textdomain( $handle, $domain = 'default', $path = null ) {
return false;
}

$path = untrailingslashit( $path );
if ( is_string( $path ) ) {
$path = untrailingslashit( $path );
}
$locale = determine_locale();

// If a path was given and the handle file exists simply return it.
Expand Down
20 changes: 0 additions & 20 deletions tests/phpunit/tests/dependencies/scripts.php
Original file line number Diff line number Diff line change
Expand Up @@ -721,16 +721,6 @@ public function test_wp_add_inline_script_before_after_concat_with_core_dependen
$wp_scripts->base_url = '';
$wp_scripts->do_concat = true;

if ( PHP_VERSION_ID >= 80100 ) {
/*
* For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
* via hooked in filter functions until a more structural solution to the
* "missing input validation" conundrum has been architected and implemented.
*/
$this->expectDeprecation();
$this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
}

$ver = get_bloginfo( 'version' );
$suffix = wp_scripts_get_suffix();
$expected = "<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5Bchunk_0%5D=jquery-core,jquery-migrate,regenerator-runtime,wp-polyfill,wp-dom-ready,wp-hooks&amp;ver={$ver}'></script>\n";
Expand Down Expand Up @@ -783,16 +773,6 @@ public function test_wp_add_inline_script_customize_dependency() {
$wp_scripts->base_url = '';
$wp_scripts->do_concat = true;

if ( PHP_VERSION_ID >= 80100 ) {
/*
* For the time being, ignoring PHP 8.1 "null to non-nullable" deprecations coming in
* via hooked in filter functions until a more structural solution to the
* "missing input validation" conundrum has been architected and implemented.
*/
$this->expectDeprecation();
$this->expectDeprecationMessageMatches( '`Passing null to parameter \#[0-9]+ \(\$[^\)]+\) of type [^ ]+ is deprecated`' );
}

$expected_tail = "<script type='text/javascript' src='/customize-dependency.js' id='customize-dependency-js'></script>\n";
$expected_tail .= "<script type='text/javascript' id='customize-dependency-js-after'>\n";
$expected_tail .= "tryCustomizeDependency()\n";
Expand Down
13 changes: 13 additions & 0 deletions tests/phpunit/tests/l10n/loadScriptTextdomain.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,17 @@ public function relative_path_from_cdn( $relative, $src ) {

return $relative;
}

/**
* @ticket 55656
*/
public function test_no_php81_notices_when_optional_params_not_passed() {
$handle = 'test-example-root';
$src = '/wp-includes/js/script.js';

wp_enqueue_script( $handle, $src );

$expected = file_get_contents( DIR_TESTDATA . '/languages/en_US-813e104eb47e13dd4cc5af844c618754.json' );
$this->assertSame( $expected, load_script_textdomain( $handle ) );
}
}

0 comments on commit 46f50cc

Please sign in to comment.