From 8944361dbe69c82da22009b267950b97c92f1650 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Tue, 10 Dec 2019 19:22:57 -0500 Subject: [PATCH 01/14] Update to latest react/socket and drop react/http for tests --- composer.json | 5 +-- tests/ab/clientRunner.php | 78 +++++++++++++++++++------------------ tests/ab/startServer.php | 81 ++++++++++++++++++++++----------------- 3 files changed, 89 insertions(+), 75 deletions(-) diff --git a/composer.json b/composer.json index 224066b..d9ff35d 100644 --- a/composer.json +++ b/composer.json @@ -25,8 +25,7 @@ "guzzlehttp/psr7": "^1.0" }, "require-dev": { - "react/http": "^0.4.1", - "react/socket-client": "^0.4.3", - "phpunit/phpunit": "4.8.*" + "phpunit/phpunit": "4.8.*", + "react/socket": "^1.3" } } diff --git a/tests/ab/clientRunner.php b/tests/ab/clientRunner.php index 0c5578a..274f82d 100644 --- a/tests/ab/clientRunner.php +++ b/tests/ab/clientRunner.php @@ -1,7 +1,14 @@ createCached('8.8.8.8', $loop); - -$factory = new \React\SocketClient\Connector($loop, $dnsResolver); +$connector = new Connector($loop); function echoStreamerFactory($conn) { - return new \Ratchet\RFC6455\Messaging\MessageBuffer( - new \Ratchet\RFC6455\Messaging\CloseFrameChecker, - function (\Ratchet\RFC6455\Messaging\MessageInterface $msg) use ($conn) { + return new MessageBuffer( + new CloseFrameChecker, + function (MessageInterface $msg) use ($conn) { /** @var Frame $frame */ foreach ($msg as $frame) { $frame->maskPayload(); } $conn->write($msg->getContents()); }, - function (\Ratchet\RFC6455\Messaging\FrameInterface $frame) use ($conn) { + function (FrameInterface $frame) use ($conn) { switch ($frame->getOpcode()) { case Frame::OP_PING: return $conn->write((new Frame($frame->getPayload(), true, Frame::OP_PONG))->maskPayload()->getContents()); @@ -42,22 +46,22 @@ function echoStreamerFactory($conn) } function getTestCases() { - global $factory; global $testServer; + global $connector; $deferred = new Deferred(); - $factory->create($testServer, 9001)->then(function (\React\Stream\Stream $stream) use ($deferred) { - $cn = new \Ratchet\RFC6455\Handshake\ClientNegotiator(); + $connector->connect($testServer . ':9001')->then(function (ConnectionInterface $connection) use ($deferred) { + $cn = new ClientNegotiator(); $cnRequest = $cn->generateRequest(new Uri('ws://127.0.0.1:9001/getCaseCount')); $rawResponse = ""; $response = null; - /** @var \Ratchet\RFC6455\Messaging\Streaming\MessageBuffer $ms */ + /** @var MessageBuffer $ms */ $ms = null; - $stream->on('data', function ($data) use ($stream, &$rawResponse, &$response, &$ms, $cn, $deferred, &$context, $cnRequest) { + $connection->on('data', function ($data) use ($connection, &$rawResponse, &$response, &$ms, $cn, $deferred, &$context, $cnRequest) { if ($response === null) { $rawResponse .= $data; $pos = strpos($rawResponse, "\r\n\r\n"); @@ -67,14 +71,14 @@ function getTestCases() { $response = \GuzzleHttp\Psr7\parse_response($rawResponse); if (!$cn->validateResponse($cnRequest, $response)) { - $stream->end(); + $connection->end(); $deferred->reject(); } else { - $ms = new \Ratchet\RFC6455\Messaging\MessageBuffer( - new \Ratchet\RFC6455\Messaging\CloseFrameChecker, - function (\Ratchet\RFC6455\Messaging\MessageInterface $msg) use ($deferred, $stream) { + $ms = new MessageBuffer( + new CloseFrameChecker, + function (MessageInterface $msg) use ($deferred, $connection) { $deferred->resolve($msg->getPayload()); - $stream->close(); + $connection->close(); }, null, false @@ -89,7 +93,7 @@ function getTestCases() { } }); - $stream->write(\GuzzleHttp\Psr7\str($cnRequest)); + $connection->write(\GuzzleHttp\Psr7\str($cnRequest)); }); return $deferred->promise(); @@ -97,15 +101,15 @@ function getTestCases() { function runTest($case) { - global $factory; + global $connector; global $testServer; $casePath = "/runCase?case={$case}&agent=" . AGENT; $deferred = new Deferred(); - $factory->create($testServer, 9001)->then(function (\React\Stream\Stream $stream) use ($deferred, $casePath, $case) { - $cn = new \Ratchet\RFC6455\Handshake\ClientNegotiator(); + $connector->connect($testServer . ':9001')->then(function (ConnectionInterface $connection) use ($deferred, $casePath, $case) { + $cn = new ClientNegotiator(); $cnRequest = $cn->generateRequest(new Uri('ws://127.0.0.1:9001' . $casePath)); $rawResponse = ""; @@ -113,7 +117,7 @@ function runTest($case) $ms = null; - $stream->on('data', function ($data) use ($stream, &$rawResponse, &$response, &$ms, $cn, $deferred, &$context, $cnRequest) { + $connection->on('data', function ($data) use ($connection, &$rawResponse, &$response, &$ms, $cn, $deferred, &$context, $cnRequest) { if ($response === null) { $rawResponse .= $data; $pos = strpos($rawResponse, "\r\n\r\n"); @@ -123,10 +127,10 @@ function runTest($case) $response = \GuzzleHttp\Psr7\parse_response($rawResponse); if (!$cn->validateResponse($cnRequest, $response)) { - $stream->end(); + $connection->end(); $deferred->reject(); } else { - $ms = echoStreamerFactory($stream); + $ms = echoStreamerFactory($connection); } } } @@ -137,34 +141,34 @@ function runTest($case) } }); - $stream->on('close', function () use ($deferred) { + $connection->on('close', function () use ($deferred) { $deferred->resolve(); }); - $stream->write(\GuzzleHttp\Psr7\str($cnRequest)); + $connection->write(\GuzzleHttp\Psr7\str($cnRequest)); }); return $deferred->promise(); } function createReport() { - global $factory; + global $connector; global $testServer; $deferred = new Deferred(); - $factory->create($testServer, 9001)->then(function (\React\Stream\Stream $stream) use ($deferred) { + $connector->connect($testServer . ':9001')->then(function (ConnectionInterface $connection) use ($deferred) { $reportPath = "/updateReports?agent=" . AGENT . "&shutdownOnComplete=true"; - $cn = new \Ratchet\RFC6455\Handshake\ClientNegotiator(); + $cn = new ClientNegotiator(); $cnRequest = $cn->generateRequest(new Uri('ws://127.0.0.1:9001' . $reportPath)); $rawResponse = ""; $response = null; - /** @var \Ratchet\RFC6455\Messaging\MessageBuffer $ms */ + /** @var MessageBuffer $ms */ $ms = null; - $stream->on('data', function ($data) use ($stream, &$rawResponse, &$response, &$ms, $cn, $deferred, &$context, $cnRequest) { + $connection->on('data', function ($data) use ($connection, &$rawResponse, &$response, &$ms, $cn, $deferred, &$context, $cnRequest) { if ($response === null) { $rawResponse .= $data; $pos = strpos($rawResponse, "\r\n\r\n"); @@ -174,12 +178,12 @@ function createReport() { $response = \GuzzleHttp\Psr7\parse_response($rawResponse); if (!$cn->validateResponse($cnRequest, $response)) { - $stream->end(); + $connection->end(); $deferred->reject(); } else { - $ms = new \Ratchet\RFC6455\Messaging\MessageBuffer( - new \Ratchet\RFC6455\Messaging\CloseFrameChecker, - function (\Ratchet\RFC6455\Messaging\MessageInterface $msg) use ($deferred, $stream) { + $ms = new MessageBuffer( + new CloseFrameChecker, + function (MessageInterface $msg) use ($deferred, $stream) { $deferred->resolve($msg->getPayload()); $stream->close(); }, @@ -196,7 +200,7 @@ function createReport() { } }); - $stream->write(\GuzzleHttp\Psr7\str($cnRequest)); + $connection->write(\GuzzleHttp\Psr7\str($cnRequest)); }); return $deferred->promise(); diff --git a/tests/ab/startServer.php b/tests/ab/startServer.php index b256ec2..4baf884 100644 --- a/tests/ab/startServer.php +++ b/tests/ab/startServer.php @@ -7,49 +7,60 @@ require_once __DIR__ . "/../bootstrap.php"; $loop = \React\EventLoop\Factory::create(); -$socket = new \React\Socket\Server($loop); -$server = new \React\Http\Server($socket); +$socket = new \React\Socket\Server('127.0.0.1:9001', $loop); $closeFrameChecker = new \Ratchet\RFC6455\Messaging\CloseFrameChecker; $negotiator = new \Ratchet\RFC6455\Handshake\ServerNegotiator(new \Ratchet\RFC6455\Handshake\RequestVerifier); $uException = new \UnderflowException; -$server->on('request', function (\React\Http\Request $request, \React\Http\Response $response) use ($negotiator, $closeFrameChecker, $uException) { - $psrRequest = new \GuzzleHttp\Psr7\Request($request->getMethod(), $request->getPath(), $request->getHeaders()); - - $negotiatorResponse = $negotiator->handshake($psrRequest); - - $response->writeHead( - $negotiatorResponse->getStatusCode(), - array_merge( - $negotiatorResponse->getHeaders(), - ["Content-Length" => "0"] - ) - ); - - if ($negotiatorResponse->getStatusCode() !== 101) { - $response->end(); - return; - } - - $parser = new \Ratchet\RFC6455\Messaging\MessageBuffer($closeFrameChecker, function(MessageInterface $message) use ($response) { - $response->write($message->getContents()); - }, function(FrameInterface $frame) use ($response, &$parser) { - switch ($frame->getOpCode()) { - case Frame::OP_CLOSE: - $response->end($frame->getContents()); - break; - case Frame::OP_PING: - $response->write($parser->newFrame($frame->getPayload(), true, Frame::OP_PONG)->getContents()); - break; +$socket->on('connection', function (React\Socket\ConnectionInterface $connection) use ($negotiator, $closeFrameChecker, $uException) { + $headerComplete = false; + $buffer = ''; + $parser = null; + $connection->on('data', function ($data) use ($connection, &$parser, &$headerComplete, &$buffer, $negotiator, $closeFrameChecker, $uException) { + if ($headerComplete) { + $parser->onData($data); + return; } - }, true, function() use ($uException) { - return $uException; - }); - $request->on('data', [$parser, 'onData']); + $buffer .= $data; + $parts = explode("\r\n\r\n", $buffer); + if (count($parts) < 2) { + return; + } + $headerComplete = true; + $psrRequest = \GuzzleHttp\Psr7\parse_request($parts[0] . "\r\n\r\n"); + $negotiatorResponse = $negotiator->handshake($psrRequest); + + $negotiatorResponse = $negotiatorResponse->withAddedHeader("Content-Length", "0"); + + $connection->write(\GuzzleHttp\Psr7\str($negotiatorResponse)); + + if ($negotiatorResponse->getStatusCode() !== 101) { + $connection->end(); + return; + } + + $parser = new \Ratchet\RFC6455\Messaging\MessageBuffer($closeFrameChecker, + function (MessageInterface $message) use ($connection) { + $connection->write($message->getContents()); + }, function (FrameInterface $frame) use ($connection, &$parser) { + switch ($frame->getOpCode()) { + case Frame::OP_CLOSE: + $connection->end($frame->getContents()); + break; + case Frame::OP_PING: + $connection->write($parser->newFrame($frame->getPayload(), true, Frame::OP_PONG)->getContents()); + break; + } + }, true, function () use ($uException) { + return $uException; + }); + + array_shift($parts); + $parser->onData(implode("\r\n\r\n", $parts)); + }); }); -$socket->listen(9001, '0.0.0.0'); $loop->run(); From 8aee2208987e217933bf30e8a2a02279afb1c454 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Tue, 10 Dec 2019 19:23:45 -0500 Subject: [PATCH 02/14] Rework MessageBuffer to better handle large buffers filled with small frames --- src/Messaging/MessageBuffer.php | 56 +++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index 22f247c..a88d713 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -37,6 +37,11 @@ class MessageBuffer { */ private $checkForMask; + /** + * @var string + */ + private $leftovers; + function __construct( CloseFrameChecker $frameChecker, callable $onMessage, @@ -53,12 +58,51 @@ class MessageBuffer { $this->onMessage = $onMessage; $this->onControl = $onControl ?: function() {}; + + $this->leftovers = ''; } public function onData($data) { - while (strlen($data) > 0) { - $data = $this->processData($data); + $data = $this->leftovers . $data; + $dataLen = strlen($data); + $spyFrame = new Frame(); + + if ($dataLen < 2) { + $this->leftovers = $data; + return; } + $currentByte = 0; + $frameStart = 0; + $spyFrame->addBuffer($data[$currentByte]); + $currentByte++; + + while ($currentByte < $dataLen) { + $spyFrame->addBuffer($data[$currentByte]); + $currentByte ++; + try { + $payload_length = $spyFrame->getPayloadLength(); + $payload_start = $spyFrame->getPayloadStartingByte(); + } catch (\UnderflowException $e) { + if ($currentByte < $dataLen) { + continue; + } + break; + } + + $isCoalesced = $dataLen - $frameStart >= $payload_length + $payload_start; + + + if (!$isCoalesced) { + break; + } + $this->processData(substr($data, $frameStart, $payload_length + $payload_start)); + $spyFrame = new Frame(); + $currentByte = $frameStart + $payload_length + $payload_start; + $frameStart = $currentByte; + } + + $this->leftovers = substr($data, $frameStart); + } /** @@ -70,16 +114,12 @@ class MessageBuffer { $this->frameBuffer ?: $this->frameBuffer = $this->newFrame(); $this->frameBuffer->addBuffer($data); - if (!$this->frameBuffer->isCoalesced()) { - return ''; - } $onMessage = $this->onMessage; $onControl = $this->onControl; $this->frameBuffer = $this->frameCheck($this->frameBuffer); - $overflow = $this->frameBuffer->extractOverflow(); $this->frameBuffer->unMaskPayload(); $opcode = $this->frameBuffer->getOpcode(); @@ -108,8 +148,6 @@ class MessageBuffer { $onMessage($msgBuffer); } } - - return $overflow; } /** @@ -230,4 +268,4 @@ class MessageBuffer { public function newCloseFrame($code, $reason = '') { return $this->newFrame(pack('n', $code) . $reason, true, Frame::OP_CLOSE); } -} +} \ No newline at end of file From 58e79971e0abacc7a3d0ba09eaf0948edde0aa14 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Tue, 10 Dec 2019 19:31:01 -0500 Subject: [PATCH 03/14] New PHP versions for travis --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 09125d8..c65dedb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,12 +1,12 @@ language: php php: - - 5.4 - - 5.5 - 5.6 - 7.0 - 7.1 - 7.2 + - 7.3 + - 7.4 before_install: - export PATH=$HOME/.local/bin:$PATH From 11a21b762817c75e165c48fe59788399d24f272b Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Tue, 10 Dec 2019 22:23:00 -0500 Subject: [PATCH 04/14] A little faster by not using Frame functions to test for frame sizes --- src/Messaging/MessageBuffer.php | 42 +++++++++++++++------------------ 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index a88d713..300b330 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -65,44 +65,40 @@ class MessageBuffer { public function onData($data) { $data = $this->leftovers . $data; $dataLen = strlen($data); - $spyFrame = new Frame(); if ($dataLen < 2) { $this->leftovers = $data; return; } - $currentByte = 0; - $frameStart = 0; - $spyFrame->addBuffer($data[$currentByte]); - $currentByte++; - while ($currentByte < $dataLen) { - $spyFrame->addBuffer($data[$currentByte]); - $currentByte ++; - try { - $payload_length = $spyFrame->getPayloadLength(); - $payload_start = $spyFrame->getPayloadStartingByte(); - } catch (\UnderflowException $e) { - if ($currentByte < $dataLen) { - continue; - } + $frameStart = 0; + while ($frameStart + 1 <= $dataLen) { + $headerSize = 2; + $payload_length = unpack('C', $data[$frameStart + 1] & "\x7f")[1]; + $isMasked = ($data[$frameStart + 1] & "\x80") === "\x80"; + $headerSize += $isMasked ? 4 : 0; + if ($payload_length > 125 && ($dataLen - $frameStart < $headerSize + 125)) { + // no point of checking - this frame is going to be bigger than the buffer is right now break; } + if ($payload_length > 125) { + $payloadLenBytes = $payload_length === 126 ? 2 : 8; + $headerSize += $payloadLenBytes; + $bytesToUpack = substr($data, $frameStart + 2, $payloadLenBytes); + $payload_length = $payload_length === 126 + ? unpack('n', $bytesToUpack)[1] + : unpack('J', $bytesToUpack)[1]; + } - $isCoalesced = $dataLen - $frameStart >= $payload_length + $payload_start; - - + $isCoalesced = $dataLen - $frameStart >= $payload_length + $headerSize; if (!$isCoalesced) { break; } - $this->processData(substr($data, $frameStart, $payload_length + $payload_start)); - $spyFrame = new Frame(); - $currentByte = $frameStart + $payload_length + $payload_start; - $frameStart = $currentByte; + $this->processData(substr($data, $frameStart, $payload_length + $headerSize)); + $frameStart = $frameStart + $payload_length + $headerSize; } $this->leftovers = substr($data, $frameStart); - } /** From 9c1df6a8e141edb4bc4bb299cfa8c92a62c95270 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Tue, 10 Dec 2019 23:05:53 -0500 Subject: [PATCH 05/14] Should be framestart + 2 --- src/Messaging/MessageBuffer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index 300b330..6b3b440 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -72,7 +72,7 @@ class MessageBuffer { } $frameStart = 0; - while ($frameStart + 1 <= $dataLen) { + while ($frameStart + 2 <= $dataLen) { $headerSize = 2; $payload_length = unpack('C', $data[$frameStart + 1] & "\x7f")[1]; $isMasked = ($data[$frameStart + 1] & "\x80") === "\x80"; From 91af8a76d57f8271e6f7b0d9fc0bfa1549200105 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Wed, 11 Dec 2019 10:18:08 -0500 Subject: [PATCH 06/14] quality of life changes Kill the php server after AB tests are done Add script runners to composer for ease of use --- composer.json | 8 ++++++++ tests/ab/run_ab_tests.sh | 2 ++ 2 files changed, 10 insertions(+) diff --git a/composer.json b/composer.json index d9ff35d..b758876 100644 --- a/composer.json +++ b/composer.json @@ -27,5 +27,13 @@ "require-dev": { "phpunit/phpunit": "4.8.*", "react/socket": "^1.3" + }, + "scripts": { + "abtests": "sh tests/ab/run_ab_tests.sh", + "phpunit": "phpunit --colors=always", + "test": [ + "@abtests", + "@phpunit" + ] } } diff --git a/tests/ab/run_ab_tests.sh b/tests/ab/run_ab_tests.sh index 8fa9ced..f06a343 100644 --- a/tests/ab/run_ab_tests.sh +++ b/tests/ab/run_ab_tests.sh @@ -9,3 +9,5 @@ sleep 2 php startServer.php & sleep 3 wstest -m fuzzingclient -s fuzzingclient.json +sleep 1 +kill $(ps aux | grep 'php startServer.php' | awk '{print $2}' | head -n 1) From 830e2f561e25970d02b1605ef333a38100875614 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Wed, 11 Dec 2019 13:27:42 -0500 Subject: [PATCH 07/14] Allow limits for maximum payload --- src/Messaging/Message.php | 19 ++- src/Messaging/MessageBuffer.php | 62 ++++++++- tests/unit/Messaging/MessageBufferTest.php | 154 +++++++++++++++++++++ 3 files changed, 223 insertions(+), 12 deletions(-) diff --git a/src/Messaging/Message.php b/src/Messaging/Message.php index 4f3b014..33a6337 100644 --- a/src/Messaging/Message.php +++ b/src/Messaging/Message.php @@ -7,8 +7,14 @@ class Message implements \IteratorAggregate, MessageInterface { */ private $_frames; + /** + * @var int + */ + private $len; + public function __construct() { $this->_frames = new \SplDoublyLinkedList; + $this->len = 0; } public function getIterator() { @@ -39,6 +45,7 @@ class Message implements \IteratorAggregate, MessageInterface { * {@inheritdoc} */ public function addFrame(FrameInterface $fragment) { + $this->len += $fragment->getPayloadLength(); $this->_frames->push($fragment); return $this; @@ -59,17 +66,7 @@ class Message implements \IteratorAggregate, MessageInterface { * {@inheritdoc} */ public function getPayloadLength() { - $len = 0; - - foreach ($this->_frames as $frame) { - try { - $len += $frame->getPayloadLength(); - } catch (\UnderflowException $e) { - // Not an error, want the current amount buffered - } - } - - return $len; + return $this->len; } /** diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index 6b3b440..9540acf 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -42,12 +42,24 @@ class MessageBuffer { */ private $leftovers; + /** + * @var int + */ + private $maxMessagePayloadSize; + + /** + * @var int + */ + private $maxFramePayloadSize; + function __construct( CloseFrameChecker $frameChecker, callable $onMessage, callable $onControl = null, $expectMask = true, - $exceptionFactory = null + $exceptionFactory = null, + $maxMessagePayloadSize = null, // null for default - zero for no limit + $maxFramePayloadSize = null // null for default - zero for no limit ) { $this->closeFrameChecker = $frameChecker; $this->checkForMask = (bool)$expectMask; @@ -60,6 +72,31 @@ class MessageBuffer { $this->onControl = $onControl ?: function() {}; $this->leftovers = ''; + + $memory_limit = \trim(\ini_get('memory_limit')); + $memory_limit_bytes = 0; + if ($memory_limit !== '') { + $shifty = ['k' => 0, 'm' => 10, 'g' => 20]; + $multiplier = strtolower($memory_limit)[-1]; + $memory_limit = (int)$memory_limit; + $memory_limit_bytes = in_array($multiplier, array_keys($shifty), true) ? $memory_limit * 1024 << $shifty[$multiplier] : $memory_limit; + } + if ($maxMessagePayloadSize === null) { + $maxMessagePayloadSize = $memory_limit_bytes / 4; + } + if ($maxFramePayloadSize === null) { + $maxFramePayloadSize = $memory_limit_bytes / 4; + } + + if (!is_int($maxFramePayloadSize) || $maxFramePayloadSize > 0x7FFFFFFFFFFFFFFF || $maxFramePayloadSize < 0) { // this should be interesting on non-64 bit systems + throw New \InvalidArgumentException('maxFramePayloadSize is not valid'); + } + $this->maxFramePayloadSize = $maxFramePayloadSize; + + if (!is_int($maxMessagePayloadSize) || $maxMessagePayloadSize > 0x7FFFFFFFFFFFFFFF || $maxMessagePayloadSize < 0) { + throw New \InvalidArgumentException('maxMessagePayloadSize is not valid'); + } + $this->maxMessagePayloadSize = $maxMessagePayloadSize; } public function onData($data) { @@ -90,6 +127,29 @@ class MessageBuffer { : unpack('J', $bytesToUpack)[1]; } + $closeFrame = null; + + if ($payload_length < 0) { + // this can happen when unpacking in php + $closeFrame = $this->newCloseFrame(Frame::CLOSE_PROTOCOL, 'Invalid frame length'); + } + + if (!$closeFrame && $this->maxFramePayloadSize > 1 && $payload_length > $this->maxFramePayloadSize) { + $closeFrame = $this->newCloseFrame(Frame::CLOSE_TOO_BIG, 'Maximum frame size exceeded'); + } + + if (!$closeFrame && $this->maxMessagePayloadSize > 0 + && $payload_length + ($this->messageBuffer ? $this->messageBuffer->getPayloadLength() : 0) > $this->maxMessagePayloadSize) { + $closeFrame = $this->newCloseFrame(Frame::CLOSE_TOO_BIG, 'Maximum message size exceeded'); + } + + if ($closeFrame !== null) { + $onControl = $this->onControl; + $onControl($closeFrame); + $this->leftovers = ''; + return; + } + $isCoalesced = $dataLen - $frameStart >= $payload_length + $headerSize; if (!$isCoalesced) { break; diff --git a/tests/unit/Messaging/MessageBufferTest.php b/tests/unit/Messaging/MessageBufferTest.php index 567afa2..eefc5dc 100644 --- a/tests/unit/Messaging/MessageBufferTest.php +++ b/tests/unit/Messaging/MessageBufferTest.php @@ -69,4 +69,158 @@ class MessageBufferTest extends \PHPUnit_Framework_TestCase $this->assertTrue($bReceived); } + + public function testInvalidFrameLength() { + $frame = new Frame(str_repeat('a', 200), true, Frame::OP_TEXT); + + $frameRaw = $frame->getContents(); + + $frameRaw[1] = "\x7f"; // 127 in the first spot + + $frameRaw[2] = "\xff"; // this will unpack to -1 + $frameRaw[3] = "\xff"; + $frameRaw[4] = "\xff"; + $frameRaw[5] = "\xff"; + $frameRaw[6] = "\xff"; + $frameRaw[7] = "\xff"; + $frameRaw[8] = "\xff"; + $frameRaw[9] = "\xff"; + + /** @var Frame $controlFrame */ + $controlFrame = null; + $messageCount = 0; + + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) use (&$messageCount) { + $messageCount++; + }, + function (Frame $frame) use (&$controlFrame) { + $this->assertNull($controlFrame); + $controlFrame = $frame; + }, + false, + null, + 0, + 10 + ); + + $messageBuffer->onData($frameRaw); + + $this->assertEquals(0, $messageCount); + $this->assertTrue($controlFrame instanceof Frame); + $this->assertEquals(Frame::OP_CLOSE, $controlFrame->getOpcode()); + $this->assertEquals([Frame::CLOSE_PROTOCOL], array_merge(unpack('n*', substr($controlFrame->getPayload(), 0, 2)))); + + } + + public function testFrameLengthTooBig() { + $frame = new Frame(str_repeat('a', 200), true, Frame::OP_TEXT); + + $frameRaw = $frame->getContents(); + + $frameRaw[1] = "\x7f"; // 127 in the first spot + + $frameRaw[2] = "\x7f"; // this will unpack to -1 + $frameRaw[3] = "\xff"; + $frameRaw[4] = "\xff"; + $frameRaw[5] = "\xff"; + $frameRaw[6] = "\xff"; + $frameRaw[7] = "\xff"; + $frameRaw[8] = "\xff"; + $frameRaw[9] = "\xff"; + + /** @var Frame $controlFrame */ + $controlFrame = null; + $messageCount = 0; + + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) use (&$messageCount) { + $messageCount++; + }, + function (Frame $frame) use (&$controlFrame) { + $this->assertNull($controlFrame); + $controlFrame = $frame; + }, + false, + null, + 0, + 10 + ); + + $messageBuffer->onData($frameRaw); + + $this->assertEquals(0, $messageCount); + $this->assertTrue($controlFrame instanceof Frame); + $this->assertEquals(Frame::OP_CLOSE, $controlFrame->getOpcode()); + $this->assertEquals([Frame::CLOSE_TOO_BIG], array_merge(unpack('n*', substr($controlFrame->getPayload(), 0, 2)))); + } + + public function testFrameLengthBiggerThanMaxMessagePayload() { + $frame = new Frame(str_repeat('a', 200), true, Frame::OP_TEXT); + + $frameRaw = $frame->getContents(); + + /** @var Frame $controlFrame */ + $controlFrame = null; + $messageCount = 0; + + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) use (&$messageCount) { + $messageCount++; + }, + function (Frame $frame) use (&$controlFrame) { + $this->assertNull($controlFrame); + $controlFrame = $frame; + }, + false, + null, + 100, + 0 + ); + + $messageBuffer->onData($frameRaw); + + $this->assertEquals(0, $messageCount); + $this->assertTrue($controlFrame instanceof Frame); + $this->assertEquals(Frame::OP_CLOSE, $controlFrame->getOpcode()); + $this->assertEquals([Frame::CLOSE_TOO_BIG], array_merge(unpack('n*', substr($controlFrame->getPayload(), 0, 2)))); + } + + public function testSecondFrameLengthPushesPastMaxMessagePayload() { + $frame = new Frame(str_repeat('a', 200), false, Frame::OP_TEXT); + $firstFrameRaw = $frame->getContents(); + $frame = new Frame(str_repeat('b', 200), true, Frame::OP_TEXT); + $secondFrameRaw = $frame->getContents(); + + /** @var Frame $controlFrame */ + $controlFrame = null; + $messageCount = 0; + + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) use (&$messageCount) { + $messageCount++; + }, + function (Frame $frame) use (&$controlFrame) { + $this->assertNull($controlFrame); + $controlFrame = $frame; + }, + false, + null, + 300, + 0 + ); + + $messageBuffer->onData($firstFrameRaw); + // only put part of the second frame in to watch it fail fast + $messageBuffer->onData(substr($secondFrameRaw, 0, 150)); + + $this->assertEquals(0, $messageCount); + $this->assertTrue($controlFrame instanceof Frame); + $this->assertEquals(Frame::OP_CLOSE, $controlFrame->getOpcode()); + $this->assertEquals([Frame::CLOSE_TOO_BIG], array_merge(unpack('n*', substr($controlFrame->getPayload(), 0, 2)))); + } } \ No newline at end of file From 921f838255b81eba5ee190fcaf21a2c2ec9e0c1c Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Wed, 11 Dec 2019 14:47:53 -0500 Subject: [PATCH 08/14] Support for unsupported PHP versions --- src/Messaging/MessageBuffer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index 9540acf..51eaad0 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -77,7 +77,7 @@ class MessageBuffer { $memory_limit_bytes = 0; if ($memory_limit !== '') { $shifty = ['k' => 0, 'm' => 10, 'g' => 20]; - $multiplier = strtolower($memory_limit)[-1]; + $multiplier = strlen($memory_limit) > 1 ? substr(strtolower($memory_limit), -1) : ''; $memory_limit = (int)$memory_limit; $memory_limit_bytes = in_array($multiplier, array_keys($shifty), true) ? $memory_limit * 1024 << $shifty[$multiplier] : $memory_limit; } From c004fa7e6452a7f65d931996400a02286f728da7 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Thu, 12 Dec 2019 11:37:11 -0500 Subject: [PATCH 09/14] Code formatting --- src/Messaging/MessageBuffer.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index 51eaad0..1d39517 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -105,6 +105,7 @@ class MessageBuffer { if ($dataLen < 2) { $this->leftovers = $data; + return; } @@ -147,6 +148,7 @@ class MessageBuffer { $onControl = $this->onControl; $onControl($closeFrame); $this->leftovers = ''; + return; } From cfc9049d13e7c37ef2f3f38f49bc32b35f503417 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Thu, 12 Dec 2019 13:59:55 -0500 Subject: [PATCH 10/14] Make memory limit testable --- src/Messaging/MessageBuffer.php | 29 ++++++++---- tests/unit/Messaging/MessageBufferTest.php | 55 ++++++++++++++++++++++ 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index 1d39517..356986f 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -73,14 +73,7 @@ class MessageBuffer { $this->leftovers = ''; - $memory_limit = \trim(\ini_get('memory_limit')); - $memory_limit_bytes = 0; - if ($memory_limit !== '') { - $shifty = ['k' => 0, 'm' => 10, 'g' => 20]; - $multiplier = strlen($memory_limit) > 1 ? substr(strtolower($memory_limit), -1) : ''; - $memory_limit = (int)$memory_limit; - $memory_limit_bytes = in_array($multiplier, array_keys($shifty), true) ? $memory_limit * 1024 << $shifty[$multiplier] : $memory_limit; - } + $memory_limit_bytes = static::getMemoryLimit(); if ($maxMessagePayloadSize === null) { $maxMessagePayloadSize = $memory_limit_bytes / 4; } @@ -326,4 +319,24 @@ class MessageBuffer { public function newCloseFrame($code, $reason = '') { return $this->newFrame(pack('n', $code) . $reason, true, Frame::OP_CLOSE); } + + /** + * This is a separate function for testing purposes + * $memory_limit is only used for testing + * + * @param null|string $memory_limit + * @return int + */ + private static function getMemoryLimit($memory_limit = null) { + $memory_limit = $memory_limit === null ? \trim(\ini_get('memory_limit')) : $memory_limit; + $memory_limit_bytes = 0; + if ($memory_limit !== '') { + $shifty = ['k' => 0, 'm' => 10, 'g' => 20]; + $multiplier = strlen($memory_limit) > 1 ? substr(strtolower($memory_limit), -1) : ''; + $memory_limit = (int)$memory_limit; + $memory_limit_bytes = in_array($multiplier, array_keys($shifty), true) ? $memory_limit * 1024 << $shifty[$multiplier] : $memory_limit; + } + + return $memory_limit_bytes; + } } \ No newline at end of file diff --git a/tests/unit/Messaging/MessageBufferTest.php b/tests/unit/Messaging/MessageBufferTest.php index eefc5dc..261878b 100644 --- a/tests/unit/Messaging/MessageBufferTest.php +++ b/tests/unit/Messaging/MessageBufferTest.php @@ -223,4 +223,59 @@ class MessageBufferTest extends \PHPUnit_Framework_TestCase $this->assertEquals(Frame::OP_CLOSE, $controlFrame->getOpcode()); $this->assertEquals([Frame::CLOSE_TOO_BIG], array_merge(unpack('n*', substr($controlFrame->getPayload(), 0, 2)))); } + + /** + * Some test cases from memory limit inspired by https://github.com/BrandEmbassy/php-memory + * + * Here is the license for that project: + * MIT License + * + * Copyright (c) 2018 Brand Embassy + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + + /** + * @dataProvider phpConfigurationProvider + * + * @param string $phpConfigurationValue + * @param int $expectedLimit + */ + public function testMemoryLimits($phpConfigurationValue, $expectedLimit) { + $method = new \ReflectionMethod('Ratchet\RFC6455\Messaging\MessageBuffer', 'getMemoryLimit'); + $method->setAccessible(true); + $actualLimit = $method->invoke(null, $phpConfigurationValue); + + $this->assertSame($expectedLimit, $actualLimit); + } + + public function phpConfigurationProvider() { + return [ + 'without unit type, just bytes' => ['500', 500], + '1 GB with big "G"' => ['1G', 1 * 1024 * 1024 * 1024], + '128 MB with big "M"' => ['128M', 128 * 1024 * 1024], + '128 MB with small "m"' => ['128m', 128 * 1024 * 1024], + '24 kB with small "k"' => ['24k', 24 * 1024], + '2 GB with small "g"' => ['2g', 2 * 1024 * 1024 * 1024], + 'unlimited memory' => ['-1', -1], + 'invalid float value' => ['2.5M', 2 * 1024 * 1024], + 'empty value' => ['', 0] + ]; + } } \ No newline at end of file From bbdd346c1c47c571142ed9ae0dff40345541b42a Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Sat, 14 Dec 2019 12:52:19 -0500 Subject: [PATCH 11/14] Test resulting settings from different memory_limits + more tests --- src/Messaging/MessageBuffer.php | 3 +- tests/unit/Messaging/MessageBufferTest.php | 88 +++++++++++++++++++++- 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index 356986f..e1d0f6f 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -74,6 +74,7 @@ class MessageBuffer { $this->leftovers = ''; $memory_limit_bytes = static::getMemoryLimit(); + if ($maxMessagePayloadSize === null) { $maxMessagePayloadSize = $memory_limit_bytes / 4; } @@ -337,6 +338,6 @@ class MessageBuffer { $memory_limit_bytes = in_array($multiplier, array_keys($shifty), true) ? $memory_limit * 1024 << $shifty[$multiplier] : $memory_limit; } - return $memory_limit_bytes; + return $memory_limit_bytes < 0 ? 0 : $memory_limit_bytes; } } \ No newline at end of file diff --git a/tests/unit/Messaging/MessageBufferTest.php b/tests/unit/Messaging/MessageBufferTest.php index 261878b..ee36b38 100644 --- a/tests/unit/Messaging/MessageBufferTest.php +++ b/tests/unit/Messaging/MessageBufferTest.php @@ -273,9 +273,93 @@ class MessageBufferTest extends \PHPUnit_Framework_TestCase '128 MB with small "m"' => ['128m', 128 * 1024 * 1024], '24 kB with small "k"' => ['24k', 24 * 1024], '2 GB with small "g"' => ['2g', 2 * 1024 * 1024 * 1024], - 'unlimited memory' => ['-1', -1], + 'unlimited memory' => ['-1', 0], 'invalid float value' => ['2.5M', 2 * 1024 * 1024], - 'empty value' => ['', 0] + 'empty value' => ['', 0], + 'invalid ini setting' => ['whatever it takes', 0] ]; } + + /** + * @expectedException \InvalidArgumentException + */ + public function testInvalidMaxFramePayloadSizes() { + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) {}, + function (Frame $frame) {}, + false, + null, + 0, + 0x8000000000000000 + ); + } + + /** + * @expectedException \InvalidArgumentException + */ + public function testInvalidMaxMessagePayloadSizes() { + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) {}, + function (Frame $frame) {}, + false, + null, + 0x8000000000000000, + 0 + ); + } + + /** + * @dataProvider phpConfigurationProvider + * + * @param string $phpConfigurationValue + * @param int $expectedLimit + * + * @runInSeparateProcess + */ + public function testIniSizes($phpConfigurationValue, $expectedLimit) { + ini_set('memory_limit', $phpConfigurationValue); + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) {}, + function (Frame $frame) {}, + false, + null + ); + + if ($expectedLimit === -1) { + $expectedLimit = 0; + } + + $prop = new \ReflectionProperty($messageBuffer, 'maxMessagePayloadSize'); + $prop->setAccessible(true); + $this->assertEquals($expectedLimit / 4, $prop->getValue($messageBuffer)); + + $prop = new \ReflectionProperty($messageBuffer, 'maxFramePayloadSize'); + $prop->setAccessible(true); + $this->assertEquals($expectedLimit / 4, $prop->getValue($messageBuffer)); + } + + /** + * @runInSeparateProcess + */ + public function testInvalidIniSize() { + ini_set('memory_limit', 'lots of memory'); + $messageBuffer = new MessageBuffer( + new CloseFrameChecker(), + function (Message $message) {}, + function (Frame $frame) {}, + false, + null + ); + + $prop = new \ReflectionProperty($messageBuffer, 'maxMessagePayloadSize'); + $prop->setAccessible(true); + $this->assertEquals(0, $prop->getValue($messageBuffer)); + + $prop = new \ReflectionProperty($messageBuffer, 'maxFramePayloadSize'); + $prop->setAccessible(true); + $this->assertEquals(0, $prop->getValue($messageBuffer)); + } } \ No newline at end of file From 9d6d7d01d3fa8b11d7133f28a0e72d887d487ce1 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Sat, 14 Dec 2019 13:10:40 -0500 Subject: [PATCH 12/14] Don't run ini_set tests in php 5 because it crashes --- tests/unit/Messaging/MessageBufferTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/Messaging/MessageBufferTest.php b/tests/unit/Messaging/MessageBufferTest.php index ee36b38..9d444bc 100644 --- a/tests/unit/Messaging/MessageBufferTest.php +++ b/tests/unit/Messaging/MessageBufferTest.php @@ -317,6 +317,7 @@ class MessageBufferTest extends \PHPUnit_Framework_TestCase * @param int $expectedLimit * * @runInSeparateProcess + * @requires PHP >= 7.0 */ public function testIniSizes($phpConfigurationValue, $expectedLimit) { ini_set('memory_limit', $phpConfigurationValue); @@ -343,6 +344,7 @@ class MessageBufferTest extends \PHPUnit_Framework_TestCase /** * @runInSeparateProcess + * @requires PHP >= 7.0 */ public function testInvalidIniSize() { ini_set('memory_limit', 'lots of memory'); From e7b72440c939a1ba828187b420909e5d99001de4 Mon Sep 17 00:00:00 2001 From: Matt Bonneau Date: Sat, 14 Dec 2019 14:26:11 -0500 Subject: [PATCH 13/14] Fix requires annotation to skip some tests on 5.6 --- tests/unit/Messaging/MessageBufferTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/Messaging/MessageBufferTest.php b/tests/unit/Messaging/MessageBufferTest.php index 9d444bc..b5925ab 100644 --- a/tests/unit/Messaging/MessageBufferTest.php +++ b/tests/unit/Messaging/MessageBufferTest.php @@ -317,7 +317,7 @@ class MessageBufferTest extends \PHPUnit_Framework_TestCase * @param int $expectedLimit * * @runInSeparateProcess - * @requires PHP >= 7.0 + * @requires PHP 7.0 */ public function testIniSizes($phpConfigurationValue, $expectedLimit) { ini_set('memory_limit', $phpConfigurationValue); @@ -344,7 +344,7 @@ class MessageBufferTest extends \PHPUnit_Framework_TestCase /** * @runInSeparateProcess - * @requires PHP >= 7.0 + * @requires PHP 7.0 */ public function testInvalidIniSize() { ini_set('memory_limit', 'lots of memory'); From e861242e1d3bf3bfccaac7f647bb955f95c3afd7 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 15 Dec 2019 05:17:35 -0500 Subject: [PATCH 14/14] Provide value in error for clarity --- src/Messaging/MessageBuffer.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Messaging/MessageBuffer.php b/src/Messaging/MessageBuffer.php index e1d0f6f..9b1a23b 100644 --- a/src/Messaging/MessageBuffer.php +++ b/src/Messaging/MessageBuffer.php @@ -83,12 +83,12 @@ class MessageBuffer { } if (!is_int($maxFramePayloadSize) || $maxFramePayloadSize > 0x7FFFFFFFFFFFFFFF || $maxFramePayloadSize < 0) { // this should be interesting on non-64 bit systems - throw New \InvalidArgumentException('maxFramePayloadSize is not valid'); + throw new \InvalidArgumentException($maxFramePayloadSize . ' is not a valid maxFramePayloadSize'); } $this->maxFramePayloadSize = $maxFramePayloadSize; if (!is_int($maxMessagePayloadSize) || $maxMessagePayloadSize > 0x7FFFFFFFFFFFFFFF || $maxMessagePayloadSize < 0) { - throw New \InvalidArgumentException('maxMessagePayloadSize is not valid'); + throw new \InvalidArgumentException($maxMessagePayloadSize . 'is not a valid maxMessagePayloadSize'); } $this->maxMessagePayloadSize = $maxMessagePayloadSize; } @@ -340,4 +340,4 @@ class MessageBuffer { return $memory_limit_bytes < 0 ? 0 : $memory_limit_bytes; } -} \ No newline at end of file +}