Skip to content

Harden against undefined index in _load_script_textdomain_from_src() and add types for wp_parse_url() #11690

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
westonruter wants to merge 6 commits into
base: trunk
Choose a base branch
from
Open

Harden against undefined index in _load_script_textdomain_from_src() and add types for wp_parse_url() #11690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2ce9b02
Add types for wp_parse_url() and dependent functions
westonruter Apr 30, 2026
bfe0eca
Handle possible undefined path index on src_url and fix type issues i…
westonruter Apr 30, 2026
8f38b36
Fall back to load_script_translations() when failing to parse src or …
westonruter Apr 30, 2026
b230450
Add unit tests for new fallback returns in _load_script_textdomain_fr…
westonruter May 1, 2026
f3e294e
Simplify test for non-string relative path filter
westonruter May 1, 2026
9dcef48
Use MockAction for content_url test, drop spy from the others
westonruter May 1, 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
53 changes: 53 additions & 0 deletions src/wp-includes/http.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,26 @@ function ms_allowed_http_request_hosts( $is_external, $host ) {
* When a specific component has been requested: null if the component
* doesn't exist in the given URL; a string or - in the case of
* PHP_URL_PORT - integer when it does. See parse_url()'s return values.
*
* @phpstan-param int<-1, 7> $component
* @phpstan-return (
* $component is -1
* ? false|array{
* scheme?: string,
* host?: string,
* port?: int<0, 65535>,
* user?: string,
* pass?: string,
* path?: string,
* query?: string,
* fragment?: string,
* }
* : (
* $component is 2
* ? int<0, 65535>|null
* : string|null
* )
Comment thread
westonruter marked this conversation as resolved.
* )
*/
function wp_parse_url( $url, $component = -1 ) {
$to_unset = array();
Expand Down Expand Up @@ -763,6 +783,36 @@ function wp_parse_url( $url, $component = -1 ) {
* When a specific component has been requested: null if the component
* doesn't exist in the given URL; a string or - in the case of
* PHP_URL_PORT - integer when it does. See parse_url()'s return values.
*
* @phpstan-param false|array{
* scheme?: string,
* host?: string,
* port?: int<0, 65535>,
* user?: string,
* pass?: string,
* path?: string,
* query?: string,
* fragment?: string,
* } $url_parts
* @phpstan-param int<-1, 7> $component
* @phpstan-return (
* $component is -1
* ? false|array{
* scheme?: string,
* host?: string,
* port?: int<0, 65535>,
* user?: string,
* pass?: string,
* path?: string,
* query?: string,
* fragment?: string,
* }
* : (
* $component is 2
* ? int<0, 65535>|null
* : string|null
Comment thread
westonruter marked this conversation as resolved.
* )
* )
*/
function _get_component_from_parsed_url_array( $url_parts, $component = -1 ) {
if ( -1 === $component ) {
Expand All @@ -789,6 +839,9 @@ function _get_component_from_parsed_url_array( $url_parts, $component = -1 ) {
*
* @param int $constant PHP_URL_* constant.
* @return string|false The named key or false.
*
* @phpstan-param int<-1, 7> $constant
* @phpstan-return 'scheme'|'host'|'port'|'user'|'pass'|'path'|'query'|'fragment'|false
*/
function _wp_translate_php_url_constant_to_key( $constant ) {
$translation = array(
Expand Down
18 changes: 15 additions & 3 deletions src/wp-includes/l10n.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,7 @@ function load_script_module_textdomain( string $id, string $domain = 'default',
* @return string|false The JSON-encoded translated strings on success, false otherwise.
*/
function _load_script_textdomain_from_src( string $handle, string $src, string $domain, string $path, bool $is_module ) {
/** @var WP_Textdomain_Registry $wp_textdomain_registry */
Copy link
Copy Markdown
Member

@mukeshpanchal27 mukeshpanchal27 May 1, 2026

Choose a reason for hiding this comment

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

Something that globally fix in #11692. Can we leave this for this PR

global $wp_textdomain_registry;

$locale = determine_locale();
Expand All @@ -1214,7 +1215,9 @@ function _load_script_textdomain_from_src( string $handle, string $src, string $
$path = $wp_textdomain_registry->get( $domain, $locale );
}

$path = untrailingslashit( $path );
if ( $path ) {
Copy link
Copy Markdown
Member Author

@westonruter westonruter Apr 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Conditional added because the $path can still be false here since $wp_textdomain_registry->get() returns string|false.

$path = untrailingslashit( $path );
}

// If a path was given and the handle file exists simply return it.
$file_base = 'default' === $domain ? $locale : $domain . '-' . $locale;
Expand All @@ -1231,8 +1234,17 @@ function _load_script_textdomain_from_src( string $handle, string $src, string $
$relative = false;
$languages_path = WP_LANG_DIR;

$src_url = wp_parse_url( $src );
$src_url = wp_parse_url( $src );
if ( ! $src_url ) {
return load_script_translations( false, $handle, $domain );
}
$src_url['path'] ??= '';
Copy link
Copy Markdown
Member Author

@westonruter westonruter Apr 30, 2026

Choose a reason for hiding this comment

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

This is the crux for fixing the undefined index error.

Copy link
Copy Markdown
Member

@manzoorwanijk manzoorwanijk May 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

should we cover these bail-outs with unit tests?

Copy link
Copy Markdown
Member Author

@westonruter westonruter May 1, 2026

Choose a reason for hiding this comment

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

Yeah, that would be idea. I've added full coverage in b230450


$content_url = wp_parse_url( content_url() );
if ( ! $content_url ) {
return load_script_translations( false, $handle, $domain );
}

$plugins_url = wp_parse_url( plugins_url() );
$site_url = wp_parse_url( site_url() );
$theme_root = get_theme_root();
Expand Down Expand Up @@ -1304,7 +1316,7 @@ function _load_script_textdomain_from_src( string $handle, string $src, string $
$relative = apply_filters( 'load_script_textdomain_relative_path', $relative, $src, $is_module );

// If the source is not from WP.
if ( false === $relative ) {
if ( ! is_string( $relative ) ) {
Copy link
Copy Markdown
Member Author

@westonruter westonruter Apr 30, 2026

Choose a reason for hiding this comment

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

Since apply_filters() can return anything, safer to check for a string specifically than for equality with false.

return load_script_translations( false, $handle, $domain );
}

Expand Down
98 changes: 98 additions & 0 deletions tests/phpunit/tests/l10n/loadScriptTextdomain.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,102 @@ public function test_does_not_throw_deprecation_notice_for_rtrim_with_default_pa
$expected = file_get_contents( DIR_TESTDATA . '/languages/en_US-813e104eb47e13dd4cc5af844c618754.json' );
$this->assertSame( $expected, load_script_textdomain( $handle ) );
}

/**
* Tests that an unparseable script source URL short-circuits to
* `load_script_translations( false, ... )` instead of falling through
* to the relative-path computation.
*
* @ticket 65015
*/
public function test_unparseable_src_returns_false(): void {
$handle = 'test-unparseable-src';
$src = 'http:///example';

$this->assertFalse( wp_parse_url( $src ), 'Test prerequisite failed: the test src should be unparseable.' );

wp_enqueue_script( $handle, $src, array(), null );

$this->assertFalse( load_script_textdomain( $handle, 'default', DIR_TESTDATA . '/languages' ) );
}

/**
* Tests that an unparseable `content_url()` return value short-circuits
* to `load_script_translations( false, ... )` instead of computing
* `$relative` from a corrupted parsed-URL array.
*
* The `MockAction` spy on `pre_load_script_translations` is necessary
* here because the function's tail end also calls `load_script_translations( false, ... )`,
* so a regression that bypasses the early return would still return false
* via the fallback path. Asserting on the recorded `$file` arguments pins
* the test to the intended branch.
*
* @ticket 65015
*/
public function test_unparseable_content_url_returns_false(): void {
$handle = 'test-unparseable-content-url';
$src = '/wp-includes/js/script.js';

add_filter(
'content_url',
static function () {
return 'http:///example';
}
);

$mock = new MockAction();
add_filter( 'pre_load_script_translations', array( $mock, 'filter' ), 10, 4 );

wp_enqueue_script( $handle, $src, array(), null );

$this->assertFalse( load_script_textdomain( $handle, 'default', DIR_TESTDATA . '/languages' ) );
$this->assertSame(
array(
DIR_TESTDATA . '/languages/en_US-' . $handle . '.json',
false,
),
array_column( $mock->get_args(), 1 ),
'Expected the unparseable content_url branch to short-circuit before any relative-path lookup.'
);
}

/**
* Tests that the `load_script_textdomain_relative_path` filter returning
* a non-string, non-false value (e.g., a callback that forgets to return)
* short-circuits via the `! is_string( $relative )` guard rather than
* falling through to string functions like `str_ends_with()` and `md5()`.
*
* @ticket 65015
*/
public function test_non_string_relative_path_filter_returns_false(): void {
$handle = 'test-non-string-relative-path';
$src = '/wp-includes/js/script.js';

add_filter( 'load_script_textdomain_relative_path', '__return_null' );

wp_enqueue_script( $handle, $src, array(), null );

$this->assertFalse( load_script_textdomain( $handle, 'default', DIR_TESTDATA . '/languages' ) );
}

/**
* Tests that a script source URL with no path component does not trigger
* an undefined index warning when the path is read further down in the
* function. The result is reached via the regular fallback path
* (no host/path match) rather than an early return.
*
* @ticket 65015
*/
public function test_src_without_path_component_does_not_warn(): void {
$handle = 'test-src-without-path';
$src = 'https://example.com';

$parsed = wp_parse_url( $src );
$this->assertIsArray( $parsed, 'Test prerequisite failed: the test src should parse.' );
$this->assertArrayNotHasKey( 'path', $parsed, 'Test prerequisite failed: the test src should have no path component.' );

wp_enqueue_script( $handle, $src, array(), null );

$this->assertFalse( load_script_textdomain( $handle, 'default', DIR_TESTDATA . '/languages' ) );
}
}
Loading