[#6026] - Fix Potential PHP Notices/Warnings in JInstallerHelper class
- Fixed in Code Base
- 8 May 2016
- Medium
- Build: staging
- # 6026
- Diff
- bassmanpaul:patch-2
- Success continuous-integration/travis-ci The Travis CI build passed Details
User tests: Successful: Unsuccessful:
I experienced a scenario where I was testing an install package (which was corrupt) and it brought to light a couple of PHP notices in the JInstallerHelper class.
The File:
libraries/cms/installer/helper.php line 197 & 221
Issue 1 (line 197):
$dirList = array_merge(JFolder::files($extractdir, ''), JFolder::folders($extractdir, ''));As JFolder::files() & JFolder::folders() can return false which would break array_merge(). One solution would be type casting:
$dirList = array_merge((array)JFolder::files($extractdir, ''), (array)JFolder::folders($extractdir, ''));Issue 2 (line 221):
if (!count($files))Again JFolder::files() could return false so the count() of false would be 1 as opposed to 0. (see: http://php.net/manual/en/function.count.php#example-5167) Therefore as this is a false positive may I suggest something like:
if (!$files || !count($files))or
if (!is_array($files) || !count($files))If everyone likes the suggestions I will happily create a Pull Request.
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Category | ⇒ | Libraries |
| Title |
|
||||||
| Rel_Number | ⇒ | 6000 | |||||
| Relation Type | ⇒ | Pull Request for | |||||
| Title |
|
||||||
Merged on review. Thankyou very much for the fix :)
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-08 15:58:09 |
| Closed_By | ⇒ | wilsonge |
| Milestone |
Added: |
||
@bassmanpaul i have just add your description from your report and add comments about codestyle to fix travis. I hope you can have a look into it.
Regarding Travis see: https://travis-ci.org/joomla/joomla-cms/jobs/50025400