From 51f46332ad081f8592fd180a701d7bd78f5933ab Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 31 Mar 2013 11:20:00 -0400 Subject: [PATCH] [WebSocket] Fixed failing Hixie handshake bug refs #80 --- .../Guzzle/Http/Message/RequestFactory.php | 6 +-- src/Ratchet/WebSocket/Version/Hixie76.php | 10 +++- src/Ratchet/WebSocket/WsServer.php | 34 ++++++++------ .../Tests/WebSocket/Version/Hixie76Test.php | 46 +++++++++++++------ 4 files changed, 64 insertions(+), 32 deletions(-) diff --git a/src/Ratchet/WebSocket/Guzzle/Http/Message/RequestFactory.php b/src/Ratchet/WebSocket/Guzzle/Http/Message/RequestFactory.php index bd7e490..c1f6b45 100644 --- a/src/Ratchet/WebSocket/Guzzle/Http/Message/RequestFactory.php +++ b/src/Ratchet/WebSocket/Guzzle/Http/Message/RequestFactory.php @@ -7,12 +7,10 @@ class RequestFactory extends GuzzleRequestFactory { /** * {@inheritdoc} */ - public function create($method, $url, $headers = null, $body = null) { + public function create($method, $url, $headers = null, $body = '') { $c = $this->entityEnclosingRequestClass; $request = new $c($method, $url, $headers); - if ($body) { - $request->setBody(EntityBody::factory($body)); - } + $request->setBody(EntityBody::factory($body)); return $request; } diff --git a/src/Ratchet/WebSocket/Version/Hixie76.php b/src/Ratchet/WebSocket/Version/Hixie76.php index fbb8e1d..4043519 100644 --- a/src/Ratchet/WebSocket/Version/Hixie76.php +++ b/src/Ratchet/WebSocket/Version/Hixie76.php @@ -36,9 +36,15 @@ class Hixie76 implements VersionInterface { /** * @param \Guzzle\Http\Message\RequestInterface $request * @return \Guzzle\Http\Message\Response + * @throws \UnderflowException If there hasn't been enough data received */ public function handshake(RequestInterface $request) { - $body = $this->sign($request->getHeader('Sec-WebSocket-Key1', true), $request->getHeader('Sec-WebSocket-Key2', true), (string)$request->getBody()); + $body = substr($request->getBody(), 0, 8); + if (8 !== strlen($body)) { + throw new \UnderflowException("Not enough data received to issue challenge response"); + } + + $challenge = $this->sign($request->getHeader('Sec-WebSocket-Key1', true), $request->getHeader('Sec-WebSocket-Key2', true), $body); $headers = array( 'Upgrade' => 'WebSocket' @@ -47,7 +53,7 @@ class Hixie76 implements VersionInterface { , 'Sec-WebSocket-Location' => 'ws://' . $request->getHeader('Host', true) . $request->getPath() ); - $response = new Response(101, $headers, $body); + $response = new Response(101, $headers, $challenge); $response->setStatus(101, 'WebSocket Protocol Handshake'); return $response; diff --git a/src/Ratchet/WebSocket/WsServer.php b/src/Ratchet/WebSocket/WsServer.php index 409259b..344e520 100644 --- a/src/Ratchet/WebSocket/WsServer.php +++ b/src/Ratchet/WebSocket/WsServer.php @@ -89,26 +89,34 @@ class WsServer implements MessageComponentInterface { */ public function onMessage(ConnectionInterface $from, $msg) { if (true !== $from->WebSocket->established) { - try { - if (null === ($request = $this->reqParser->onMessage($from, $msg))) { - return; + if (isset($from->WebSocket->request)) { + $from->WebSocket->request->getBody()->write($msg); + } else { + try { + if (null === ($request = $this->reqParser->onMessage($from, $msg))) { + return; + } + } catch (\OverflowException $oe) { + return $this->close($from, 413); } - } catch (\OverflowException $oe) { - return $this->close($from, 413); + + if (!$this->versioner->isVersionEnabled($request)) { + return $this->close($from); + } + + $from->WebSocket->request = $request; + $from->WebSocket->version = $this->versioner->getVersion($request); } - if (!$this->versioner->isVersionEnabled($request)) { - return $this->close($from); + try { + $response = $from->WebSocket->version->handshake($from->WebSocket->request); + } catch (\UnderflowException $e) { + return; } - - $from->WebSocket->request = $request; - $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', ',')))) { + if ('' !== ($agreedSubProtocols = $this->getSubProtocolString($from->WebSocket->request->getTokenizedHeader('Sec-WebSocket-Protocol', ',')))) { $response->setHeader('Sec-WebSocket-Protocol', $agreedSubProtocols); } diff --git a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php index e0a35b5..7107a1b 100644 --- a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php +++ b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php @@ -7,6 +7,9 @@ use Ratchet\WebSocket\WsServer; * @covers Ratchet\WebSocket\Version\Hixie76 */ class Hixie76Test extends \PHPUnit_Framework_TestCase { + protected $_crlf = "\r\n"; + protected $_body = '6dW+XgKfWV0='; + protected $_version; public function setUp() { @@ -36,22 +39,39 @@ class Hixie76Test extends \PHPUnit_Framework_TestCase { ); } - public function testTcpFragmentedUpgrade() { + public function headerProvider() { $key1 = base64_decode('QTN+ICszNiA2IDJvICBWOG4gNyAgc08yODhZ'); $key2 = base64_decode('TzEyICAgeVsgIFFSNDUgM1IgLiAyOFggNC00dn4z'); - $body = base64_decode('6dW+XgKfWV0='); - - $crlf = "\r\n"; $headers = "GET / HTTP/1.1"; - $headers .= "Upgrade: WebSocket{$crlf}"; - $headers .= "Connection: Upgrade{$crlf}"; - $headers .= "Host: home.chrisboden.ca{$crlf}"; - $headers .= "Origin: http://fiddle.jshell.net{$crlf}"; - $headers .= "Sec-WebSocket-Key1:17 Z4< F94 N3 7P41 7{$crlf}"; - $headers .= "Sec-WebSocket-Key2:1 23C3:,2% 1-29 4 f0{$crlf}"; - $headers .= "(Key3):70:00:EE:6E:33:20:90:69{$crlf}"; - $headers .= $crlf; + $headers .= "Upgrade: WebSocket{$this->_crlf}"; + $headers .= "Connection: Upgrade{$this->_crlf}"; + $headers .= "Host: home.chrisboden.ca{$this->_crlf}"; + $headers .= "Origin: http://fiddle.jshell.net{$this->_crlf}"; + $headers .= "Sec-WebSocket-Key1:17 Z4< F94 N3 7P41 7{$this->_crlf}"; + $headers .= "Sec-WebSocket-Key2:1 23C3:,2% 1-29 4 f0{$this->_crlf}"; + $headers .= "(Key3):70:00:EE:6E:33:20:90:69{$this->_crlf}"; + $headers .= $this->_crlf; + + return $headers; + } + + public function testNoUpgradeBeforeBody() { + $headers = $this->headerProvider(); + $body = base64_decode($this->_body); + + $mockConn = $this->getMock('\\Ratchet\\ConnectionInterface'); + $mockApp = $this->getMock('\\Ratchet\\MessageComponentInterface'); + + $server = new WsServer($mockApp); + $server->onOpen($mockConn); + $mockApp->expects($this->exactly(0))->method('onOpen'); + $server->onMessage($mockConn, $headers); + } + + public function testTcpFragmentedUpgrade() { + $headers = $this->headerProvider(); + $body = base64_decode($this->_body); $mockConn = $this->getMock('\\Ratchet\\ConnectionInterface'); $mockApp = $this->getMock('\\Ratchet\\MessageComponentInterface'); @@ -61,6 +81,6 @@ class Hixie76Test extends \PHPUnit_Framework_TestCase { $server->onMessage($mockConn, $headers); $mockApp->expects($this->once())->method('onOpen'); - $server->onMessage($mockConn, $body . $crlf . $crlf); + $server->onMessage($mockConn, $body . $this->_crlf . $this->_crlf); } } \ No newline at end of file