From 3b23d50c5de181aae16ddf89b903c1491290d751 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Thu, 13 Nov 2025 22:54:44 +0100 Subject: [PATCH] phar: Fix SplFileInfo::openFile() in write mode This stopped working after e735d2bc3b 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. --- ext/phar/phar.c | 2 +- ext/phar/phar_internal.h | 1 + ext/phar/phar_object.c | 4 +-- ext/phar/tar.c | 2 +- .../tests/SplFileInfo_openFile_write.phpt | 31 +++++++++++++++++++ ext/phar/tests/gh17808.phpt | 20 ++++++++---- ext/phar/zip.c | 2 +- 7 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 ext/phar/tests/SplFileInfo_openFile_write.phpt diff --git a/ext/phar/phar.c b/ext/phar/phar.c index 4c2cab27dcea4..6e06ca5e08247 100644 --- a/ext/phar/phar.c +++ b/ext/phar/phar.c @@ -418,7 +418,7 @@ void phar_entry_remove(phar_entry_data *idata, char **error) /* {{{ */ phar = idata->phar; - if (idata->internal_file->fp_refcount < 2) { + if (idata->internal_file->fp_refcount < 2 && idata->internal_file->fileinfo_lock_count == 0) { if (idata->fp && idata->fp != idata->phar->fp && idata->fp != idata->phar->ufp && idata->fp != idata->internal_file->fp) { php_stream_close(idata->fp); } diff --git a/ext/phar/phar_internal.h b/ext/phar/phar_internal.h index 9f8a46b65ec60..4afa72db231aa 100644 --- a/ext/phar/phar_internal.h +++ b/ext/phar/phar_internal.h @@ -249,6 +249,7 @@ typedef struct _phar_entry_info { php_stream *fp; php_stream *cfp; int fp_refcount; + unsigned int fileinfo_lock_count; char *tmp; phar_archive_data *phar; char *link; /* symbolic link to another file */ diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 7c1bb6293782e..6d61adee776e4 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -4492,7 +4492,7 @@ PHP_METHOD(PharFileInfo, __construct) entry_obj->entry = entry_info; if (!entry_info->is_persistent && !entry_info->is_temp_dir) { - ++entry_info->fp_refcount; + ++entry_info->fileinfo_lock_count; /* The phar data must exist to keep the alias locked. */ ZEND_ASSERT(!phar_data->is_persistent); ++phar_data->refcount; @@ -4540,7 +4540,7 @@ PHP_METHOD(PharFileInfo, __destruct) efree(entry); entry_obj->entry = NULL; } else if (!entry->is_persistent) { - --entry->fp_refcount; + --entry->fileinfo_lock_count; /* The entry itself still lives in the manifest, * which will either be freed here if the file info was the last reference; or freed later. */ entry_obj->entry = NULL; diff --git a/ext/phar/tar.c b/ext/phar/tar.c index 3bbaad5968260..4424ad43d8954 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -721,7 +721,7 @@ static int phar_tar_writeheaders_int(phar_entry_info *entry, void *argument) /* } if (entry->is_deleted) { - if (entry->fp_refcount <= 0) { + if (entry->fp_refcount <= 0 && entry->fileinfo_lock_count == 0) { return ZEND_HASH_APPLY_REMOVE; } else { /* we can't delete this in-memory until it is closed */ diff --git a/ext/phar/tests/SplFileInfo_openFile_write.phpt b/ext/phar/tests/SplFileInfo_openFile_write.phpt new file mode 100644 index 0000000000000..f63baf5c7ad10 --- /dev/null +++ b/ext/phar/tests/SplFileInfo_openFile_write.phpt @@ -0,0 +1,31 @@ +--TEST-- +SplFileInfo::openFile() in write mode +--EXTENSIONS-- +phar +--INI-- +phar.readonly=0 +--FILE-- +addFromString('test', 'contents'); +var_dump($phar['test']->openFile('w')); + +?> +--CLEAN-- + +--EXPECTF-- +object(SplFileObject)#%d (%d) { + ["pathName":"SplFileInfo":private]=> + string(%d) "phar://%stest" + ["fileName":"SplFileInfo":private]=> + string(4) "test" + ["openMode":"SplFileObject":private]=> + string(1) "w" + ["delimiter":"SplFileObject":private]=> + string(1) "," + ["enclosure":"SplFileObject":private]=> + string(1) """ +} diff --git a/ext/phar/tests/gh17808.phpt b/ext/phar/tests/gh17808.phpt index 03e54ff264bfa..a5c13a5405e24 100644 --- a/ext/phar/tests/gh17808.phpt +++ b/ext/phar/tests/gh17808.phpt @@ -5,18 +5,26 @@ phar zlib --FILE-- getContent())); -unlink("$file"); +unlink($file); var_dump($file->getATime()); ?> +--CLEAN-- + --EXPECTF-- -string(%d) "phar://%spackage.xml" +object(PharFileInfo)#%d (%d) { + ["pathName":"SplFileInfo":private]=> + string(%d) "phar://%spackage.xml" + ["fileName":"SplFileInfo":private]=> + string(11) "package.xml" +} int(6747) - -Warning: unlink(): phar error: "package.xml" in phar %s, has open file pointers, cannot unlink in %s on line %d int(33188) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index b5133063e4460..d168c2a73ff2f 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -833,7 +833,7 @@ static int phar_zip_changed_apply_int(phar_entry_info *entry, void *arg) /* {{{ } if (entry->is_deleted) { - if (entry->fp_refcount <= 0) { + if (entry->fp_refcount <= 0 && entry->fileinfo_lock_count == 0) { return ZEND_HASH_APPLY_REMOVE; } else { /* we can't delete this in-memory until it is closed */