From 2f9d86b67787592c2532939df4f91957cd3df7c4 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Thu, 13 Nov 2025 23:06:21 +0100 Subject: [PATCH 1/2] phar: Fix broken return value of fflush() for phar file entries The flush functions always return EOF, even in the success path. The success path should return 0 to indicate success. Closes GH-20474. --- NEWS | 1 + ext/phar/phar.c | 2 +- ext/phar/tar.c | 2 +- .../fflush_phar_file_report_success.phpt | 25 +++++++++++++++++++ ext/phar/zip.c | 2 +- 5 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 ext/phar/tests/fflush_phar_file_report_success.phpt diff --git a/NEWS b/NEWS index 52cfb5ee638..a7b5e126e3a 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,7 @@ PHP NEWS - Phar: . Fixed bug GH-20442 (Phar does not respect case-insensitiveness of __halt_compiler() when reading stub). (ndossche, TimWolla) + . Fix broken return value of fflush() for phar file entries. (ndossche) - PHPDBG: . Fixed ZPP type violation in phpdbg_get_executable() and phpdbg_end_oplog(). diff --git a/ext/phar/phar.c b/ext/phar/phar.c index aa9a8821d8e..a63a8dd8eb6 100644 --- a/ext/phar/phar.c +++ b/ext/phar/phar.c @@ -3225,7 +3225,7 @@ int phar_flush(phar_archive_data *phar, char *user_stub, zend_long len, int conv return EOF; } - return EOF; + return 0; cleanup: if (shared_cfp != NULL) { diff --git a/ext/phar/tar.c b/ext/phar/tar.c index e675b869432..3bbaad59682 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -1378,6 +1378,6 @@ nostub: php_stream_close(newfile); } } - return EOF; + return 0; } /* }}} */ diff --git a/ext/phar/tests/fflush_phar_file_report_success.phpt b/ext/phar/tests/fflush_phar_file_report_success.phpt new file mode 100644 index 00000000000..615f4e58e56 --- /dev/null +++ b/ext/phar/tests/fflush_phar_file_report_success.phpt @@ -0,0 +1,25 @@ +--TEST-- +fflush() on phar file should report success +--EXTENSIONS-- +phar +--INI-- +phar.readonly=0 +--FILE-- +addFromString('test', 'contents'); +unset($phar); + +$f = fopen('phar://' . __DIR__.'/fflush_phar_file_report_success.phar/test', 'w'); +var_dump(fflush($f)); +var_dump(fclose($f)); + +?> +--CLEAN-- + +--EXPECT-- +bool(true) +bool(true) diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 63c56108ed9..b5133063e44 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -1542,6 +1542,6 @@ nocentralerror: if (closeoldfile) { php_stream_close(oldfile); } - return EOF; + return 0; } /* }}} */ From fd5c14e682a12a681ebeae66208cd3e09c8952d1 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+ndossche@users.noreply.github.com> Date: Sat, 15 Nov 2025 13:57:47 +0100 Subject: [PATCH 2/2] Revert "ext/phar: Voidify flush function as it always returns EOL" This reverts commit 2513258a2b4e83a5c864cc1152914414d6565375. --- ext/phar/phar.c | 41 ++++++++++++++++++------------------ ext/phar/phar_internal.h | 8 +++---- ext/phar/stream.c | 5 +++-- ext/phar/tar.c | 45 ++++++++++++++++++++-------------------- ext/phar/zip.c | 29 +++++++++++++------------- 5 files changed, 66 insertions(+), 62 deletions(-) diff --git a/ext/phar/phar.c b/ext/phar/phar.c index d6c402eefae..c01ffc9b6e0 100644 --- a/ext/phar/phar.c +++ b/ext/phar/phar.c @@ -2496,8 +2496,8 @@ zend_string *phar_create_default_stub(const char *index_php, const char *web_ind } /* }}} */ -void phar_flush(phar_archive_data *phar, char **error) { - phar_flush_ex(phar, NULL, false, error); +int phar_flush(phar_archive_data *phar, char **error) { + return phar_flush_ex(phar, NULL, false, error); } /** @@ -2505,7 +2505,7 @@ void phar_flush(phar_archive_data *phar, char **error) { * * if user_stub is NULL the default or existing stub should be used */ -void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error) /* {{{ */ +int phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error) /* {{{ */ { static const char halt_stub[] = "__HALT_COMPILER();"; @@ -2533,7 +2533,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (error) { spprintf(error, 0, "internal error: attempt to flush cached zip-based phar \"%s\"", phar->fname); } - return; + return EOF; } if (error) { @@ -2541,23 +2541,21 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa } if (!zend_hash_num_elements(&phar->manifest) && !user_stub) { - return; + return EOF; } zend_hash_clean(&phar->virtual_dirs); if (phar->is_zip) { - phar_zip_flush(phar, user_stub, is_default_stub, error); - return; + return phar_zip_flush(phar, user_stub, is_default_stub, error); } if (phar->is_tar) { - phar_tar_flush(phar, user_stub, is_default_stub, error); - return; + return phar_tar_flush(phar, user_stub, is_default_stub, error); } if (PHAR_G(readonly)) { - return; + return EOF; } if (phar->fp && !phar->is_brandnew) { @@ -2576,7 +2574,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (must_close_old_file) { php_stream_close(oldfile); } - return; + return EOF; } if (user_stub) { @@ -2590,7 +2588,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (error) { spprintf(error, 0, "illegal stub for phar \"%s\" (__HALT_COMPILER(); is missing)", phar->fname); } - return; + return EOF; } size_t len = pos - ZSTR_VAL(user_stub) + strlen(halt_stub); @@ -2608,7 +2606,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (error) { spprintf(error, 0, "unable to create stub from string in new phar \"%s\"", phar->fname); } - return; + return EOF; } phar->halt_offset = len + end_sequence_len; } else { @@ -2638,7 +2636,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (new_stub) { zend_string_free(new_stub); } - return; + return EOF; } if (new_stub) { zend_string_free(new_stub); @@ -2734,7 +2732,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (error) { spprintf(error, 0, "unable to seek to start of file \"%s\" while creating new phar \"%s\"", entry->filename, phar->fname); } - return; + return EOF; } newcrc32 = php_crc32_bulk_init(); php_crc32_stream_bulk_update(&newcrc32, file, entry->uncompressed_filesize); @@ -2760,7 +2758,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa spprintf(error, 0, "unable to bzip2 compress file \"%s\" to new phar \"%s\"", entry->filename, phar->fname); } } - return; + return EOF; } /* create new file that holds the compressed versions */ @@ -3086,7 +3084,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa php_stream_close(oldfile); } php_stream_close(newfile); - return; + return EOF; } php_stream_write(newfile, digest, digest_len); @@ -3138,7 +3136,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (error) { spprintf(error, 4096, "unable to open new phar \"%s\" for writing", phar->fname); } - return; + return EOF; } if (phar->flags & PHAR_FILE_COMPRESSED_GZ) { @@ -3154,7 +3152,7 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (error) { spprintf(error, 4096, "unable to compress all contents of phar \"%s\" using zlib, PHP versions older than 5.2.6 have a buggy zlib", phar->fname); } - return; + return EOF; } php_stream_filter_append(&phar->fp->writefilters, filter); @@ -3184,9 +3182,10 @@ void phar_flush_ex(phar_archive_data *phar, zend_string *user_stub, bool is_defa if (error) { spprintf(error, 0, "unable to seek to __HALT_COMPILER(); in new phar \"%s\"", phar->fname); } + return EOF; } - return; + return EOF; cleanup: if (shared_cfp != NULL) { @@ -3198,6 +3197,8 @@ cleanup: entry->header_offset = 0; } } ZEND_HASH_FOREACH_END(); + + return EOF; } /* }}} */ diff --git a/ext/phar/phar_internal.h b/ext/phar/phar_internal.h index 2f9ae7c1c84..d8e319eeb6e 100644 --- a/ext/phar/phar_internal.h +++ b/ext/phar/phar_internal.h @@ -447,12 +447,12 @@ zend_result phar_copy_on_write(phar_archive_data **pphar); bool phar_is_tar(char *buf, char *fname); zend_result phar_parse_tarfile(php_stream* fp, char *fname, size_t fname_len, char *alias, size_t alias_len, phar_archive_data** pphar, uint32_t compression, char **error); zend_result phar_open_or_create_tar(char *fname, size_t fname_len, char *alias, size_t alias_len, int is_data, uint32_t options, phar_archive_data** pphar, char **error); -void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error); +int phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error); /* zip functions in zip.c */ int phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, char *alias, size_t alias_len, phar_archive_data** pphar, char **error); int phar_open_or_create_zip(char *fname, size_t fname_len, char *alias, size_t alias_len, int is_data, uint32_t options, phar_archive_data** pphar, char **error); -void phar_zip_flush(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error); +int phar_zip_flush(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error); #ifdef PHAR_MAIN extern const php_stream_wrapper php_stream_phar_wrapper; @@ -468,8 +468,8 @@ phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, size_t path_len, char dir, char **error, int security); phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security); zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security); -void phar_flush_ex(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error); -void phar_flush(phar_archive_data *archive, char **error); +int phar_flush_ex(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error); +int phar_flush(phar_archive_data *archive, char **error); zend_result phar_detect_phar_fname_ext(const char *filename, size_t filename_len, const char **ext_str, size_t *ext_len, int executable, int for_create, int is_complete); zend_result phar_split_fname(const char *filename, size_t filename_len, char **arch, size_t *arch_len, char **entry, size_t *entry_len, int executable, int for_create); diff --git a/ext/phar/stream.c b/ext/phar/stream.c index 41f1ed9c0dd..9a7fd3763e3 100644 --- a/ext/phar/stream.c +++ b/ext/phar/stream.c @@ -471,16 +471,17 @@ static ssize_t phar_stream_write(php_stream *stream, const char *buf, size_t cou static int phar_stream_flush(php_stream *stream) /* {{{ */ { char *error; + int ret; phar_entry_data *data = (phar_entry_data *) stream->abstract; if (data->internal_file->is_modified) { data->internal_file->timestamp = time(0); - phar_flush(data->phar, &error); + ret = phar_flush(data->phar, &error); if (error) { php_stream_wrapper_log_error(stream->wrapper, REPORT_ERRORS, "%s", error); efree(error); } - return EOF; + return ret; } else { return EOF; } diff --git a/ext/phar/tar.c b/ext/phar/tar.c index 63a9cdbf88f..d53d3a7107e 100644 --- a/ext/phar/tar.c +++ b/ext/phar/tar.c @@ -959,7 +959,7 @@ static int phar_tar_setupmetadata(zval *zv, void *argument) /* {{{ */ } /* }}} */ -void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error) /* {{{ */ +int phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error) /* {{{ */ { static const char newstub[] = "fname); } - return; + return EOF; } if (phar->is_data) { @@ -1001,7 +1001,7 @@ void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def if (entry.fp == NULL) { efree(entry.filename); spprintf(error, 0, "phar error: unable to create temporary file"); - return; + return -1; } if (phar->alias_len != php_stream_write(entry.fp, phar->alias, phar->alias_len)) { if (error) { @@ -1009,7 +1009,7 @@ void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def } php_stream_close(entry.fp); efree(entry.filename); - return; + return EOF; } entry.uncompressed_filesize = phar->alias_len; @@ -1029,7 +1029,7 @@ void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def if (error) { spprintf(error, 0, "illegal stub for tar-based phar \"%s\"", phar->fname); } - return; + return EOF; } size_t len = pos - ZSTR_VAL(user_stub) + strlen(halt_stub); @@ -1039,7 +1039,7 @@ void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def entry.fp = php_stream_fopen_tmpfile(); if (entry.fp == NULL) { spprintf(error, 0, "phar error: unable to create temporary file"); - return; + return EOF; } entry.uncompressed_filesize = len + end_sequence_len; @@ -1051,7 +1051,7 @@ void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def spprintf(error, 0, "unable to create stub from string in new tar-based phar \"%s\"", phar->fname); } php_stream_close(entry.fp); - return; + return EOF; } entry.filename = estrndup(".phar/stub.php", sizeof(".phar/stub.php")-1); @@ -1062,14 +1062,14 @@ void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def entry.fp = php_stream_fopen_tmpfile(); if (entry.fp == NULL) { spprintf(error, 0, "phar error: unable to create temporary file"); - return; + return EOF; } if (sizeof(newstub)-1 != php_stream_write(entry.fp, newstub, sizeof(newstub)-1)) { php_stream_close(entry.fp); if (error) { spprintf(error, 0, "unable to %s stub in%star-based phar \"%s\", failed", user_stub ? "overwrite" : "create", user_stub ? " " : " new ", phar->fname); } - return; + return EOF; } entry.uncompressed_filesize = entry.compressed_filesize = sizeof(newstub) - 1; @@ -1084,7 +1084,7 @@ void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def if (error) { spprintf(error, 0, "unable to create stub in tar-based phar \"%s\"", phar->fname); } - return; + return EOF; } } else { php_stream_close(entry.fp); @@ -1112,7 +1112,7 @@ nostub: if (must_close_old_file) { php_stream_close(oldfile); } - return; + return EOF; } pass.old = oldfile; @@ -1128,7 +1128,7 @@ nostub: if (must_close_old_file) { php_stream_close(oldfile); } - return; + return EOF; } } else { phar_entry_info newentry = {0}; @@ -1144,7 +1144,7 @@ nostub: if (must_close_old_file) { php_stream_close(oldfile); } - return; + return EOF; } if (ZEND_HASH_APPLY_KEEP != phar_tar_setmetadata(&phar->metadata_tracker, mentry, error)) { @@ -1152,7 +1152,7 @@ nostub: if (must_close_old_file) { php_stream_close(oldfile); } - return; + return EOF; } } } @@ -1166,7 +1166,7 @@ nostub: /* on error in the hash iterator above, error is set */ php_stream_close(newfile); - return; + return EOF; } zend_hash_apply_with_argument(&phar->manifest, phar_tar_writeheaders, (void *) &pass); @@ -1195,7 +1195,7 @@ nostub: } php_stream_close(newfile); - return; + return EOF; } entry.filename = ".phar/signature.bin"; @@ -1209,7 +1209,7 @@ nostub: php_stream_close(oldfile); } php_stream_close(newfile); - return; + return EOF; } #ifdef WORDS_BIGENDIAN # define PHAR_SET_32(destination, source) do { \ @@ -1235,7 +1235,7 @@ nostub: php_stream_close(oldfile); } php_stream_close(newfile); - return; + return EOF; } efree(signature); @@ -1249,7 +1249,7 @@ nostub: } /* error is set by writeheaders */ php_stream_close(newfile); - return; + return EOF; } } /* signature */ @@ -1265,7 +1265,7 @@ nostub: /* on error in the hash iterator above, error is set */ if (error && *error) { php_stream_close(newfile); - return; + return EOF; } if (phar->fp && pass.free_fp) { @@ -1292,7 +1292,7 @@ nostub: if (error) { spprintf(error, 0, "unable to open new phar \"%s\" for writing", phar->fname); } - return; + return EOF; } if (phar->flags & PHAR_FILE_COMPRESSED_GZ) { @@ -1316,7 +1316,7 @@ nostub: if (error) { spprintf(error, 4096, "unable to compress all contents of phar \"%s\" using zlib, PHP versions older than 5.2.6 have a buggy zlib", phar->fname); } - return; + return EOF; } php_stream_filter_append(&phar->fp->writefilters, filter); @@ -1343,5 +1343,6 @@ nostub: php_stream_close(newfile); } } + return EOF; } /* }}} */ diff --git a/ext/phar/zip.c b/ext/phar/zip.c index 026fe3935c5..5bb0e6986bb 100644 --- a/ext/phar/zip.c +++ b/ext/phar/zip.c @@ -1236,7 +1236,7 @@ static int phar_zip_applysignature(phar_archive_data *phar, struct _phar_zip_pas } /* }}} */ -void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error) /* {{{ */ +int phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error) /* {{{ */ { static const char newstub[] = "fname); } - return; + return EOF; } if (phar->is_data) { @@ -1273,14 +1273,14 @@ void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def entry.fp = php_stream_fopen_tmpfile(); if (entry.fp == NULL) { spprintf(error, 0, "phar error: unable to create temporary file"); - return; + return EOF; } if (phar->alias_len != php_stream_write(entry.fp, phar->alias, phar->alias_len)) { php_stream_close(entry.fp); if (error) { spprintf(error, 0, "unable to set alias in zip-based phar \"%s\"", phar->fname); } - return; + return EOF; } entry.uncompressed_filesize = entry.compressed_filesize = phar->alias_len; @@ -1295,7 +1295,7 @@ void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def /* register alias */ if (phar->alias_len) { if (FAILURE == phar_get_archive(&phar, phar->fname, phar->fname_len, phar->alias, phar->alias_len, error)) { - return; + return EOF; } } @@ -1307,7 +1307,7 @@ void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def if (error) { spprintf(error, 0, "illegal stub for zip-based phar \"%s\"", phar->fname); } - return; + return EOF; } size_t len = pos - ZSTR_VAL(user_stub) + strlen(halt_stub); @@ -1317,7 +1317,7 @@ void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def entry.fp = php_stream_fopen_tmpfile(); if (entry.fp == NULL) { spprintf(error, 0, "phar error: unable to create temporary file"); - return; + return EOF; } entry.uncompressed_filesize = len + end_sequence_len; @@ -1329,7 +1329,7 @@ void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def spprintf(error, 0, "unable to create stub from string in new zip-based phar \"%s\"", phar->fname); } php_stream_close(entry.fp); - return; + return EOF; } entry.filename = estrndup(".phar/stub.php", sizeof(".phar/stub.php")-1); @@ -1341,14 +1341,14 @@ void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def entry.fp = php_stream_fopen_tmpfile(); if (entry.fp == NULL) { spprintf(error, 0, "phar error: unable to create temporary file"); - return; + return EOF; } if (sizeof(newstub)-1 != php_stream_write(entry.fp, newstub, sizeof(newstub)-1)) { php_stream_close(entry.fp); if (error) { spprintf(error, 0, "unable to %s stub in%szip-based phar \"%s\", failed", user_stub ? "overwrite" : "create", user_stub ? " " : " new ", phar->fname); } - return; + return EOF; } entry.uncompressed_filesize = entry.compressed_filesize = sizeof(newstub) - 1; @@ -1363,7 +1363,7 @@ void phar_zip_flush(phar_archive_data *phar, zend_string *user_stub, bool is_def if (error) { spprintf(error, 0, "unable to create stub in zip-based phar \"%s\"", phar->fname); } - return; + return EOF; } } else { php_stream_close(entry.fp); @@ -1395,7 +1395,7 @@ fperror: if (error) { spprintf(error, 4096, "phar zip flush of \"%s\" failed: unable to open temporary file", phar->fname); } - return; + return EOF; } pass.centralfp = php_stream_fopen_tmpfile(); @@ -1435,7 +1435,7 @@ nocentralerror: if (must_close_old_file) { php_stream_close(oldfile); } - return; + return EOF; } if (FAILURE == phar_zip_applysignature(phar, &pass)) { @@ -1516,7 +1516,7 @@ nocentralerror: if (error) { spprintf(error, 4096, "unable to open new phar \"%s\" for writing", phar->fname); } - return; + return EOF; } php_stream_rewind(pass.filefp); php_stream_copy_to_stream_ex(pass.filefp, phar->fp, PHP_STREAM_COPY_ALL, NULL); @@ -1527,5 +1527,6 @@ nocentralerror: if (must_close_old_file) { php_stream_close(oldfile); } + return EOF; } /* }}} */