WordPress version check is passing "false" value to "esc_url" causing errors (in the logs)

[jipmoors]

At least one code-path that leads to this situation is via the following line: ​https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/update.php#L197

The response of WordPress.org for core does not return a value for all "package" entries. Thus passing false to the esc_url array_map.

Array
(
    [full] => https://downloads.wordpress.org/release/wordpress-5.6.1.zip
    [no_content] => https://downloads.wordpress.org/release/wordpress-5.6.1-no-content.zip
    [new_bundled] => https://downloads.wordpress.org/release/wordpress-5.6.1-new-bundled.zip
    [partial] =>
    [rollback] =>
)

In the esc_url function, the value is checked for an empty string, but no type safety is applied.

The subsequent logic calls ltrim which expects the input to be a string.
If this is not the case, there will be an error thrown about this.

This has been added on July 1st, 2019.
See: ​https://github.com/WordPress/wordpress-develop/commit/78e096fe98531d0799c42705d1329f808e9ee944

[jipmoors]

Add a type check before using ltrim

[SergeyBiryukov]

Thanks for the patch! Linking the original changeset here for reference: [45578] / #36369.

Could you share the exact error message you're getting? I could not reproduce the issue on a clean install:

• Passing a false or null value to ltrim() does in fact seem to work as expected on all PHP versions, including PHP 8: ​https://3v4l.org/XtVCn
• The only issue I see is when passing an array() to ltrim(): ​https://3v4l.org/lXd4s. This causes a warning on PHP 7 and a fatal error on PHP 8.
• Or, passing a non-array value to array_map(): ​https://3v4l.org/IOuSW. This also causes a warning on PHP 7 and a fatal error on PHP 8.

Passing a non-string value to esc_url() is a developer error, so the developer should be notified of that. Patching the function itself to hide the warning would just pass the issue further. The function is documented to receive and return a string, so returning something else instead might have unintended side effects elsewhere. I think the current behavior is appropriate.

Passing a non-array value to array_map() in wp_version_check() seems like something we could protect against, see 52458.diff​, though I could not reproduce that scenario in my testing either, so it does not seem to occur often. For that to happen, the WP.org API would have to return a non-empty offers array with an empty packages key.

[lukecarbis]

@jipmoors Is this bug specific to the 5.7? Would it be possible to punt this ticket to 5.8, as the 5.7 beta period is almost lapsed.

[jipmoors]

@lukecarbis not specific to 5.7, as the source indicates, the code has been touched in the summer of last year.

@SergeyBiryukov I'm going to dive some deeper in the specific environment I've encountered this in. Quite possibly some plugin that is causing the specific problems.

[hellofromTonya]

With 5.7 RC1 landing tomorrow, punting this ticket to Future Release.

If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

[SergeyBiryukov]

52458.test.diff​ is an initial test for the current behavior of esc_url() with non-string values.