Skip to content

Commit 0c99bd7

Browse files
iliaalGirgias
authored andcommitted
phar: call phar_entry_delref before goto finish in phar_add_file error paths
phar_add_file opens or creates an entry via phar_get_or_create_entry_data_rw, which increments the entry's reference count and must be balanced by a phar_entry_delref call. Two error paths inside the content-write block jumped to finish: with goto, skipping the phar_entry_delref at line 3714. The finish: label comes after the delref, so both paths leaked the entry reference. Add phar_entry_delref(data) before each goto finish in the short-write and missing-resource branches. Closes GH-21798 Closes GH-21803
1 parent b9aaa05 commit 0c99bd7

2 files changed

Lines changed: 31 additions & 0 deletions

File tree

ext/phar/phar_object.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3684,11 +3684,13 @@ static void phar_add_file(phar_archive_data **pphar, zend_string *file_name, con
36843684
size_t written_len = php_stream_write(data->fp, ZSTR_VAL(content), ZSTR_LEN(content));
36853685
if (written_len != contents_len) {
36863686
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
3687+
phar_entry_delref(data);
36873688
goto finish;
36883689
}
36893690
} else {
36903691
if (!(php_stream_from_zval_no_verify(contents_file, zresource))) {
36913692
zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Entry %s could not be written to", filename);
3693+
phar_entry_delref(data);
36923694
goto finish;
36933695
}
36943696
php_stream_copy_to_stream_ex(contents_file, data->fp, PHP_STREAM_COPY_ALL, &contents_len);
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GH-21798: phar_add_file must call phar_entry_delref on write error paths
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.readonly=0
7+
phar.require_hash=0
8+
--FILE--
9+
<?php
10+
// Regression baseline: verify addFile and addFromString succeed and
11+
// phar_entry_delref is not skipped on the happy path.
12+
$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.phar';
13+
14+
$phar = new Phar($fname);
15+
$phar->addFromString('hello.txt', 'hello world');
16+
$phar->addFromString('empty.txt', '');
17+
unset($phar);
18+
19+
$phar = new Phar($fname);
20+
echo $phar['hello.txt']->getContent() . "\n";
21+
echo ($phar->offsetExists('empty.txt') ? 'empty exists' : 'missing') . "\n";
22+
echo "no crash\n";
23+
?>
24+
--CLEAN--
25+
<?php @unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.phar'); ?>
26+
--EXPECT--
27+
hello world
28+
empty exists
29+
no crash

0 commit comments

Comments
 (0)