Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

fix(read stream) consume buffer multiple times #53

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 27 additions & 15 deletions Connection/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -658,12 +658,14 @@ public function getRemoteAddress()
* Warning: if this method returns false, it means that the buffer is empty.
* You should use the Hoa\Stream::setStreamBlocking(true) method.
*
* @param int $length Length.
* @param int $flags Flags.
* @param int $length Length.
* @param int $flags Flags.
* @param boolean $keepReading If we loop over stream to seek length data ot not.
* @param int $chunk Length bloc read in the stream.
* @return string
* @throws \Hoa\Socket\Exception
*/
public function read($length, $flags = 0)
public function read($length, $flags = 0, $keepReading = true, $chunk = null)
{
if (null === $this->getStream()) {
throw new Socket\Exception(
Expand All @@ -681,21 +683,31 @@ public function read($length, $flags = 0)
);
}

if (true === $this->isEncrypted()) {
return fread($this->getStream(), $length);
if (is_int($chunk)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- if (is_int($chunk))
+ if (is_int($chunk) && 0 < $chunk))

to ensure the chunk is greater than zero. Could it be greater than or equal to zero?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You right! never trust the userland!

stream_set_chunk_size($this->getStream(), $chunk);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we collect the previous chunk size (returned by stream_set_chunk_size) and restore it just after the call? I'm not sure how it acts with concurrent accesses…

}

if (false === $this->isRemoteAddressConsidered()) {
return stream_socket_recvfrom($this->getStream(), $length, $flags);
}
$out = '';

$out = stream_socket_recvfrom(
$this->getStream(),
$length,
$flags,
$address
);
$this->_remoteAddress = !empty($address) ? $address : null;
do {
$readLength = $length - strlen($out);

if (0 >= $readLength) {
break;
}

if (true === $this->isEncrypted()) {
$buffer = fread($this->getStream(), $readLength);
} else {
$buffer = stream_socket_recvfrom($this->getStream(), $readLength, $flags);
}

if ('' !== $buffer && is_string($buffer)) {
$out .= $buffer;
} else {
break;
}
} while($keepReading);

return $out;
}
Expand Down