diff --git a/src/Messaging/Protocol/Frame.php b/src/Messaging/Protocol/Frame.php index 89ba2b9..13e8bfb 100644 --- a/src/Messaging/Protocol/Frame.php +++ b/src/Messaging/Protocol/Frame.php @@ -61,7 +61,6 @@ class Frame implements FrameInterface { */ protected $secondByte = -1; - /** * @param string|null $payload * @param bool $final @@ -446,4 +445,34 @@ class Frame implements FrameInterface { return ''; } + + /** + * Determine if a close code is valid + * @param int|string + * @return bool + */ + public function isValidCloseCode($val) { + if (in_array($val, [ + static::CLOSE_NORMAL, + static::CLOSE_GOING_AWAY, + static::CLOSE_PROTOCOL, + static::CLOSE_BAD_DATA, + //static::CLOSE_NO_STATUS, + //static::CLOSE_ABNORMAL, + static::CLOSE_BAD_PAYLOAD, + static::CLOSE_POLICY, + static::CLOSE_TOO_BIG, + static::CLOSE_MAND_EXT, + static::CLOSE_SRV_ERR, + //static::CLOSE_TLS, + ])) { + return true; + } + + if ($val >= 3000 && $val <= 4999) { + return true; + } + + return false; + } } diff --git a/src/Messaging/Protocol/Message.php b/src/Messaging/Protocol/Message.php index 1ff8fd6..7d78c71 100644 --- a/src/Messaging/Protocol/Message.php +++ b/src/Messaging/Protocol/Message.php @@ -53,15 +53,29 @@ class Message implements MessageInterface { /** * {@inheritdoc} * @todo Also, I should perhaps check the type...control frames (ping/pong/close) are not to be considered part of a message - * @todo What should we do if there are binary and text mixed together? */ public function addFrame(FrameInterface $fragment) { + // should the validation stuff be somewhere else? + // it really needs the context of the message to know whether there is a problem if ($this->_frames->isEmpty()) { $this->binary = $fragment->getOpcode() == Frame::OP_BINARY; } + + // check to see if this is a continuation frame when there is no + // frames yet added + if ($this->_frames->count() == 0 && $fragment->getOpcode() == Frame::OP_CONTINUE) { + return Frame::CLOSE_PROTOCOL; + } + + // check to see if this is not a continuation frame when there is already frames + if ($this->_frames->count() > 0 && $fragment->getOpcode() != Frame::OP_CONTINUE) { + return Frame::CLOSE_PROTOCOL; + } + $this->_frames->push($fragment); - return $this; + return true; + //return $this; } /** @@ -106,6 +120,8 @@ class Message implements MessageInterface { $buffer .= $frame->getPayload(); } + echo "Reassembled " . strlen($buffer) . " bytes in " . $this->_frames->count() . " frames\n"; + return $buffer; } diff --git a/src/Messaging/Protocol/MessageInterface.php b/src/Messaging/Protocol/MessageInterface.php index a103145..f3d8a64 100644 --- a/src/Messaging/Protocol/MessageInterface.php +++ b/src/Messaging/Protocol/MessageInterface.php @@ -12,4 +12,9 @@ interface MessageInterface extends DataInterface, \ArrayAccess, \Countable { * @return int */ function getOpcode(); + + /** + * @return bool + */ + function isBinary(); } diff --git a/src/Messaging/Streaming/MessageStreamer.php b/src/Messaging/Streaming/MessageStreamer.php index 9f13957..5c38e8b 100644 --- a/src/Messaging/Streaming/MessageStreamer.php +++ b/src/Messaging/Streaming/MessageStreamer.php @@ -4,8 +4,10 @@ namespace Ratchet\RFC6455\Messaging\Streaming; use Evenement\EventEmitterInterface; use Evenement\EventEmitterTrait; +use Ratchet\RFC6455\Encoding\Validator; use Ratchet\RFC6455\Messaging\Protocol\Frame; use Ratchet\RFC6455\Messaging\Protocol\Message; +use Ratchet\RFC6455\Messaging\Validation\MessageValidator; class MessageStreamer implements EventEmitterInterface { use EventEmitterTrait; @@ -16,12 +18,16 @@ class MessageStreamer implements EventEmitterInterface { /** @var Message */ private $currentMessage; - /** @var array */ - private $closeCodes = []; + /** @var MessageValidator */ + private $validator; - function __construct() + /** @var bool */ + private $checkForMask; + + function __construct($client = false) { - $this->setCloseCodes(); + $this->checkForMask = !$client; + $this->validator = new MessageValidator(new Validator(), $this->checkForMask); } @@ -41,6 +47,12 @@ class MessageStreamer implements EventEmitterInterface { $frame->addBuffer($data); if ($frame->isCoalesced()) { + $validFrame = $this->validator->validateFrame($frame); + if ($validFrame !== true) { + $this->emit('close', [$validFrame]); + return; + } + $opcode = $frame->getOpcode(); if ($opcode > 2) { if ($frame->getPayloadLength() > 125) { @@ -63,7 +75,7 @@ class MessageStreamer implements EventEmitterInterface { list($closeCode) = array_merge(unpack('n*', substr($bin, 0, 2))); } - if (!$this->isValidCloseCode($closeCode)) { + if (!$frame->isValidCloseCode($closeCode)) { $this->emit('close', [$frame::CLOSE_PROTOCOL]); return; } @@ -104,11 +116,20 @@ class MessageStreamer implements EventEmitterInterface { $overflow = $frame->extractOverflow(); - $this->currentMessage->addFrame($this->currentFrame); + $frameAdded = $this->currentMessage->addFrame($this->currentFrame); + if ($frameAdded !== true) { + $this->emit('close', [$frameAdded]); + } unset($this->currentFrame); } if ($this->currentMessage->isCoalesced()) { + $msgCheck = $this->validator->checkMessage($this->currentMessage); + if ($msgCheck !== true) { + if ($msgCheck === false) $msgCheck = null; + $this->emit('close', [$msgCheck]); + return; + } $this->emit('message', [$this->currentMessage]); //$parsed = $from->WebSocket->message->getPayload(); unset($this->currentMessage); @@ -135,39 +156,4 @@ class MessageStreamer implements EventEmitterInterface { public function newFrame($payload = null, $final = null, $opcode = null) { return new Frame($payload, $final, $opcode); } - - /** - * Determine if a close code is valid - * @param int|string - * @return bool - */ - public function isValidCloseCode($val) { - if (array_key_exists($val, $this->closeCodes)) { - return true; - } - - if ($val >= 3000 && $val <= 4999) { - return true; - } - - return false; - } - - /** - * Creates a private lookup of valid, private close codes - */ - protected function setCloseCodes() { - $this->closeCodes[Frame::CLOSE_NORMAL] = true; - $this->closeCodes[Frame::CLOSE_GOING_AWAY] = true; - $this->closeCodes[Frame::CLOSE_PROTOCOL] = true; - $this->closeCodes[Frame::CLOSE_BAD_DATA] = true; - //$this->closeCodes[Frame::CLOSE_NO_STATUS] = true; - //$this->closeCodes[Frame::CLOSE_ABNORMAL] = true; - $this->closeCodes[Frame::CLOSE_BAD_PAYLOAD] = true; - $this->closeCodes[Frame::CLOSE_POLICY] = true; - $this->closeCodes[Frame::CLOSE_TOO_BIG] = true; - $this->closeCodes[Frame::CLOSE_MAND_EXT] = true; - $this->closeCodes[Frame::CLOSE_SRV_ERR] = true; - //$this->closeCodes[Frame::CLOSE_TLS] = true; - } -} \ No newline at end of file +} \ No newline at end of file diff --git a/src/Messaging/Validation/MessageValidator.php b/src/Messaging/Validation/MessageValidator.php index b15b726..e47eb2c 100644 --- a/src/Messaging/Validation/MessageValidator.php +++ b/src/Messaging/Validation/MessageValidator.php @@ -1,15 +1,18 @@ validator = $validator; + $this->checkForMask = $checkForMask; } /** @@ -25,7 +28,7 @@ class MessageValidator { $frame = $message[0]; - $frameCheck = $this->checkFrame($frame); + $frameCheck = $this->validateFrame($frame); if (true !== $frameCheck) { return $frameCheck; } @@ -35,19 +38,22 @@ class MessageValidator { return $frame::CLOSE_PROTOCOL; } - if (count($message) > 0 && $frame::OP_CONTINUE !== $frame->getOpcode()) { - return $frame::CLOSE_PROTOCOL; - } + // I (mbonneau) don't understand this - seems to always kill the tests +// if (count($message) > 0 && $frame::OP_CONTINUE !== $frame->getOpcode()) { +// return $frame::CLOSE_PROTOCOL; +// } - $parsed = $message->getPayload(); - if (!$this->validator->checkEncoding($parsed, 'UTF-8')) { - return $frame::CLOSE_BAD_PAYLOAD; + if (!$message->isBinary()) { + $parsed = $message->getPayload(); + if (!$this->validator->checkEncoding($parsed, 'UTF-8')) { + return $frame::CLOSE_BAD_PAYLOAD; + } } return true; } - public function validateFrame(FrameInterface $frame) { + public function validateFrame(Frame $frame) { if (false !== $frame->getRsv1() || false !== $frame->getRsv2() || false !== $frame->getRsv3() @@ -73,15 +79,20 @@ class MessageValidator { $bin = $frame->getPayload(); + if (empty($bin)) { return $frame::CLOSE_NORMAL; } + if (strlen($bin) == 1) { + return $frame::CLOSE_PROTOCOL; + } + if (strlen($bin) >= 2) { list($closeCode) = array_merge(unpack('n*', substr($bin, 0, 2))); } - if (!$this->isValidCloseCode($closeCode)) { + if (!$frame->isValidCloseCode($closeCode)) { return $frame::CLOSE_PROTOCOL; } diff --git a/tests/ab/clientRunner.php b/tests/ab/clientRunner.php index ade81d0..e1621e2 100644 --- a/tests/ab/clientRunner.php +++ b/tests/ab/clientRunner.php @@ -26,7 +26,7 @@ function getTestCases() { $rawResponse = ""; $response = null; - $ms = new \Ratchet\RFC6455\Messaging\Streaming\MessageStreamer(); + $ms = new \Ratchet\RFC6455\Messaging\Streaming\MessageStreamer(true); $ms->on('message', function (Message $msg) use ($stream, $deferred) { $deferred->resolve($msg->getPayload()); @@ -38,7 +38,7 @@ function getTestCases() { $ms->on('close', function ($code) use ($stream) { if ($code === null) { - $stream->close(); + $stream->end(); return; } $frame = new Frame(pack('n', $code), true, Frame::OP_CLOSE); @@ -82,16 +82,17 @@ function runTest($case) $deferred = new Deferred(); - $factory->create('127.0.0.1', 9001)->then(function (\React\Stream\Stream $stream) use ($deferred, $casePath) { + $factory->create('127.0.0.1', 9001)->then(function (\React\Stream\Stream $stream) use ($deferred, $casePath, $case) { $cn = new \Ratchet\RFC6455\Handshake\ClientNegotiator($casePath); $cnRequest = $cn->getRequest(); $rawResponse = ""; $response = null; - $ms = new \Ratchet\RFC6455\Messaging\Streaming\MessageStreamer(); + $ms = new \Ratchet\RFC6455\Messaging\Streaming\MessageStreamer(true); - $ms->on('message', function (Message $msg) use ($stream, $deferred) { + $ms->on('message', function (Message $msg) use ($stream, $deferred, $case) { + echo "Got message on case " . $case . "\n"; $opcode = $msg->isBinary() ? Frame::OP_BINARY : Frame::OP_TEXT; $frame = new Frame($msg->getPayload(), true, $opcode); $frame->maskPayload(); @@ -107,7 +108,7 @@ function runTest($case) $ms->on('close', function ($code) use ($stream, $deferred) { if ($code === null) { - $stream->close(); + $stream->end(); return; } $frame = new Frame(pack('n', $code), true, Frame::OP_CLOSE); @@ -161,7 +162,7 @@ function createReport() { $rawResponse = ""; $response = null; - $ms = new \Ratchet\RFC6455\Messaging\Streaming\MessageStreamer(); + $ms = new \Ratchet\RFC6455\Messaging\Streaming\MessageStreamer(true); $ms->on('message', function (Message $msg) use ($stream, $deferred) { $deferred->resolve($msg->getPayload()); @@ -173,7 +174,7 @@ function createReport() { $ms->on('close', function ($code) use ($stream) { if ($code === null) { - $stream->close(); + $stream->end(); return; } $frame = new Frame(pack('n', $code), true, Frame::OP_CLOSE); @@ -212,14 +213,26 @@ function createReport() { $testPromises = []; -getTestCases()->then(function ($count) { +getTestCases()->then(function ($count) use ($loop) { echo "Running " . $count . " test cases.\n"; - for ($i = 0; $i < $count; $i++) { - $testPromises[] = runTest($i + 1); - } + $allDeferred = new Deferred(); - \React\Promise\all($testPromises)->then(function () { + $runNextCase = function () use (&$i, &$runNextCase, $count, $allDeferred) { + $i++; + if ($i > $count) { + $allDeferred->resolve(); + return; + } + echo "Running " . $i . "\n"; + runTest($i)->then($runNextCase); + }; + + $i = 0; + $runNextCase(); + + $allDeferred->promise()->then(function () { + echo "Generating report...\n"; createReport(); }); }); diff --git a/tests/ab/fuzzingclient.json b/tests/ab/fuzzingclient.json index f2fea0a..20e1bd2 100644 --- a/tests/ab/fuzzingclient.json +++ b/tests/ab/fuzzingclient.json @@ -8,8 +8,7 @@ "options": {"version": 18}} ], - "casesy": ["1.1.1", "1.1.2"], - "cases": ["1.*", "2.*", "3.*", "4.*","5.*"], - "exclude-cases": [], + "cases": ["1.*"], + "exclude-cases": ["12.*","13.*"], "exclude-agent-cases": {} } diff --git a/tests/ab/fuzzingserver.json b/tests/ab/fuzzingserver.json index 4193e62..8041bf1 100644 --- a/tests/ab/fuzzingserver.json +++ b/tests/ab/fuzzingserver.json @@ -4,9 +4,10 @@ "failByDrop": false } , "outdir": "./reports/clients" - , "casesy": ["*"] - , "cases": ["1.*", "2.*", "3.*", "4.*", "5.*"] - , "casesx": ["1.1.8"] - , "exclude-cases": ["9.8.6"] + , "casesa": ["*"] + , "casesj": ["9.3.2", "9.3.3", "9.3.4", "9.4.2", "9.4.3", "9.4.4", "9.7.6", "9.8.6"] + , "cases": ["9.3.1", "9.3.2"] + , "casesx": ["1.*","2.*"] + , "exclude-cases": ["12.*", "13.*"] , "exclude-agent-cases": {} } \ No newline at end of file