Skip to content
This repository has been archived by the owner on Feb 25, 2020. It is now read-only.

PHP Fatal Error when calling make with a non-zip file path #116

Open
SlyRock opened this issue Dec 4, 2017 · 1 comment
Open

PHP Fatal Error when calling make with a non-zip file path #116

SlyRock opened this issue Dec 4, 2017 · 1 comment

Comments

@SlyRock
Copy link

SlyRock commented Dec 4, 2017

Hi,

First of all, congrats for this package which provides an elegant api for the ZipArchive !

When trying to create a Zipper instance with a path pointing to a false zip file (Wanted to make sure my app handles correctly these kind of cases) :

try {
    $this->zip = Zipper::make('path/to/false/zip');
    dump($this->zip->getStatus());
} catch (\Exception $e) {
    dump($e->getMessage());
}  

I get this error (not trapped by the "try/catch") :

PHP Fatal error:  Uncaught Error: Call to a member function close() on string in .../vendor/chumper/zipper/src/Chumper/Zipper/Zipper.php:67
Stack trace:
#0 [internal function]: Chumper\Zipper\Zipper->__destruct()
#1 {main}
  thrown in .../vendor/chumper/zipper/src/Chumper/Zipper/Zipper.php on line 67

After a bit of research it comes from this part (line 89) of the "Zipper::make" method :

$this->repository = $type;
if (is_string($objectOrName)) {
    $this->repository = new $objectOrName($pathToFile, $new);
}

The "new $objectOrName" statement throws an Exception, so the make is interrupted, but the repository fields is initialized with the text value. Later the destructor of the class tries to close the repository (line 67 of the same class) assuming there is an object in this field :

public function __destruct()
{
    if (null !== $this->repository) {
        $this->repository->close();
    }
}

I tried replacing the faulty code in the Zipper class with that one :

$this->repository = (is_string($objectOrName)) ? new $objectOrName($pathToFile, $new): $objectOrName;

Worked like a charm :) I was able to catch the exception in my app to handle my bad zip file case.

As a workaround I finally instantiated the Repository in the "make" call :

try {
    $this->zip = Zipper::make($this->path, new ZipRepository($this->path));
    dump($this->zip->getStatus());
} catch (\Exception $e) {
    dump($e->getMessage());
}

Seems like a fix-worthy issue to me !

Cheers,
Sylvain

@mavsan
Copy link

mavsan commented Dec 12, 2017

If you open a broken archive, the same error occurs. In this case, the make method will throw an exception, and then there will be another exception in __destruct.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants