com php-src: Fix #70322: ZipArchive::close() doesn't indicate errors: ext/zip/php_zip.c ext/zip/tests/bug70322.phpt ext/zip/tests/oo_delete.phpt

Commit: c77f783777e0169b2a4b7b9439cad76b372290a3 Author: Christoph M. Becker <cmbecker69@gmx.de> Fri, 21 Aug 2015 22:26:26 +0200 Committer: Christoph M. Becker <cmb@php.net> Sat, 5 Sep 2015 00:34:10 +0200 Parents: 4b1dff6f438f84f7694df701b68744edbdd86153 Branches: PHP-5..6 master Link: http://git.php.net/?p=php-src.git;a=commitdiff;h=c77f783777e0169b2a4b7b9439cad76b372290a3 Log: Fix #70322: ZipArchive::close() doesn't indicate errors If an archive can't be written, ZipArchive::close() nonetheless returns TRUE. We fix the return value to properly return success, and additionally raise a warning on failure. Bugs: https://bugs.php.net/70322 Changed paths: M ext/zip/php_zip.c A ext/zip/tests/bug70322.phpt M ext/zip/tests/oo_delete.phpt Diff: diff --git a/ext/zip/php_zip.c b/ext/zip/php_zip.c index d4f77eb..0355db4 100644 --- a/ext/zip/php_zip.c +++ b/ext/zip/php_zip.c @@ -1616,6 +1616,7 @@ static ZIPARCHIVE_METHOD(close) struct zip *intern; zval *this = getThis(); ze_zip_object *ze_obj; + int err; if (!this) { RETURN_FALSE; @@ -1625,7 +1626,8 @@ static ZIPARCHIVE_METHOD(close) ze_obj = (ze_zip_object*) zend_object_store_get_object(this TSRMLS_CC); - if (zip_close(intern)) { + if (err = zip_close(intern)) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, zip_strerror(intern)); zip_discard(intern); } @@ -1634,7 +1636,11 @@ static ZIPARCHIVE_METHOD(close) ze_obj->filename_len = 0; ze_obj->za = NULL; - RETURN_TRUE; + if (!err) { + RETURN_TRUE; + } else { + RETURN_FALSE; + } } /* }}} */ diff --git a/ext/zip/tests/bug70322.phpt b/ext/zip/tests/bug70322.phpt new file mode 100644 index 0000000..a9b4915 --- /dev/null +++ b/ext/zip/tests/bug70322.phpt @@ -0,0 +1,29 @@ +--TEST-- +Bug #70322 (ZipArchive::close() doesn't indicate errors) +--DESCRIPTION-- +We want to test whether ZipArchive::close() returns FALSE and raises a warning +on failure, so we force the failure by adding a file to the archive, which we +delete before closing. +--SKIPIF-- +<?php +if (!extension_loaded('zip')) die('skip requires zip extension'); +?> +--FILE-- +<?php +$zipfile = __DIR__ . '/bug70322.zip'; +$textfile = __DIR__ . '/bug70322.txt'; +touch($textfile); +$zip = new ZipArchive(); +$zip->open($zipfile, ZipArchive::CREATE); +$zip->addFile($textfile); +unlink($textfile); +var_dump($zip->close()); +?> +--CLEAN-- +<?php +// we don't expect the archive to be created, but clean up just in case... +@unlink(__DIR__ . '/bug70322.zip'); +?> +--EXPECTF-- +Warning: ZipArchive::close(): Read error: No such file or directory in %s%ebug70322.php on line %d +bool(false) diff --git a/ext/zip/tests/oo_delete.phpt b/ext/zip/tests/oo_delete.phpt index 9eac821...69c6151 100644 --- a/ext/zip/tests/oo_delete.phpt +++ b/ext/zip/tests/oo_delete.phpt @@ -63,7 +63,8 @@ $sb = $zip->statIndex(1); var_dump($sb); $sb = $zip->statIndex(2); var_dump($sb); -$zip->close(); +// suppress irrelevant error message: +@$zip->close(); unset($zip); if (file_exists($file)) {