Stream related error handling is buggy

[GoogleCodeExporter]

I know at least 1 place, where copy and paste from PHP docs introduces an 
error. Function writeWholeBuffer, in functions.php, has bad implementation. See 
details at http://www.php.net/manual/en/function.fwrite.php#96951. As a result, 
I encounter the following problem: if a user is disconnected on its own, fwrite 
returns 0, and PHPWS keeps sending data to the brocken socket. Reading from 
network stream seems also lacking error handling.

Original issue reported on code.google.com by mo...@tushino.ru on 29 Dec 2011 at 6:13

[GoogleCodeExporter]

Also in server.php lines starting 153 is problematic, because $bytes got from 
strlen never can be false.

Original comment by mo...@tushino.ru on 29 Dec 2011 at 6:46

[GoogleCodeExporter]

Regarding read errors, lines 159, 160 of server.php should be:
if($buffer === false){
    $socket->close();

and line 70 of functions.php should be replaced with:

$result = fread($resource, $buffsize);
if($result === false || feof($resource))
{
        return false;
}
$buffer .= $result; 

Original comment by mo...@tushino.ru on 29 Dec 2011 at 7:06

[GoogleCodeExporter]

Agreed, error handling is not as it should be yet. Will have at your 
suggestions later :)

Original comment by ch...@devristo.com on 29 Dec 2011 at 7:48

[GoogleCodeExporter]

The problem is that these errors lead to infinite looping, and keep internal 
references to broken streams - I got 100% CPU usage after any client 
disonnection and a lot of continuously printed warnings.

One correction to my code above: the check for feof should be done before 
calling fread.

Original comment by mo...@tushino.ru on 29 Dec 2011 at 8:34

[GoogleCodeExporter]

I know :) Had that issue here as well (the infinite looping)

Original comment by ch...@devristo.com on 30 Dec 2011 at 12:28

[GoogleCodeExporter]

Should the feof break instead of return? Feof is not an error imho and should 
just return the buffer as is.

Original comment by ch...@devristo.com on 30 Dec 2011 at 12:29

[GoogleCodeExporter]

To me feof is the only way to check for broken connection. If you know other 
way, let use it. Without feof I have endless loop while reading from broken 
socket. So break will not solve the problem - we must dispose invalid object. 
This is why I suggest, that feof should go first before fread and if it's true, 
just exit from the function returning false(!).

Original comment by mo...@tushino.ru on 30 Dec 2011 at 1:53

[GoogleCodeExporter]

Okay, will return false then ;)

Original comment by ch...@devristo.com on 30 Dec 2011 at 3:59

[GoogleCodeExporter]

In RC1, module functions.php, line 76, there is still the checkup for '|| 
feof($resource)' remaining. As I wrote at 29-th Dec, this is an error, because 
we could read new portion of data here, and both $resource can be at eof, but 
we need to add this new portion to the buffer, in order to see if it fits 
$metadata['unread_bytes']. You have already moved feof a couple lines up, and 
that's ok, so the old checkup is excessive.

Original comment by mo...@tushino.ru on 18 Feb 2012 at 7:03

[GoogleCodeExporter]

Thanks, will remove it! Bit sloppy from my side :$

Original comment by ch...@devristo.com on 25 Feb 2012 at 9:33

[GoogleCodeExporter]

Original comment by ch...@devristo.com on 30 May 2012 at 7:16

• Changed state: Fixed