Skip to content

Commit eca7718

Browse files
committed
phar: Fix SplFileInfo::openFile() in write mode
This stopped working after e735d2b because fp_refcount is increased, making phar think that the file has open read pointers. To fix this, the refcount shouldn't be increased but that would re-introduce the previous bug. Instead, we need to add a field that "locks" the existence of the internal entry separate from the refcount.
1 parent 22aaa20 commit eca7718

File tree

7 files changed

+51
-11
lines changed

7 files changed

+51
-11
lines changed

ext/phar/phar.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ void phar_entry_remove(phar_entry_data *idata, char **error) /* {{{ */
418418

419419
phar = idata->phar;
420420

421-
if (idata->internal_file->fp_refcount < 2) {
421+
if (idata->internal_file->fp_refcount < 2 && idata->internal_file->fileinfo_lock_count == 0) {
422422
if (idata->fp && idata->fp != idata->phar->fp && idata->fp != idata->phar->ufp && idata->fp != idata->internal_file->fp) {
423423
php_stream_close(idata->fp);
424424
}

ext/phar/phar_internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ typedef struct _phar_entry_info {
249249
php_stream *fp;
250250
php_stream *cfp;
251251
int fp_refcount;
252+
unsigned int fileinfo_lock_count;
252253
char *tmp;
253254
phar_archive_data *phar;
254255
char *link; /* symbolic link to another file */

ext/phar/phar_object.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4492,7 +4492,7 @@ PHP_METHOD(PharFileInfo, __construct)
44924492

44934493
entry_obj->entry = entry_info;
44944494
if (!entry_info->is_persistent && !entry_info->is_temp_dir) {
4495-
++entry_info->fp_refcount;
4495+
++entry_info->fileinfo_lock_count;
44964496
/* The phar data must exist to keep the alias locked. */
44974497
ZEND_ASSERT(!phar_data->is_persistent);
44984498
++phar_data->refcount;
@@ -4540,7 +4540,7 @@ PHP_METHOD(PharFileInfo, __destruct)
45404540
efree(entry);
45414541
entry_obj->entry = NULL;
45424542
} else if (!entry->is_persistent) {
4543-
--entry->fp_refcount;
4543+
--entry->fileinfo_lock_count;
45444544
/* The entry itself still lives in the manifest,
45454545
* which will either be freed here if the file info was the last reference; or freed later. */
45464546
entry_obj->entry = NULL;

ext/phar/tar.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,7 @@ static int phar_tar_writeheaders_int(phar_entry_info *entry, void *argument) /*
721721
}
722722

723723
if (entry->is_deleted) {
724-
if (entry->fp_refcount <= 0) {
724+
if (entry->fp_refcount <= 0 && entry->fileinfo_lock_count == 0) {
725725
return ZEND_HASH_APPLY_REMOVE;
726726
} else {
727727
/* we can't delete this in-memory until it is closed */
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
SplFileInfo::openFile() in write mode
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.readonly=0
7+
--FILE--
8+
<?php
9+
10+
$phar = new Phar(__DIR__.'/SplFileInfo_openFile_write.phar');
11+
$phar->addFromString('test', 'contents');
12+
var_dump($phar['test']->openFile('w'));
13+
14+
?>
15+
--CLEAN--
16+
<?php
17+
@unlink(__DIR__.'/SplFileInfo_openFile_write.phar');
18+
?>
19+
--EXPECTF--
20+
object(SplFileObject)#%d (%d) {
21+
["pathName":"SplFileInfo":private]=>
22+
string(%d) "phar://%stest"
23+
["fileName":"SplFileInfo":private]=>
24+
string(4) "test"
25+
["openMode":"SplFileObject":private]=>
26+
string(1) "w"
27+
["delimiter":"SplFileObject":private]=>
28+
string(1) ","
29+
["enclosure":"SplFileObject":private]=>
30+
string(1) """
31+
}

ext/phar/tests/gh17808.phpt

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,26 @@ phar
55
zlib
66
--FILE--
77
<?php
8-
$fname = __DIR__.'/tar/files/Structures_Graph-1.0.3.tgz';
8+
$fname = __DIR__.'/tar/files/gh17808.tgz';
9+
copy(__DIR__.'/tar/files/Structures_Graph-1.0.3.tgz', $fname);
910
$tar = new PharData($fname);
1011
foreach (new RecursiveIteratorIterator($tar) as $file) {
1112
}
12-
var_dump("$file");
13+
var_dump($file);
1314
var_dump(strlen($file->getContent()));
14-
unlink("$file");
15+
unlink($file);
1516
var_dump($file->getATime());
1617
?>
18+
--CLEAN--
19+
<?php
20+
@unlink(__DIR__.'/tar/files/gh17808.tgz');
21+
?>
1722
--EXPECTF--
18-
string(%d) "phar://%spackage.xml"
23+
object(PharFileInfo)#6 (2) {
24+
["pathName":"SplFileInfo":private]=>
25+
string(89) "phar://%spackage.xml"
26+
["fileName":"SplFileInfo":private]=>
27+
string(11) "package.xml"
28+
}
1929
int(6747)
20-
21-
Warning: unlink(): phar error: "package.xml" in phar %s, has open file pointers, cannot unlink in %s on line %d
2230
int(33188)

ext/phar/zip.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ static int phar_zip_changed_apply_int(phar_entry_info *entry, void *arg) /* {{{
833833
}
834834

835835
if (entry->is_deleted) {
836-
if (entry->fp_refcount <= 0) {
836+
if (entry->fp_refcount <= 0 && entry->fileinfo_lock_count == 0) {
837837
return ZEND_HASH_APPLY_REMOVE;
838838
} else {
839839
/* we can't delete this in-memory until it is closed */

0 commit comments

Comments
 (0)