Skip to content
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.

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

Ensure count() is not called on a string #2888

Merged

Conversation

mattdavenport
Copy link
Contributor

This PR fixes the PHP 8.1 deprecation warning on setUploadFileId by checking the existence of preg_match values using isset() rather than count() which is now deprecated for string types.

@github-actions github-actions bot added Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* labels Jan 3, 2023
@sreichel sreichel added the PHP 8.1 Related to PHP 8.1 label Jan 3, 2023
lib/Varien/File/Uploader.php Outdated Show resolved Hide resolved
@colinmollenhour
Copy link
Member

Why not simply this?

if (preg_match('/^(\w+)\[(\w+)\]$/', $fileId, $file)) {

If the statement is true then $file[1] and $file[2] are guaranteed to be set and non-emtpy by virtue of the regex pattern (notice I changed the regex as well). Also the regex was too permissive as the subpatterns should not match special characters so there is some small chance that allowing them could be exploited somewhere later.

lib/Varien/File/Uploader.php Outdated Show resolved Hide resolved
Instead of checking the results of preg_match by counting the length of
the string, simply check for the success value. This prevents PHP 8.1
deprecation warnings.
@mattdavenport mattdavenport force-pushed the fix/php8.1/upload-countable-bug branch from 31f75b0 to 3ec2e39 Compare January 6, 2023 16:09
@mattdavenport
Copy link
Contributor Author

Code updated. Thanks everyone for the feedback!

@sreichel sreichel merged commit 844ec38 into OpenMage:1.9.4.x Jan 6, 2023
fballiano pushed a commit that referenced this pull request Jan 6, 2023
Instead of checking the results of preg_match by counting the length of
the string, simply check for the success value. This prevents PHP 8.1
deprecation warnings.
@fballiano
Copy link
Contributor

cherry-picked to 20.0 since there were no conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* PHP 8.1 Related to PHP 8.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants