Refactoring

onMessage delivers unMaked payload
msg validation moved from Message to MessageValidation
Unify return types
Context return should be input
Remove deprecated Connection

src/Connection.php

@@ -1,45 +0,0 @@
-<?php
-namespace Ratchet\RFC6455\Version\RFC6455;
-use Ratchet\AbstractConnectionDecorator;
-use Ratchet\RFC6455\Version\DataInterface;
-
-/**
- * @deprecated
- * {@inheritdoc}
- * @property \StdClass $WebSocket
- */
-class Connection extends AbstractConnectionDecorator {
-    /**
-     * {@inheritdoc}
-     */
-    public function send($msg) {
-        if (!$this->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;
-    }
-}

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()
+    public function isBinary() {
-    {
+        if ($this->_frames->isEmpty()) {
-        return $this->binary;
+            throw new \UnderflowException('Not enough data has been received to determine if message is binary');
+        }
+
+        return Frame::OP_BINARY === $this->_frames->bottom()->getOpcode();
     }
 }

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);
 
     /**

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());
+        $message = $context->getMessage() ?: $context->setMessage($this->newMessage());
-        $context->getFrame() || $context->setFrame($this->newFrame());
+        $frame   = $context->getFrame()   ?: $context->setFrame($this->newFrame());
-
-        $frame = $context->getFrame();
 
         $frame->addBuffer($data);
         if ($frame->isCoalesced()) {
-            $validFrame = $this->validator->validateFrame($frame);
+            $frameCount = $message->count();
-            if (true !== $validFrame) {
+            $prevFrame  = $frameCount  > 0 ? $message[$frameCount - 1] : null;
-                $context->onClose($validFrame);
 
-                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);
+            $frame->unMaskPayload();
-            if (true !== $frameAdded) {
+            $message->addFrame($frame);
-                $context->onClose($frameAdded);
-            }
             $context->setFrame(null);
         }
 
-        if ($context->getMessage()->isCoalesced()) {
+        if ($message->isCoalesced()) {
-            $msgCheck = $this->validator->checkMessage($context->getMessage());
+            $msgCheck = $this->validator->checkMessage($message);
-            if ($msgCheck !== true) {
+            if (true !== $msgCheck) {
-                $context->onClose($msgCheck);
+                return $context->onClose($msgCheck);
-                return;
             }
-            $context->onMessage($context->getMessage());
+
+            $context->onMessage($message);
             $context->setMessage(null);
         }
 

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
+     * @param FrameInterface $frame
-     * @return bool|int Return true if everything is good, an integer close code if not
+     * @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_PING:
-                case $frame::OP_PONG:
+                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;
     }
 }

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());
     }