diff --git a/src/Connection.php b/src/Connection.php deleted file mode 100644 index fd404f3..0000000 --- a/src/Connection.php +++ /dev/null @@ -1,45 +0,0 @@ -WebSocket->closing) { - if (!($msg instanceof DataInterface)) { - $msg = new Frame($msg); - } - - $this->getConnection()->send($msg->getContents()); - } - - return $this; - } - - /** - * {@inheritdoc} - */ - public function close($code = 1000) { - if ($this->WebSocket->closing) { - return; - } - - if ($code instanceof DataInterface) { - $this->send($code); - } else { - $this->send(new Frame(pack('n', $code), true, Frame::OP_CLOSE)); - } - - $this->getConnection()->close(); - - $this->WebSocket->closing = true; - } -} diff --git a/src/Messaging/Protocol/Message.php b/src/Messaging/Protocol/Message.php index be87123..2ac28a7 100644 --- a/src/Messaging/Protocol/Message.php +++ b/src/Messaging/Protocol/Message.php @@ -7,9 +7,6 @@ class Message implements \IteratorAggregate, MessageInterface { */ private $_frames; - /** @var bool */ - private $binary = false; - public function __construct() { $this->_frames = new \SplDoublyLinkedList; } @@ -56,30 +53,9 @@ class Message implements \IteratorAggregate, MessageInterface { /** * {@inheritdoc} - * @todo Also, I should perhaps check the type...control frames (ping/pong/close) are not to be considered part of a message */ 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 true; - //return $this; } /** @@ -147,8 +123,11 @@ class Message implements \IteratorAggregate, MessageInterface { /** * @return boolean */ - public function isBinary() - { - return $this->binary; + public function isBinary() { + if ($this->_frames->isEmpty()) { + throw new \UnderflowException('Not enough data has been received to determine if message is binary'); + } + + return Frame::OP_BINARY === $this->_frames->bottom()->getOpcode(); } } diff --git a/src/Messaging/Streaming/ContextInterface.php b/src/Messaging/Streaming/ContextInterface.php index 5f29ff8..0bd101f 100644 --- a/src/Messaging/Streaming/ContextInterface.php +++ b/src/Messaging/Streaming/ContextInterface.php @@ -4,6 +4,10 @@ use Ratchet\RFC6455\Messaging\Protocol\MessageInterface; use Ratchet\RFC6455\Messaging\Protocol\FrameInterface; interface ContextInterface { + /** + * @param FrameInterface $frame + * @return FrameInterface + */ public function setFrame(FrameInterface $frame = null); /** @@ -11,6 +15,10 @@ interface ContextInterface { */ public function getFrame(); + /** + * @param MessageInterface $message + * @return MessageInterface + */ public function setMessage(MessageInterface $message = null); /** diff --git a/src/Messaging/Streaming/MessageStreamer.php b/src/Messaging/Streaming/MessageStreamer.php index d45fe5a..4c541e0 100644 --- a/src/Messaging/Streaming/MessageStreamer.php +++ b/src/Messaging/Streaming/MessageStreamer.php @@ -6,7 +6,9 @@ use Ratchet\RFC6455\Messaging\Protocol\Message; use Ratchet\RFC6455\Messaging\Validation\MessageValidator; class MessageStreamer { - /** @var MessageValidator */ + /** + * @var MessageValidator + */ private $validator; function __construct(ValidatorInterface $encodingValidator, $expectMask = false) { @@ -17,27 +19,27 @@ class MessageStreamer { public function onData($data, ContextInterface $context) { $overflow = ''; - $context->getMessage() || $context->setMessage($this->newMessage()); - $context->getFrame() || $context->setFrame($this->newFrame()); - - $frame = $context->getFrame(); + $message = $context->getMessage() ?: $context->setMessage($this->newMessage()); + $frame = $context->getFrame() ?: $context->setFrame($this->newFrame()); $frame->addBuffer($data); if ($frame->isCoalesced()) { - $validFrame = $this->validator->validateFrame($frame); - if (true !== $validFrame) { - $context->onClose($validFrame); + $frameCount = $message->count(); + $prevFrame = $frameCount > 0 ? $message[$frameCount - 1] : null; - return; + $frameStatus = $this->validator->validateFrame($frame, $prevFrame); + + if (0 !== $frameStatus) { + return $context->onClose($frameStatus); } $opcode = $frame->getOpcode(); if ($opcode > 2) { switch ($opcode) { - case $frame::OP_PING: + case Frame::OP_PING: $context->onPing($frame); break; - case $frame::OP_PONG: + case Frame::OP_PONG: $context->onPong($frame); break; } @@ -54,20 +56,18 @@ class MessageStreamer { $overflow = $frame->extractOverflow(); - $frameAdded = $context->getMessage()->addFrame($frame); - if (true !== $frameAdded) { - $context->onClose($frameAdded); - } + $frame->unMaskPayload(); + $message->addFrame($frame); $context->setFrame(null); } - if ($context->getMessage()->isCoalesced()) { - $msgCheck = $this->validator->checkMessage($context->getMessage()); - if ($msgCheck !== true) { - $context->onClose($msgCheck); - return; + if ($message->isCoalesced()) { + $msgCheck = $this->validator->checkMessage($message); + if (true !== $msgCheck) { + return $context->onClose($msgCheck); } - $context->onMessage($context->getMessage()); + + $context->onMessage($message); $context->setMessage(null); } diff --git a/src/Messaging/Validation/MessageValidator.php b/src/Messaging/Validation/MessageValidator.php index b67eb34..de51a9c 100644 --- a/src/Messaging/Validation/MessageValidator.php +++ b/src/Messaging/Validation/MessageValidator.php @@ -18,31 +18,11 @@ class MessageValidator { /** * Determine if a message is valid * @param \Ratchet\RFC6455\Messaging\Protocol\MessageInterface - * @return bool|int true if valid - false if incomplete - int of recomended close code + * @return bool|int true if valid - false if incomplete - int of recommended close code */ public function checkMessage(MessageInterface $message) { - // Need a progressive and complete check...this is only satisfying complete - if (!$message->isCoalesced()) { - return false; - } - $frame = $message[0]; - $frameCheck = $this->validateFrame($frame); - if (true !== $frameCheck) { - return $frameCheck; - } - - // This seems incorrect - how could a frame exist with message count being 0? - if ($frame::OP_CONTINUE === $frame->getOpcode() && 0 === count($message)) { - 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; -// } - if (!$message->isBinary()) { $parsed = $message->getPayload(); if (!$this->validator->checkEncoding($parsed, 'UTF-8')) { @@ -54,41 +34,42 @@ class MessageValidator { } /** - * @param Frame $frame - * @return bool|int Return true if everything is good, an integer close code if not + * @param FrameInterface $frame + * @param FrameInterface $previousFrame + * @return int Return 0 if everything is good, an integer close code if not */ - public function validateFrame(Frame $frame) { + public function validateFrame(FrameInterface $frame, FrameInterface $previousFrame = null) { if (false !== $frame->getRsv1() || false !== $frame->getRsv2() || false !== $frame->getRsv3() ) { - return $frame::CLOSE_PROTOCOL; + return Frame::CLOSE_PROTOCOL; } // Should be checking all frames if ($this->checkForMask && !$frame->isMasked()) { - return $frame::CLOSE_PROTOCOL; + return Frame::CLOSE_PROTOCOL; } $opcode = $frame->getOpcode(); if ($opcode > 2) { if ($frame->getPayloadLength() > 125 || !$frame->isFinal()) { - return $frame::CLOSE_PROTOCOL; + return Frame::CLOSE_PROTOCOL; } switch ($opcode) { - case $frame::OP_CLOSE: + case Frame::OP_CLOSE: $closeCode = 0; $bin = $frame->getPayload(); if (empty($bin)) { - return $frame::CLOSE_NORMAL; + return Frame::CLOSE_NORMAL; } if (strlen($bin) == 1) { - return $frame::CLOSE_PROTOCOL; + return Frame::CLOSE_PROTOCOL; } if (strlen($bin) >= 2) { @@ -96,24 +77,34 @@ class MessageValidator { } if (!$frame->isValidCloseCode($closeCode)) { - return $frame::CLOSE_PROTOCOL; + return Frame::CLOSE_PROTOCOL; } if (!$this->validator->checkEncoding(substr($bin, 2), 'UTF-8')) { - return $frame::CLOSE_BAD_PAYLOAD; + return Frame::CLOSE_BAD_PAYLOAD; } - return $frame::CLOSE_NORMAL; + return Frame::CLOSE_NORMAL; break; - case $frame::OP_PING: - case $frame::OP_PONG: + case Frame::OP_PING: + case Frame::OP_PONG: break; default: - return $frame::CLOSE_PROTOCOL; + return Frame::CLOSE_PROTOCOL; break; } + + return 0; } - return true; + if (Frame::OP_CONTINUE === $frame->getOpcode() && null === $previousFrame) { + return Frame::CLOSE_PROTOCOL; + } + + if (null !== $previousFrame && Frame::OP_CONTINUE != $frame->getOpcode()) { + return Frame::CLOSE_PROTOCOL; + } + + return 0; } } diff --git a/tests/ab/startServer.php b/tests/ab/startServer.php index 0d8ef88..3cdde1e 100644 --- a/tests/ab/startServer.php +++ b/tests/ab/startServer.php @@ -18,6 +18,8 @@ class ConnectionContext implements Ratchet\RFC6455\Messaging\Streaming\ContextIn public function setFrame(\Ratchet\RFC6455\Messaging\Protocol\FrameInterface $frame = null) { $this->_frame = $frame; + + return $frame; } public function getFrame() { @@ -26,6 +28,8 @@ class ConnectionContext implements Ratchet\RFC6455\Messaging\Streaming\ContextIn public function setMessage(\Ratchet\RFC6455\Messaging\Protocol\MessageInterface $message = null) { $this->_message = $message; + + return $message; } public function getMessage() { @@ -33,10 +37,6 @@ class ConnectionContext implements Ratchet\RFC6455\Messaging\Streaming\ContextIn } public function onMessage(\Ratchet\RFC6455\Messaging\Protocol\MessageInterface $msg) { - foreach ($msg as $frame) { - $frame->unMaskPayload(); - } - $this->_conn->write($msg->getContents()); }