Feeling Lucky
?
[#27323] - Correction of the fix #19781
- Closed
- 20 Dec 2019
- Medium
- Build: staging
- # 27323
In my opinion, the correct interpretation of the code should look like this (#19781):
foreach ($new as $path)
{
$path = trim($path);
if (!in_array($path, $paths) || is_dir($path)) # Use || (pipes) instead of &&
{
array_unshift($paths, $path);
}
}
In the original code it is first checked if the path is in the array, if not it is checked if the path is really a path (a continue if true is missing).
In the fix after the check in the array, i.e. the path is in the array, there is no check if it is really a path.
This does not correspond to the meaning of the original code.
joomla-cms-bot
- | Labels |
Added:
?
|
||
degobbis
- comment
- 20 Dec 2019
I don't know what's in $new or $paths, but if you want to prevent a file without an extension from being displayed as a path, you need to run the is_dir() check and not leave the path in the array, right?
At least that's what the code has done before.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-12-20 13:31:19 |
| Closed_By | ⇒ | degobbis |
It is fine to skip checking the path actually exists at this point. This is just building the potential list of lookup paths for later use, the use later will necessitate proper path exists checks for the resource in question.
Filesystem I/O remains a performance bottleneck even in current PHP versions. Adding a check both when paths are pushed into the registry and when the registry is used will introduce a performance slowdown.
All in all, there is no harm in storing invalid paths here. At worst you have an couple extra 100 character strings inside an array, hardly worth the extra I/O costs to keep that clean.