Skip to content

Commit c07cc85

Browse files
bukkailuuu1994
andauthored
Fix bug #60110 (fclose(), file_put_contents(), copy() do not return false properly) (#12067)
Propagate stream flush and close failures to the return value. Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>
1 parent eafcd68 commit c07cc85

7 files changed

Lines changed: 123 additions & 9 deletions

File tree

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ PHP NEWS
3636
. Fixed bug GH-23385 (SplDoublyLinkedList::serialize() use-after-free when
3737
__serialize() removes an element). (David Carlier)
3838

39+
- Standard:
40+
. Fixed bug #60110 (fclose(), file_put_contents(), copy() do not return false
41+
properly). (Jakub Zelenka, Ilija Tovilo)
42+
3943

4044
10 Sep 2026, PHP 8.6.0beta3
4145

UPGRADING

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,9 @@ PHP 8.6 UPGRADE NOTES
733733
directive when $details is true. It holds the built-in default value of the
734734
directive (or null if it has none), independent of values set in php.ini,
735735
on the command line, or at runtime.
736+
. fclose(), file_put_contents() and copy() now return false when flushing
737+
or closing the stream fails. Previously such failures were silently
738+
ignored.
736739

737740
- Zip:
738741
. zip_entry_close() return type has been narrowed from bool to true. The

ext/phar/stream.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ static int phar_stream_flush(php_stream *stream) /* {{{ */
488488
}
489489
return ret;
490490
} else {
491-
return EOF;
491+
return 0;
492492
}
493493
}
494494
/* }}} */

ext/standard/file.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -571,10 +571,10 @@ PHP_FUNCTION(file_put_contents)
571571
numbytes = -1;
572572
break;
573573
}
574-
php_stream_close(stream);
574+
int close_result = php_stream_close(stream);
575575
php_stream_error_operation_end(context);
576576

577-
if (numbytes < 0) {
577+
if (numbytes < 0 || close_result) {
578578
RETURN_FALSE;
579579
}
580580

@@ -776,12 +776,12 @@ PHPAPI PHP_FUNCTION(fclose)
776776
}
777777

778778
php_stream_error_operation_begin();
779-
php_stream_free(stream,
779+
int free_result = php_stream_free(stream,
780780
PHP_STREAM_FREE_KEEP_RSRC |
781781
(stream->is_persistent ? PHP_STREAM_FREE_CLOSE_PERSISTENT : PHP_STREAM_FREE_CLOSE));
782782
php_stream_error_operation_end_for_stream(stream);
783783

784-
RETURN_TRUE;
784+
RETURN_BOOL(!free_result);
785785
}
786786
/* }}} */
787787

@@ -1594,8 +1594,8 @@ PHPAPI zend_result php_copy_file_ctx(const char *src, const char *dest, int src_
15941594
ret = php_stream_copy_to_stream_ex(srcstream, deststream, PHP_STREAM_COPY_ALL, NULL);
15951595
}
15961596
php_stream_close(srcstream);
1597-
if (deststream) {
1598-
php_stream_close(deststream);
1597+
if (deststream && php_stream_close(deststream)) {
1598+
ret = FAILURE;
15991599
}
16001600
return ret;
16011601
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
--TEST--
2+
Bug #60110 (fclose(), file_put_contents(), copy() do not return false properly)
3+
--FILE--
4+
<?php
5+
class astream
6+
{
7+
public static $max = 100;
8+
9+
public $context;
10+
11+
protected $read = 0;
12+
13+
protected $flush = false;
14+
15+
function stream_open($path, $mode) {
16+
$this->flush = basename($path) === 'flush';
17+
return true;
18+
}
19+
20+
function stream_write($data) {
21+
var_dump($data);
22+
return strlen($data);
23+
}
24+
25+
function stream_read($length) {
26+
if ($length > self::$max - $this->read) {
27+
$length = self::$max - $this->read;
28+
}
29+
$this->read += $length;
30+
return str_repeat('a', $length);
31+
}
32+
33+
function stream_tell() {
34+
return $this->read;
35+
}
36+
37+
function stream_eof() {
38+
return $this->read == self::$max;
39+
}
40+
41+
function stream_flush() {
42+
return $this->flush;
43+
}
44+
45+
function stream_stat() {
46+
return fstat(fopen('php://memory', "r"));
47+
}
48+
49+
function url_stat() {
50+
return fstat(fopen('php://memory', "r"));
51+
}
52+
}
53+
54+
stream_wrapper_register('as', 'astream');
55+
56+
$stream = fopen('as://flush', 'r+');
57+
var_dump(fwrite($stream, "data"));
58+
var_dump(fread($stream, 3));
59+
var_dump(fclose($stream));
60+
61+
$stream = fopen('as://nothing', 'r+');
62+
var_dump(fwrite($stream, "data"));
63+
var_dump(fread($stream, 3));
64+
var_dump(fclose($stream));
65+
66+
var_dump(file_put_contents('as://', 'test nothing'));
67+
var_dump(file_put_contents('as://flush', 'test flush'));
68+
69+
$path = __DIR__ . '/bug60110_test_file.txt';
70+
var_dump(file_put_contents($path, 'sdata'));
71+
var_dump(copy($path, 'as://nothing'));
72+
var_dump(copy($path, 'as://flush'));
73+
?>
74+
--CLEAN--
75+
<?php
76+
@unlink(__DIR__ . '/bug60110_test_file.txt');
77+
?>
78+
--EXPECT--
79+
string(4) "data"
80+
int(4)
81+
string(3) "aaa"
82+
bool(true)
83+
string(4) "data"
84+
int(4)
85+
string(3) "aaa"
86+
bool(false)
87+
string(12) "test nothing"
88+
bool(false)
89+
string(10) "test flush"
90+
int(10)
91+
int(5)
92+
string(5) "sdata"
93+
bool(false)
94+
string(5) "sdata"
95+
bool(true)

main/php_streams.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,9 @@ typedef struct _php_stream_ops {
114114
/* stdio like functions - these are mandatory! */
115115
ssize_t (*write)(php_stream *stream, const char *buf, size_t count);
116116
ssize_t (*read)(php_stream *stream, char *buf, size_t count);
117+
/* returns 0 on success and non-zero on failure */
117118
int (*close)(php_stream *stream, int close_handle);
119+
/* returns 0 on success (including nothing to flush) and non-zero on failure */
118120
int (*flush)(php_stream *stream);
119121

120122
const char *label; /* label for this ops structure */

main/streams/streams.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,9 +335,12 @@ fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remov
335335
(close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0);
336336
#endif
337337

338+
int flush_result;
338339
if (stream->flags & PHP_STREAM_FLAG_WAS_WRITTEN || stream->writefilters.head) {
339340
/* make sure everything is saved */
340-
php_stream_flush_ex(stream, true);
341+
flush_result = php_stream_flush_ex(stream, true);
342+
} else {
343+
flush_result = 0;
341344
}
342345

343346
/* 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
361364
Let's let the cookie code clean it all up.
362365
*/
363366
stream->in_free = 0;
364-
return fclose(stream->stdiocast);
367+
ret = fclose(stream->stdiocast);
368+
if (!ret) {
369+
ret = flush_result;
370+
}
371+
return ret;
365372
}
366373

367374
ret = stream->ops->close(stream, preserve_handle ? 0 : 1);
375+
if (!ret) {
376+
ret = flush_result;
377+
}
368378
stream->abstract = NULL;
369379

370380
/* tidy up any FILE* that might have been fdopened */

0 commit comments

Comments
 (0)