Edit report at https://bugs.php.net/bug.php?id=60110&edit=1
ID: 60110
Updated by: cataphr...@php.net
Reported by: tom at punkave dot com
Summary: fclose(), file_put_contents(), copy() do not return
false properly
Status: Open
Type: Bug
Package: Filesystem function related
Operating System: all
PHP Version: 5.3.8
Block user comment: N
Private report: N
New Comment:
fclose() calls php_stream_close since PHP 5.4, see
http://svn.php.net/viewvc?view=revision&revision=309491
Previous Comments:
------------------------------------------------------------------------
[2011-12-29 04:45:53] tstarl...@php.net
It would be nice if the return value of php_stream_close() was checked, but
note that fclose() does not call php_stream_close(), it calls
zend_list_delete(), which can't return anything sensible because
rsrc_dtor_func_t returns void.
It would be ugly and difficult to change rsrc_dtor_func_t at this stage, and in
any case, during shutdown or zval destruction we don't really care about flush
failures. I think the way to go would be to call php_stream_close() from
fclose() before calling the resource destructor. There probably won't be any
dangling pointers, it looks pretty well guarded already.
As for cataphract's concern: we could audit all the wrapper close functions,
there's not that many of them is there?
------------------------------------------------------------------------
[2011-10-24 17:05:00] tom at punkave dot com
How about a close_checks_flush php.ini flag which defaults to false in 5.3 and
to
true in 5.4?
------------------------------------------------------------------------
[2011-10-21 21:35:52] tom at punkave dot com
This can definitely happen with the regular stdio stuff. stdio employs
buffering
as a matter of course. It is a serious bug, not a change in behavior.
As for stream wrappers, the documentation specifies what stream_flush is
supposed to return, and fflush() would already be failing for people with bad
stream wrappers who did not heed that documentation.
stream_close is not supposed to return anything but is not affected by this bug
because stream_flush has already been called (and cheerfully ignored) before
stream_close is called (I checked).
So there is no need to change the behavior of stream_close (which would be a bc
break). We just need to pay attention to what stream_flush is already telling
us.
------------------------------------------------------------------------
[2011-10-21 21:23:22] cataphr...@php.net
See bug #53328. This is a good candidate for trunk, for stable versions I fear
(perhaps unfoundedly) that, because the return value of php_stream_close/free
is almost always ignored, some wrappers might have gotten away with incorrect
return values on the close handler.
------------------------------------------------------------------------
[2011-10-21 19:38:23] tom at punkave dot com
Description:
------------
The fclose() function does not check the result of flushing the stream:
if (!stream->is_persistent) {
zend_list_delete(stream->rsrc_id);
} else {
php_stream_pclose(stream);
}
RETURN_TRUE;
php_stream_pclose has a return value but it is ignored.
php_stream_pclose, in turn, calls _php_stream_free which flushes the stream but
does not check the return value either:
/* make sure everything is saved */
_php_stream_flush(stream, 1 TSRMLS_CC);
Contrary to the comment we did not make sure of anything (:
So the fix has to be made at least as deep as _php_stream_flush to be
effective.
I have verified that _php_stream_flush does pay attention to the result of the
underlying flush operation. Note that fflush() reports write errors
successfully
while fclose() doesn't).
In many environments, including stdio (plain old files), the final write to
disk
might not sometimes occur until the stream is flushed by _php_stream_flush. If
this fails, for instance because the disk is full, PHP pays no heed to the
error.
This is especially obvious if you use a stream wrapper that does its actual
writing when the stream is closed (for instance storing an object with a single
HTTP request), but again, it could happen with normal files too.
Because of this deeper issue with _php_stream_free, all other PHP convenience
functions for files such as copy() and file_put_contents() also fail to
correctly report false when the final flush of data to disk fails when closing
a
file.
This has serious consequences for any application that is counting on data
integrity, which would be pretty much every application that uses files I guess.
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=60110&edit=1
