From 439ac1234f0c539c310a5f79f65efb66bbcae9d5 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Thu, 14 Jun 2012 11:24:18 -0400 Subject: [PATCH] [WebSocket] Cleanup Removed some obsolete code Handshakes always returns a response --- src/Ratchet/WebSocket/HttpRequestParser.php | 7 ++--- src/Ratchet/WebSocket/Version/RFC6455.php | 15 ++++------ .../WebSocket/Version/RFC6455/Connection.php | 5 ---- src/Ratchet/WebSocket/WsServer.php | 29 ++++++++++++------- .../Tests/WebSocket/Version/RFC6455Test.php | 10 ++++--- 5 files changed, 32 insertions(+), 34 deletions(-) diff --git a/src/Ratchet/WebSocket/HttpRequestParser.php b/src/Ratchet/WebSocket/HttpRequestParser.php index 73bbcf7..f8159df 100644 --- a/src/Ratchet/WebSocket/HttpRequestParser.php +++ b/src/Ratchet/WebSocket/HttpRequestParser.php @@ -5,7 +5,6 @@ use Ratchet\ConnectionInterface; use Ratchet\WebSocket\Guzzle\Http\Message\RequestFactory; use Ratchet\WebSocket\Version\VersionInterface; use Guzzle\Http\Message\RequestInterface; -use Guzzle\Http\Message\Response; class HttpRequestParser implements MessageInterface { const EOM = "\r\n\r\n"; @@ -18,9 +17,9 @@ class HttpRequestParser implements MessageInterface { public $maxSize = 4096; /** - * @param StdClass + * @param Ratchet\ConnectionInterface * @param string Data stream to buffer - * @return Guzzle\Http\Message\Response|null Response object if it's done parsing, null if there's more to be buffered + * @return Guzzle\Http\Message\RequestInterface|null * @throws OverflowException */ public function onMessage(ConnectionInterface $context, $data) { @@ -32,8 +31,6 @@ class HttpRequestParser implements MessageInterface { if (strlen($context->httpBuffer) > (int)$this->maxSize) { throw new \OverflowException("Maximum buffer size of {$this->maxSize} exceeded parsing HTTP header"); - - //return new Response(413, array('X-Powered-By' => \Ratchet\VERSION)); } if ($this->isEom($context->httpBuffer)) { diff --git a/src/Ratchet/WebSocket/Version/RFC6455.php b/src/Ratchet/WebSocket/Version/RFC6455.php index 0d04513..21841cd 100644 --- a/src/Ratchet/WebSocket/Version/RFC6455.php +++ b/src/Ratchet/WebSocket/Version/RFC6455.php @@ -43,23 +43,17 @@ class RFC6455 implements VersionInterface { /** * {@inheritdoc} - * @todo Decide what to do on failure...currently throwing an exception and I think socket connection is closed. Should be sending 40x error - but from where? */ public function handshake(RequestInterface $request) { if (true !== $this->_verifier->verifyAll($request)) { - // new header with 4xx error message - - throw new \InvalidArgumentException('Invalid HTTP header'); + return new Response(400); } - $headers = array( + return new Response(101, array( 'Upgrade' => 'websocket' , 'Connection' => 'Upgrade' , 'Sec-WebSocket-Accept' => $this->sign($request->getHeader('Sec-WebSocket-Key')) - , 'X-Powered-By' => \Ratchet\VERSION - ); - - return new Response(101, $headers); + )); } /** @@ -112,8 +106,11 @@ class RFC6455 implements VersionInterface { if ($opcode > 2) { switch ($opcode) { case $frame::OP_CLOSE: + $from->close($frame->getPayload()); +/* $from->send($frame->unMaskPayload()); $from->getConnection()->close(); +*/ // $from->send(Frame::create(Frame::CLOSE_NORMAL, true, Frame::OP_CLOSE)); return; diff --git a/src/Ratchet/WebSocket/Version/RFC6455/Connection.php b/src/Ratchet/WebSocket/Version/RFC6455/Connection.php index badd488..1e1110b 100644 --- a/src/Ratchet/WebSocket/Version/RFC6455/Connection.php +++ b/src/Ratchet/WebSocket/Version/RFC6455/Connection.php @@ -2,17 +2,12 @@ namespace Ratchet\WebSocket\Version\RFC6455; use Ratchet\ConnectionInterface; use Ratchet\AbstractConnectionDecorator; -use Ratchet\WebSocket\Version\VersionInterface; use Ratchet\WebSocket\Version\FrameInterface; /** * {@inheritdoc} */ class Connection extends AbstractConnectionDecorator { - public function __construct(ConnectionInterface $conn) { - parent::__construct($conn); - } - public function send($msg) { if (!($msg instanceof FrameInterface)) { $msg = new Frame($msg); diff --git a/src/Ratchet/WebSocket/WsServer.php b/src/Ratchet/WebSocket/WsServer.php index dc388d1..4108b76 100644 --- a/src/Ratchet/WebSocket/WsServer.php +++ b/src/Ratchet/WebSocket/WsServer.php @@ -81,24 +81,21 @@ class WsServer implements MessageComponentInterface { */ public function onMessage(ConnectionInterface $from, $msg) { if (true !== $from->WebSocket->established) { - if (null === ($request = $this->reqParser->onMessage($from, $msg))) { - return; + try { + if (null === ($request = $this->reqParser->onMessage($from, $msg))) { + return; + } + } catch (\OverflowException $oe) { + return $this->close($from, 413); } if (!$this->versioner->isVersionEnabled($request)) { - $response = new Response(400, array( - 'Sec-WebSocket-Version' => $this->versioner->getSupportedVersionString() - , 'X-Powered-By' => \Ratchet\VERSION - )); - - $from->send((string)$response); - $from->close(); - - return; + return $this->close($from); } $from->WebSocket->version = $this->versioner->getVersion($request); $response = $from->WebSocket->version->handshake($request); + $response->setHeader('X-Powered-By', \Ratchet\VERSION); // This needs to be refactored later on, incorporated with routing if ('' !== ($agreedSubProtocols = $this->getSubProtocolString($request->getTokenizedHeader('Sec-WebSocket-Protocol', ',')))) { @@ -150,6 +147,16 @@ class WsServer implements MessageComponentInterface { } } + protected function close(ConnectionInterface $conn, $code = 400) { + $response = new Response($code, array( + 'Sec-WebSocket-Version' => $this->versioner->getSupportedVersionString() + , 'X-Powered-By' => \Ratchet\VERSION + )); + + $conn->send((string)$response); + $conn->close(); + } + /** * @param string * @return boolean diff --git a/tests/Ratchet/Tests/WebSocket/Version/RFC6455Test.php b/tests/Ratchet/Tests/WebSocket/Version/RFC6455Test.php index 99ba1f6..c9989bf 100644 --- a/tests/Ratchet/Tests/WebSocket/Version/RFC6455Test.php +++ b/tests/Ratchet/Tests/WebSocket/Version/RFC6455Test.php @@ -129,13 +129,15 @@ class RFC6455Test extends \PHPUnit_Framework_TestCase { * @dataProvider headerHandshakeProvider */ public function testVariousHeadersToCheckHandshakeTolerance($pass, $header) { - $request = RequestFactory::getInstance()->fromMessage($header); + $request = RequestFactory::getInstance()->fromMessage($header); + $response = $this->version->handshake($request); + + $this->assertInstanceOf('\\Guzzle\\Http\\Message\\Response', $response); if ($pass) { - $this->assertInstanceOf('\\Guzzle\\Http\\Message\\Response', $this->version->handshake($request)); + $this->assertEquals(101, $response->getStatusCode()); } else { - $this->setExpectedException('InvalidArgumentException'); - $this->version->handshake($request); + $this->assertGreaterThanOrEqual(400, $response->getStatusCode()); } }