- Issue created by @donquixote
- Status changed to Needs review
8 months ago 4:35am 7 November 2023 - last update
8 months ago 30,486 pass, 2 fail - Status changed to Needs work
8 months ago 2:25pm 7 November 2023
The class `Drupal\Core\Archiver\Zip` is an adapter/wrapper for the PHP's native ZipArchive class, to implement Drupal's ArchiverInterface.
The native ZipArchive already has a flawed error behavior: In some cases, it produces warnings instead of throwing exceptions.
One such case is in ZipArchive::extractTo(), if it needs to overwrite a destination file that is not writable.
In addition to the warning, it returns FALSE (I think).
The Drupal Zip is even worse:
It does not have a return value that would indicate failure. It simply ignores the return from ZipArchive::extractTo(), and returns $this instead.
(this example requires custom code)
Create a directory with some files inside sites/default/files.
Manually create a zip of that directory, also inside sites/default/files.
Make one of the files non-writable.
Use Drupal Zip::extract() _or_ native php ZipArchive::extractTo() to unzip the zip file into the directory, overwriting the original files.
Expected: Exception because it cannot overwrite one of the destination files.
Actual (Zip): A warning is produced, but no exception. Also no useful return value.
Actual (ZipArchive): A warning is produced, and FALSE is returned.
Let Zip::extract() throw an exception.
This can be done with set_error_handler() + try/finally.
public function extract($path, array $files = []) {
set_error_handler(function ($severity, $message, $filename, $lineno) {
throw new \ErrorException($message, 0, $severity, $filename, $lineno);
});
try {
$this->zip->extractTo($path, $files ?: NULL);
}
finally {
restore_error_handler();
}
return $this;
}
Advertise the exception in the phpdoc of ArchiverInterface::extract().
Needs work
11.0 🔥
Last updated