]> granicus.if.org Git - php/commitdiff
Fix #70417: PharData::compress() doesn't close temp file
authorChristoph M. Becker <cmbecker69@gmx.de>
Thu, 3 Sep 2015 14:37:29 +0000 (16:37 +0200)
committerChristoph M. Becker <cmbecker69@gmx.de>
Thu, 5 Jan 2017 13:12:31 +0000 (14:12 +0100)
According to the comment, it has not been deemed necessary to close compressed
files. However, we don't want to keep unclosed file handles to save ressources.
So we're also closing compressed archives, if they're not aliased.

ext/phar/phar.c
ext/phar/tests/tar/bug70417.phpt [new file with mode: 0644]

index bf89220e0ebdb68ebb5379ffc0a32997f94e77e7..9d91fd26ae99260111b934cc25174387d4bd7059 100644 (file)
@@ -284,11 +284,13 @@ int phar_archive_delref(phar_archive_data *phar) /* {{{ */
                PHAR_G(last_phar) = NULL;
                PHAR_G(last_phar_name) = PHAR_G(last_alias) = NULL;
 
-               if (phar->fp && !(phar->flags & PHAR_FILE_COMPRESSION_MASK)) {
+               if (phar->fp && (!(phar->flags & PHAR_FILE_COMPRESSION_MASK) || !phar->alias)) {
                        /* close open file handle - allows removal or rename of
                        the file on windows, which has greedy locking
                        only close if the archive was not already compressed.  If it
-                       was compressed, then the fp does not refer to the original file */
+                       was compressed, then the fp does not refer to the original file.
+                       We're also closing compressed files to save resources,
+                       but only if the archive isn't aliased. */
                        php_stream_close(phar->fp);
                        phar->fp = NULL;
                }
diff --git a/ext/phar/tests/tar/bug70417.phpt b/ext/phar/tests/tar/bug70417.phpt
new file mode 100644 (file)
index 0000000..0096b5a
--- /dev/null
@@ -0,0 +1,36 @@
+--TEST--
+Bug #70417 (PharData::compress() doesn't close temp file)
+--SKIPIF--
+<?php
+if (!extension_loaded('phar') || !extension_loaded('zlib')) {
+    die("skip ext/phar or ext/zlib not available");
+}
+exec('lsof -p ' . getmypid(), $out, $status);
+if ($status !== 0) {
+    die("skip lsof(8) not available");
+}
+?>
+--FILE--
+<?php
+function countOpenFiles() {
+    exec('lsof -p ' . getmypid(), $out);
+    return count($out);
+}
+$filename = __DIR__ . '/bug70417.tar';
+@unlink("$filename.gz");
+$openFiles1 = countOpenFiles();
+$arch = new PharData($filename);
+$arch->addFromString('foo', 'bar');
+$arch->compress(Phar::GZ);
+unset($arch);
+$openFiles2 = countOpenFiles();
+var_dump($openFiles1 === $openFiles2);
+?>
+--CLEAN--
+<?php
+$filename = __DIR__ . '/bug70417.tar';
+@unlink($filename);
+@unlink("$filename.gz");
+?>
+--EXPECT--
+bool(true)