Skip to content

Commit 995ecff

Browse files
committed
Fix #70417: PharData::compress() doesn't close temp file
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.
1 parent 2022dac commit 995ecff

File tree

2 files changed

+40
-2
lines changed

2 files changed

+40
-2
lines changed

ext/phar/phar.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,11 +284,13 @@ int phar_archive_delref(phar_archive_data *phar) /* {{{ */
284284
PHAR_G(last_phar) = NULL;
285285
PHAR_G(last_phar_name) = PHAR_G(last_alias) = NULL;
286286

287-
if (phar->fp && !(phar->flags & PHAR_FILE_COMPRESSION_MASK)) {
287+
if (phar->fp && (!(phar->flags & PHAR_FILE_COMPRESSION_MASK) || !phar->alias)) {
288288
/* close open file handle - allows removal or rename of
289289
the file on windows, which has greedy locking
290290
only close if the archive was not already compressed. If it
291-
was compressed, then the fp does not refer to the original file */
291+
was compressed, then the fp does not refer to the original file.
292+
We're also closing compressed files to save resources,
293+
but only if the archive isn't aliased. */
292294
php_stream_close(phar->fp);
293295
phar->fp = NULL;
294296
}

ext/phar/tests/tar/bug70417.phpt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
Bug #70417 (PharData::compress() doesn't close temp file)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('phar') || !extension_loaded('zlib')) {
6+
die("skip ext/phar or ext/zlib not available");
7+
}
8+
exec('lsof -p ' . getmypid(), $out, $status);
9+
if ($status !== 0) {
10+
die("skip lsof(8) not available");
11+
}
12+
?>
13+
--FILE--
14+
<?php
15+
function countOpenFiles() {
16+
exec('lsof -p ' . getmypid(), $out);
17+
return count($out);
18+
}
19+
$filename = __DIR__ . '/bug70417.tar';
20+
@unlink("$filename.gz");
21+
$openFiles1 = countOpenFiles();
22+
$arch = new PharData($filename);
23+
$arch->addFromString('foo', 'bar');
24+
$arch->compress(Phar::GZ);
25+
unset($arch);
26+
$openFiles2 = countOpenFiles();
27+
var_dump($openFiles1 === $openFiles2);
28+
?>
29+
--CLEAN--
30+
<?php
31+
$filename = __DIR__ . '/bug70417.tar';
32+
@unlink($filename);
33+
@unlink("$filename.gz");
34+
?>
35+
--EXPECT--
36+
bool(true)

0 commit comments

Comments
 (0)