Edit report at https://bugs.php.net/bug.php?id=62106&edit=1

 ID:                 62106
 Updated by:         a...@php.net
 Reported by:        t dot glaser at tarent dot de
 Summary:            zip can (and will, in real-life cases) leak huge
                     tempfiles
-Status:             Open
+Status:             Feedback
 Type:               Bug
 Package:            Zip Related
 Operating System:   Unix/POSIX, maybe Windows
 PHP Version:        master-Git-2012-05-22 (Git)
-Assigned To:        
+Assigned To:        ab
 Block user comment: N
 Private report:     N



Previous Comments:
------------------------------------------------------------------------
[2012-07-10 14:20:04] a...@php.net

Please test the supplied patch. Ideally you could do this with your productive 
dev environment. Because cleanup happens on request shutdown, looks like it's 
nearly impossible to test this with phpt. Consider the following piece of code:

<?php

set_time_limit(0);
ini_set('memory_limit', '768M');

$zip_fname = dirname(__FILE__) . DIRECTORY_SEPARATOR . 'bug62106.zip';

$s = 'a';
foreach (array(1024, 1024, 200) as $b) {
        $s = str_repeat($s, $b);
}

$zip = new ZipArchive();
$r = $zip->open($zip_fname, ZipArchive::CREATE);

if ($r) {
        $zip->addFromString('huhu.txt', $s);

        set_time_limit(3);
        $zip->close();
}

register_shutdown_function will not work here because it's a part of request. 
But it's clearly to see - without patch there are temp files there, and with 
the 
patch the cleanup works. But the best were of course if you test it directly 
with your app. If you have an idea how to test this with phpt, it would be also 
great :)

------------------------------------------------------------------------
[2012-07-10 14:14:00] a...@php.net

The following patch has been added/updated:

Patch Name: 62106.patch
Revision:   1341929640
URL:        
https://bugs.php.net/patch-display.php?bug=62106&patch=62106.patch&revision=1341929640

------------------------------------------------------------------------
[2012-05-22 07:47:45] t dot glaser at tarent dot de

Description:
------------
I’ve been investigating a disc full case on some of our servers, which was 
fixed by removing eight Gibibytes (!) of *.zip.?????? files (about a dozen 
files) that looked like mkstemp(3)-generated, and investigated this problem.

To cut a long story short: an instance of the ZipArchive class does 
its “action” when the close() method is called. (There should also be an 
abort() method which frees all ressources and unlocks all files, for if an 
error is detected in between open/addfile and further addfile calls or the 
close call, but that’s not the problem at hand, it just irritated me.) This 
ends up being [php-src.git] / ext / zip / lib / zip_close.c (I’ve just looked 
at gitweb master to reconfirm this), which has the following problem:

zip_close() forcibly allocates a temporary file, operates on that and then 
atomically renames the temporary file to the desired output filename. This may 
be a good thing in a regular Unix environment (and looking at the authors, I 
know at least one of them from NetBSD®, so the code probably does come from a 
Unix environment). HOWEVER, this is a PHP environment, not a regular Unix 
environment, even if it may run on Unix, and PHP, when running in a webserver, 
but also as CLI, asserts certain resource limits – execution time (not in 
CLI), memory size, etc.

When the script is terminated due to such a limit (in our case, probably the 
execution time limit in apache2 mod_php), the tempfile is not cleaned up. 
Period. And when you were zipping up 400 MiB, and the termination happens when 
300 MiB of that are already written to disc… well, not nice.

The cleanest way would probably be for the PHP core to offer an extension (C, 
but PHP would also make sense) to register cleanup hooks that run when script 
execution is terminated. This would be more generically usable than just a 
method with which an extension could register tempfiles which were then 
removed, as cleanup may involve other tasks. (As a general suggestion, cleanup 
execution time could be limited to the greater of 5 seconds and a tenth of the 
script execution time limit, and memory size should be similarly extended by a 
small amount so it won’t terminate abnormally during the cleanup phase.)

Test script:
---------------
createNewestReleaseFilesAsZip method (at the end of the file) in:

https://evolvis.org/scm/viewvc.php/trunk/gforge_base/evolvisforge-5.1/src/common/frs/FRSPackage.class.php?revision=18116&root=evolvis&view=markup

Note how a later revision of the same file already contains some attempts at 
error handling, and how it the lack of an abort method makes that awkward.

Expected result:
----------------
No tempfile leak. If no resource limit is hit, the .zip file is created; if 
one is hit, the .zip file is either untouched or removed (unlinked).

Actual result:
--------------
If a resource limit is hit, a tempfile of arbitrary size (at least several 
hundred Mebibytes confirmed) is leaked.


------------------------------------------------------------------------



-- 
Edit this bug report at https://bugs.php.net/bug.php?id=62106&edit=1

Reply via email to