From aff47f8c124edd9e84b610253a8f63bbd0038788 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Mon, 28 Aug 2023 13:22:17 +0100 Subject: [PATCH] Fix bug #60110 (fclose(), file_put_contents(), copy() do not return false properly) --- ext/standard/file.c | 17 ++--- ext/standard/tests/file/bug60110.phpt | 94 +++++++++++++++++++++++++++ main/streams/streams.c | 9 ++- 3 files changed, 107 insertions(+), 13 deletions(-) create mode 100644 ext/standard/tests/file/bug60110.phpt diff --git a/ext/standard/file.c b/ext/standard/file.c index edaba57748cb3..033c704d867cf 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -582,9 +582,8 @@ PHP_FUNCTION(file_put_contents) numbytes = -1; break; } - php_stream_close(stream); - if (numbytes < 0) { + if (php_stream_close(stream) || numbytes < 0) { RETURN_FALSE; } @@ -782,11 +781,9 @@ PHPAPI PHP_FUNCTION(fclose) RETURN_FALSE; } - php_stream_free(stream, + RETURN_BOOL(!php_stream_free(stream, PHP_STREAM_FREE_KEEP_RSRC | - (stream->is_persistent ? PHP_STREAM_FREE_CLOSE_PERSISTENT : PHP_STREAM_FREE_CLOSE)); - - RETURN_TRUE; + (stream->is_persistent ? PHP_STREAM_FREE_CLOSE_PERSISTENT : PHP_STREAM_FREE_CLOSE))); } /* }}} */ @@ -1617,11 +1614,11 @@ PHPAPI int php_copy_file_ctx(const char *src, const char *dest, int src_flg, php if (srcstream && deststream) { ret = php_stream_copy_to_stream_ex(srcstream, deststream, PHP_STREAM_COPY_ALL, NULL); } - if (srcstream) { - php_stream_close(srcstream); + if (srcstream && php_stream_close(srcstream)) { + ret = FAILURE; } - 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 0000000000000..4298fa86fa174 --- /dev/null +++ b/ext/standard/tests/file/bug60110.phpt @@ -0,0 +1,94 @@ +--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) \ No newline at end of file diff --git a/main/streams/streams.c b/main/streams/streams.c index bc0b551b5208d..b8e661f1962e8 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -446,9 +446,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_ret; if (stream->flags & PHP_STREAM_FLAG_WAS_WRITTEN || stream->writefilters.head) { /* make sure everything is saved */ - _php_stream_flush(stream, 1); + flush_ret = _php_stream_flush(stream, 1); + } else { + flush_ret = 0; } /* If not called from the resource dtor, remove the stream from the resource list. */ @@ -472,10 +475,10 @@ 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); + return fclose(stream->stdiocast) || flush_ret; } - ret = stream->ops->close(stream, preserve_handle ? 0 : 1); + ret = stream->ops->close(stream, preserve_handle ? 0 : 1) || flush_ret; stream->abstract = NULL; /* tidy up any FILE* that might have been fdopened */