Bug #66786 [Fbk->Opn]: ZipArchive fails when an added file is removed before close()

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: Feedback +Status: Open 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: > 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. Previous Comments: ------------------------------------------------------------------------ [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