Page MenuHomePhabricator

Uncaught ValueError: strcspn(): Argument #4 ($length) must be contained in argument #1 ($str)
Closed, ResolvedPublic

Description

The usage of strcspn slightly changed in PHP 8.0 alpha 1, which now checks, that the lenght parameter does not exceed the length of the string to search in. This was changed in php with this commit:
https://github.com/php/php-src/commit/5d9ab53a5d53f11a18ae11ed31b17ff87c8d52a7

Occurrences where the length parameter can exceed the length of the first argument:
ValueError: strcspn(): Argument #4 ($length) must be contained in argument #1 ($str) in /code/w/includes/resourceloader/ResourceLoader.php:1899
ValueError from line 474 of /code/w/includes/parser/Preprocessor_Hash.php: strspn(): Argument #4 ($length) must be contained in argument #1 ($str)

Event Timeline

Change 631926 had a related patch set uploaded (by Florianschmidtwelzow; owner: Florianschmidtwelzow):
[mediawiki/core@master] Fix length parameter exceeding length of string for strcspn call

https://gerrit.wikimedia.org/r/631926

One more in Preprocessor_Hash, adding to the task description.

Florian renamed this task from Uncaught ValueError: strcspn(): Argument #4 ($length) must be contained in argument #1 ($str) in /code/w/includes/resourceloader/ResourceLoader.php:1899 to Uncaught ValueError: strcspn(): Argument #4 ($length) must be contained in argument #1 ($str).Oct 3 2020, 3:21 PM
Florian updated the task description. (Show Details)

It seems neither the NEWS nor UPGRADING documentation for PHP 8 alpha mentions this breaking change.

As I understand it, PHP usually only introduces exceptions for scenarios that were first given a warning (using a message with a level of core warning or deprecation warning). So for them not to do this, and not to announce it, suggests it may've been done unintentionally.

I commented upstream to let ask whether this was intentional and/or whether it will be documented or backported as warning.

Anyway, the fix is fine to go ahead either way, even if PHP 8.0 might revert this, probably better not to rely on this behaviour.

Change 631926 merged by jenkins-bot:
[mediawiki/core@master] Fix PHP 8 compat with strcspn() $length parameter exceeding string

https://gerrit.wikimedia.org/r/631926

Change 631824 had a related patch set uploaded (by Reedy; owner: Florianschmidtwelzow):
[mediawiki/core@REL1_35] Fix PHP 8 compat with strcspn() $length parameter exceeding string

https://gerrit.wikimedia.org/r/631824

Change 631824 merged by jenkins-bot:
[mediawiki/core@REL1_35] Fix PHP 8 compat with strcspn() $length parameter exceeding string

https://gerrit.wikimedia.org/r/631824

I commented upstream to let ask whether this was intentional and/or whether it will be documented or backported as warning.

I agree that this looks rather unintentional. Do you've a link to the comment upstream? :)

I created a pull request to document the change at least in the UPGRADING file: https://github.com/php/php-src/pull/6381. I don't know where the actual manual lives, despite how to update it.

@Krinkle's comment is at https://github.com/php/php-src/commit/5d9ab53a5d53f11a18ae11ed31b17ff87c8d52a7#commitcomment-42942140, but I'm afraid it might get lost there.