Bug #60110 [Opn]: fclose(), file_put_contents(), copy() do not return false properly

From: Date: Mon, 20 Aug 2012 05:38:22 +0000
Subject: Bug #60110 [Opn]: fclose(), file_put_contents(), copy() do not return false properly
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-172709@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=60110&edit=1

 ID:                 60110
 Updated by:         stas@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:

This looks like a bug, I think it'd be fine to have the fix in 5.4. Any pull reqs 
with fixes?


Previous Comments:
------------------------------------------------------------------------
[2011-12-30 17:25:04] cataphract@php.net

fclose() calls php_stream_close since PHP 5.4, see http://svn.php.net/viewvc?view=revision&revision=309491

------------------------------------------------------------------------
[2011-12-29 04:45:53] tstarling@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] cataphract@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.

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=60110


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=60110&edit=1


Thread (12 messages)

« previous php.bugs (#172709) next »