From 1d7e76cbf41b6d11256f9267974b9b88414831c5 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Tue, 15 Sep 2026 17:55:20 +0200 Subject: [PATCH] Fix bug #60110 (fclose(), file_put_contents(), copy() do not return false properly) Propagate stream flush and close failures to the return value. Co-authored-by: Ilija Tovilo --- NEWS | 4 ++ UPGRADING | 3 + ext/phar/stream.c | 2 +- ext/standard/file.c | 12 ++-- ext/standard/tests/file/bug60110.phpt | 95 +++++++++++++++++++++++++++ main/php_streams.h | 2 + main/streams/streams.c | 14 +++- 7 files changed, 123 insertions(+), 9 deletions(-) create mode 100644 ext/standard/tests/file/bug60110.phpt diff --git a/NEWS b/NEWS index 193316edf845..385f15ac4b43 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,10 @@ PHP NEWS . Fixed bug GH-23385 (SplDoublyLinkedList::serialize() use-after-free when __serialize() removes an element). (David Carlier) +- Standard: + . Fixed bug #60110 (fclose(), file_put_contents(), copy() do not return false + properly). (Jakub Zelenka, Ilija Tovilo) + 10 Sep 2026, PHP 8.6.0beta3 diff --git a/UPGRADING b/UPGRADING index ee7f4c3ab73d..d7310022e815 100644 --- a/UPGRADING +++ b/UPGRADING @@ -730,6 +730,9 @@ PHP 8.6 UPGRADE NOTES directive when $details is true. It holds the built-in default value of the directive (or null if it has none), independent of values set in php.ini, on the command line, or at runtime. + . fclose(), file_put_contents() and copy() now return false when flushing + or closing the stream fails. Previously such failures were silently + ignored. - Zip: . zip_entry_close() return type has been narrowed from bool to true. The diff --git a/ext/phar/stream.c b/ext/phar/stream.c index 9474498e3892..335f8a073424 100644 --- a/ext/phar/stream.c +++ b/ext/phar/stream.c @@ -488,7 +488,7 @@ static int phar_stream_flush(php_stream *stream) /* {{{ */ } return ret; } else { - return EOF; + return 0; } } /* }}} */ diff --git a/ext/standard/file.c b/ext/standard/file.c index b52e5ba9525f..19e60a41b05b 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -571,10 +571,10 @@ PHP_FUNCTION(file_put_contents) numbytes = -1; break; } - php_stream_close(stream); + int close_result = php_stream_close(stream); php_stream_error_operation_end(context); - if (numbytes < 0) { + if (numbytes < 0 || close_result) { RETURN_FALSE; } @@ -776,12 +776,12 @@ PHPAPI PHP_FUNCTION(fclose) } php_stream_error_operation_begin(); - php_stream_free(stream, + int free_result = php_stream_free(stream, PHP_STREAM_FREE_KEEP_RSRC | (stream->is_persistent ? PHP_STREAM_FREE_CLOSE_PERSISTENT : PHP_STREAM_FREE_CLOSE)); php_stream_error_operation_end_for_stream(stream); - RETURN_TRUE; + RETURN_BOOL(!free_result); } /* }}} */ @@ -1594,8 +1594,8 @@ PHPAPI zend_result php_copy_file_ctx(const char *src, const char *dest, int src_ ret = php_stream_copy_to_stream_ex(srcstream, deststream, PHP_STREAM_COPY_ALL, NULL); } php_stream_close(srcstream); - if (deststream) { - php_stream_close(deststream); + if (deststream && php_stream_close(deststream)) { + ret = FAILURE; } return ret; } diff --git a/ext/standard/tests/file/bug60110.phpt b/ext/standard/tests/file/bug60110.phpt new file mode 100644 index 000000000000..2bfb5a4a2660 --- /dev/null +++ b/ext/standard/tests/file/bug60110.phpt @@ -0,0 +1,95 @@ +--TEST-- +Bug #60110 (fclose(), file_put_contents(), copy() do not return false properly) +--FILE-- +flush = basename($path) === 'flush'; + return true; + } + + function stream_write($data) { + var_dump($data); + return strlen($data); + } + + function stream_read($length) { + if ($length > self::$max - $this->read) { + $length = self::$max - $this->read; + } + $this->read += $length; + return str_repeat('a', $length); + } + + function stream_tell() { + return $this->read; + } + + function stream_eof() { + return $this->read == self::$max; + } + + function stream_flush() { + return $this->flush; + } + + function stream_stat() { + return fstat(fopen('php://memory', "r")); + } + + function url_stat() { + return fstat(fopen('php://memory', "r")); + } +} + +stream_wrapper_register('as', 'astream'); + +$stream = fopen('as://flush', 'r+'); +var_dump(fwrite($stream, "data")); +var_dump(fread($stream, 3)); +var_dump(fclose($stream)); + +$stream = fopen('as://nothing', 'r+'); +var_dump(fwrite($stream, "data")); +var_dump(fread($stream, 3)); +var_dump(fclose($stream)); + +var_dump(file_put_contents('as://', 'test nothing')); +var_dump(file_put_contents('as://flush', 'test flush')); + +$path = __DIR__ . '/bug60110_test_file.txt'; +var_dump(file_put_contents($path, 'sdata')); +var_dump(copy($path, 'as://nothing')); +var_dump(copy($path, 'as://flush')); +?> +--CLEAN-- + +--EXPECT-- +string(4) "data" +int(4) +string(3) "aaa" +bool(true) +string(4) "data" +int(4) +string(3) "aaa" +bool(false) +string(12) "test nothing" +bool(false) +string(10) "test flush" +int(10) +int(5) +string(5) "sdata" +bool(false) +string(5) "sdata" +bool(true) diff --git a/main/php_streams.h b/main/php_streams.h index 8a8fa4b3a567..fb0c57ecf83d 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -114,7 +114,9 @@ typedef struct _php_stream_ops { /* stdio like functions - these are mandatory! */ ssize_t (*write)(php_stream *stream, const char *buf, size_t count); ssize_t (*read)(php_stream *stream, char *buf, size_t count); + /* returns 0 on success and non-zero on failure */ int (*close)(php_stream *stream, int close_handle); + /* returns 0 on success (including nothing to flush) and non-zero on failure */ int (*flush)(php_stream *stream); const char *label; /* label for this ops structure */ diff --git a/main/streams/streams.c b/main/streams/streams.c index a09a2180921d..7cd63f0038d3 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -335,9 +335,12 @@ fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remov (close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0); #endif + int flush_result; if (stream->flags & PHP_STREAM_FLAG_WAS_WRITTEN || stream->writefilters.head) { /* make sure everything is saved */ - php_stream_flush_ex(stream, true); + flush_result = php_stream_flush_ex(stream, true); + } else { + flush_result = 0; } /* If not called from the resource dtor, remove the stream from the resource list. */ @@ -361,10 +364,17 @@ fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remov Let's let the cookie code clean it all up. */ stream->in_free = 0; - return fclose(stream->stdiocast); + ret = fclose(stream->stdiocast); + if (!ret) { + ret = flush_result; + } + return ret; } ret = stream->ops->close(stream, preserve_handle ? 0 : 1); + if (!ret) { + ret = flush_result; + } stream->abstract = NULL; /* tidy up any FILE* that might have been fdopened */