phplist

NOTE:: Before reporting an issue, make sure you are running the latest version, currently 3.3.1


View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0016922phplist applicationpluginspublic16-10-13 10:4018-12-14 14:06
Reporterduncanc 
PrioritynormalSeverityminorReproducibilitysometimes
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.0.5 
Target Version3.0.XFixed in Version3.0.11 
Summary0016922: Install plugin does not handle invalid zip file
DescriptionTrying to track down why a plugin would not install from GitHub, for some reason instead of the zip file being downloaded there was a text file containing some html about being redirected. It looks as if redirects were not handled transparently in that configuration, but that is a different problem.

The failure to install showed that the code to open a zip file does not handle errors correctly.
It should check the return from $zip->open for being a resoure instead of true, as an integer error code can be returned.

See file admin/plugins.php line
change from
  if (!empty($filename) && $zip->open($GLOBALS['tmpdir'].'/phpListPlugin-'.$filename)) {

to
  if (!empty($filename) && is_resource($zip->open($GLOBALS['tmpdir'].'/phpListPlugin-'.$filename))) {

Then at the end of that if statement reinstate the else part that is currently commented out. It might also be useful to include the error code returned by $zip->open although I can't find any explanation of what the codes actually are.

  //} else {
    //Error(s('Invalid plugin package'));

The effect of this problem is that the install does fail later but for an undetermined reason.
TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0055775)
michiel (manager)
18-12-14 12:00

I've put the error back (Invalid package) but when I tried using the is_resource it failed even on a valid zip file.
(0055777)
duncanc (developer)
18-12-14 12:35

My mistake, I was looking at the documentation for the zip_open function instead of the method.
See http://php.net/manual/en/ziparchive.open.php [^]

The line should probably be:

  if (!empty($filename) && $zip->open($GLOBALS['tmpdir'].'/phpListPlugin-'.$filename) === TRUE) {
(0055779)
michiel (manager)
18-12-14 13:13

well, but that's the same as just testing on the open result.
(0055780)
duncanc (developer)
18-12-14 13:26

I think that you are missing my point.

The return from zip_open() will evaluate true even when it is one of the error codes documented here http://php.net/manual/en/ziparchive.open.php [^]

A strict comparison using === is necessary to test that it actually has returned true instead of an error code. There are some examples on that documentation page.
(0055781)
michiel (manager)
18-12-14 14:06

ok, added the evaluation: https://github.com/michield/phplist/commit/d33bed42fcd16035a2b329a5c98ef2915fd76be8 [^]


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker