Skip to content

Conversation

@iNem0o
Copy link

@iNem0o iNem0o commented Jun 24, 2025

Hello !

Questions Answers
Description? The scandir() PHP function can return false if the path does not exist or is not a directory (e.g., due to permission issues). In the current code, this false value is passed directly to preg_grep(), which expects its second argument to be an array. This causes a TypeError: preg_grep(): Argument #2 ($array) must be of type array, bool given.

This PR adds a check to verify the return value of scandir(). If it returns false, the property $this->imageFiles is initialized as an empty array [].
Type? bug fix
BC breaks? no
Deprecations? no
How to test? This bug appears when the category image directory (/img/c/) is missing or not readable when generating the module widget variables.

boherm
boherm previously approved these changes Jun 25, 2025
Copy link
Member

@boherm boherm left a comment

Choose a reason for hiding this comment

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

Nice catch, thanks @iNem0o!

@github-project-automation github-project-automation bot moved this to Ready for review in PR Dashboard Jun 25, 2025
Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

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

I don't think it's good practice to verify basic software integrity on runtime.

The folder absolutely has to be there, is there when installing it and there is no way to delete it by the user.

fix code style
@iNem0o
Copy link
Author

iNem0o commented Jun 25, 2025

Hello @Hlavtox

I’d argue this is not about integrity checks but defensive programming around uncertain I/O (filesystem rights, mounts, etc.).

@iNem0o iNem0o requested a review from Hlavtox August 22, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

3 participants