Bug #60671 [Bgs->Opn]: fread does not fail when operating on a write only stream

From: Date: Tue, 10 Jan 2012 13:38:55 +0000
Subject: Bug #60671 [Bgs->Opn]: fread does not fail when operating on a write only stream
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-166322@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=60671&edit=1

 ID:                 60671
 Updated by:         cataphract@php.net
 Reported by:        james dot turner dot phpninja at gmail dot com
 Summary:            fread does not fail when operating on a write only
                     stream
-Status:             Bogus
+Status:             Open
 Type:               Bug
 Package:            Streams related
 Operating System:   Ubuntu 11.04
 PHP Version:        5.3.8
-Assigned To:        
+Assigned To:        cataphract
 Block user comment: N
 Private report:     N

 New Comment:

All right, I'll give a bit more thought to this.


Previous Comments:
------------------------------------------------------------------------
[2012-01-08 21:45:18] phpmpan at mpan dot pl

You are right: no one should try to read from write-only stream. Also no one should try to divide by
0 and no one should pass something other than stream to fread. But people do it all the
time and that's why errors, warnings, exceptions and status codes exist. Using an invalid type
of a stream is an error too and should be reported.

If returning FALSE from fread breaks something, than I agree:
fread bahaviour should not be changed. However a warning does not break anything, so it
can be emitted. Since php_stream stores fopen flags in mode,
I believe this can be done easily:
--------------- BEGIN DIFF ---------------
diff php5.3-201201041830/ext/standard/file.c php5.3-201201041830-modified/ext/standard/file.c
1893a1894,1898
> 	
> 	if (strpbrk(stream->mode, "+r") == NULL) { /* r or any + mode is fine */
> 	    php_error_docref(NULL TSRMLS_CC, E_WARNING,
> 	          "Reading from a write-only stream");
> 	}
---------------- END DIFF ----------------
After this modification some of ext/standard/tests/file/007* tests will fail, but this is expected.

I agree that this case is a feature request.

------------------------------------------------------------------------
[2012-01-08 09:29:07] james dot turner dot phpninja at gmail dot com

I feel I should say that just because the behaviour is analogous to the way the underlying C works
does not make it correct nor useful.
While I concur that one should not really attempt to read from a write-only stream, I DO feel that
any attempt to perform such an action should warrant an exception/warning/error of some sort. This
is currently not the case and as a result can lead to potential indefinite loops given recommended
usage in the PHP documentation.
Perhaps this should be a feature/change request?

------------------------------------------------------------------------
[2012-01-08 02:51:16] phpmpan at mpan dot pl

Let me change your code just a bit:
-------------------------------------------
#include<stdio.h>

int main(void)
{
    FILE *f = fopen("/tmp/foobaz", "w");
    printf("feof: %d\n", feof(f));
    printf("ferror: %d\n", ferror(f)); // <- HERE
    printf("read: %zd\n", fread((char[100]) {}, 1, 100, f));
    printf("feof: %d\n", feof(f));
    printf("ferror: %d\n", ferror(f)); // <- AND HERE
    return 0;
}

gcc --std=c99 h.c && ./a.out
feof: 0
ferror: 0
read: 0
feof: 0
ferror: 1

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

In PHP there is no ferror. feof does work of both stdio's
feof and ferror, as described in the documentation [1]: "Returns TRUE
if the file pointer is at EOF or an error occurs".
Therefore, if we use analogy to stdio, feof should return TRUE in this
case.

[1] http://pl.php.net/manual/en/function.feof.php

------------------------------------------------------------------------
[2012-01-06 21:17:49] cataphract@php.net

This is not a bug. fread only returns false if given invalid arguments. The bug is that you try to
read from a stream that's write-only.

This C program has analogous behavior:

#include<stdio.h>

int main(void)
{
    FILE *f = fopen("/tmp/foobaz", "w");
    printf("feof: %d\n", feof(f));
    printf("read: %zd\n", fread((char[100]) {}, 1, 100, f));
    printf("feof: %d\n", feof(f));
    return 0;
}

gcc --std=c99 h.c && ./a.out
feof: 0
read: 0
feof: 0

------------------------------------------------------------------------
[2012-01-06 16:01:07] phpmpan at mpan dot pl

CONFIRMED for both 5.3.8 and 5.3.7 on Arch64, and for 5.3.4 on an unknown Linux.

Note however, that the test script provided by OP is wrong. It should be:
------------ BEGIN CODE ------------
$tmp = tempnam(sys_get_temp_dir(), 'test_');

$stream = fopen($tmp, 'w');
$data = "";

while(!feof($stream)){
  if(false === ($data = fread($stream, 8192))){
    break;         // ^---- no dot here
  };
}
------------- END CODE -------------
OP's code will fail regardless of the bug, because .= always produces a string.

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


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=60671


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


Thread (10 messages)

« previous php.bugs (#166322) next »