Edit report at https://bugs.php.net/bug.php?id=66786&edit=1
ID: 66786
User updated by: lists dot ban at herbesfolles dot org
Reported by: lists dot ban at herbesfolles dot org
Summary: ZipArchive fails when an added file is removed
before close()
Status: Not a bug
Type: Bug
Package: Zip Related
Operating System: Linux (Debian Sid), probably any
PHP Version: master-Git-2014-02-26 (Git)
Block user comment: N
Private report: N
New Comment:
> It is not really a bug but how libzip works.
OK, fair enough.
> I did not hear about a change in this area but when this change happens, the doc will be
> updated accordingly.
As I said earlier, I believe the documentation should be updated to be more clear in any case.
Currently, even native English speakers agree that the second sentence ("In short, it means you
can first delete an added file after the archive is closed") is at best confusing, and I for
myself (non-native speaker) could simply not understand anything in this sentence.
I propose to e.g. simply replace "first" with "only":
"In short, it means you can only delete an added file after the archive is closed."
This way, I would have understood, and AFAICT it's only clearer for everyone.
Though, I read the previous sentence ("When a file is set to be added to the archive, PHP will
attempt to lock the file and it is only released once the ZIP operation is done.") and its
"PHP will attempt to lock the file" as that at worse unlink() would fail if I call it
before the zip operation is done.
So maybe the part about "PHP will attempt to lock the file" should be removed since in
practice it's not the case (or it doesn't work under Linux).
---
Also, again, the test case in the patch (even without the patch) show a regression in Git where if
the file was unlinked before save() and after addFile() *no error* would be reported by the API.
This is a bug in any case, and 5.5.9 used to report a problem on close().
Should I create new reports for each of these issues? One against the documentation and the other
again against the Zip extension for that regression?
Previous Comments:
------------------------------------------------------------------------
[2014-03-01 07:18:15] pajoye@php.net
Files handles are kept until the archive is closed. That's why they cannot be deleted or
removed.
addFromString uses memory buffers, so you do not have this issue.
It is not really a bug but how libzip works. I did not hear about a change in this area but when
this change happens, the doc will be updated accordingly.
move to "not a bug".
------------------------------------------------------------------------
[2014-03-01 00:28:13] lists dot ban at herbesfolles dot org
> Maybe I misread it, but according to the quote you gave it shouldn't
> work:
>
> "you can first delete an added file after the archive is closed"
After your comment I did some research and asked a few English speakers, and even though no
dictionary helped me understand this sentence (particularly the use of "first" is never
suggest to also mean something like "only"), the asked native speakers understood it like
you seem to do -- while admitting the sentence was confusing.
However, the first sentence taken alone, "When a file is set to be added to the archive, *PHP
will attempt to lock the file* and it is only released once the ZIP operation is done" is
consistently understood as that it should either just work or unlink() should fail.
So, I suggest the note be rewritten to avoid any confusion -- and if actually PHP don't attempt
any locking (or if it doesn't work in practice) maybe the note should not suggest it does.
> Looking at those code, the diff between the current situation and
> your patch were only that you pass an open descriptor. When the
> file is deleted, the descriptor will be still open, true. That
> however will not work on Windows, so different handling.
Right, Windows will not accept unlinking an open file. Well, alternatively this could work if PHP
delayed the unlinking because the file was open by the script, but I guess this is way out of scope.
> Were it not safer just using addFromString?
I was worried that adding several files with addFromString() would consume unreasonable amount of
memory -- but I must admit I didn't perform actual tests to compare.
---
So, I guess this isn't really a bug in the Zip extension, and only a possibly confusing note in
its documentation.
Though, there still seem to be a problem in the Git version, since a test case reported absolutely
no error through the API -- see the test in the patch, it will of course fail, but neither addFile()
nor close() will report any problem.
------------------------------------------------------------------------
[2014-02-28 10:09:39] ab@php.net
Maybe I misread it, but according to the quote you gave it shouldn't work:
"you can first delete an added file after the archive is closed"
In 5.5 there's libzip-0.10.1 and in 5.6+ is libzip 0.11.2 . Looking at those code, the diff
between the current situation and your patch were only that you pass an open descriptor. When the
file is deleted, the descriptor will be still open, true. That however will not work on Windows, so
different handling. Were it not safer just using addFromString?
------------------------------------------------------------------------
[2014-02-26 23:16:49] lists dot ban at herbesfolles dot org
Description:
------------
When adding a file to a Zip archive, the user expects to be able to unlink the original file without
issues. The documentation on ZipArchive::addFile() even has a (admittedly strangely worded) note
suggesting it would work:
"When a file is set to be added to the archive, PHP will attempt to lock the file and it is
only released once the ZIP operation is done. In short, it means you can first delete an added file
after the archive is closed."
However, in practice it doesn't work, and depending on the PHP version (or build?) it could
even silently fail. PHP 5.5.9 from Debian Sid fails on ZipArchive::close() with StatusString as
"No error"; PHP 5.7.0-dev from today Git don't even report any problem. In any
case, the result zip is not created.
This is both with system (0.11.2) and bundled libzip.
Test script:
---------------
$zip = new ZipArchive();
$zip->open('output.zip', ZipArchive::CREATE | ZipArchive::OVERWRITE);
file_put_contents('dummy.txt', 'hello world');
if (! $zip->addFile('dummy.txt'))
echo "addFile() failed\n";
unlink('dummy.txt');
if (! $zip->close())
echo "close() failed\n";
echo "output.zip size: ", filesize('output.zip'), "\n";
@unlink('output.zip');
Expected result:
----------------
output.zip size: 129
Actual result:
--------------
output.zip size:
Warning: filesize(): stat failed for output.zip in zipbug.php on line 9
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=66786&edit=1