Fix #70322: ZipArchive::close() doesn't indicate errors

[cmb69]

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.

@pierrejoye @remicollet Okay to merge? Or should this be applied to pecl/zip first?

[cmb69]

Wrt. changing oo_delete.phpt: the error message would only occur on Windows:

Warning: ZipArchive::close(): Can't remove file: Permission denied in …oo_delete.php on line 59

Rather interesting to see that some platform dependent programming errors are hidden by suppressing zip_close() failures. 😄

[cmb69]

The failing check seems unrelated to this PR.

[pierrejoye]

It looks good. While I wish we had use exceptions :)

For the phpt, any reason to silent the warning?

[cmb69]

It looks good. While I wish we had use exceptions :)

IMHO, that would be too much of a BC break, at least for PHP 5.6.

For the phpt, any reason to silent the warning?

For oo_delete.phpt ZipArchive::close() fails on Windows only. It seems to me that it's not worth to have platform dependent tests, and actually changing the test doesn't seem to be appropriate for this PR.

[pierrejoye]

On Sat, Aug 22, 2015 at 9:15 AM, Christoph M. Becker <
notifications@github.com> wrote:

It looks good. While I wish we had use exceptions :)

Yes, hence why I wish we had done it to begin with :)

IMHO, that would be too much of a BC break, at least for PHP 5.6.

For the phpt, any reason to silent the warning?

For oo_delete.phpt ZipArchive::close() fails on Windows only. It seems to
me that it's not worth to have platform dependent tests, and actually
changing the test doesn't seem to be appropriate for this PR.

We may check the underlying code in libzip but I wonder how it could never
fail on linux (did not have the code in mind right now :)

Pierre

@pierrejoye | http://www.libgd.org

[cmb69]

@pierrejoye

We may check the underlying code in libzip but I wonder how it could never fail on linux (did not have the code in mind right now :)

The issue is that libzip 0.11.2 calls remove() when an empty archive is closed, even though the file handle may still be open (it gets closed only in zip_discard()). Windows doesn't allow that operation, but Linux does. As of libzip 1.0.1 (probably already as of 1.0.0) this issue has been fixed, and so oo_delete.php passes also without the silence operator.

Likewise, with libzip 1.0.1 the new test (bug70233.phpt) fails (Windows and Linux). I'll replace this test with a more portable and reliable one.

[php-pulls]

Comment on behalf of cmb at php.net:

Applied.