From 492d1985d4386e31d8222c75180131c45c6dc0ca Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sat, 30 Mar 2013 15:07:11 -0400 Subject: [PATCH 1/6] [WebSocket] New failing test for Hixie fragmentation bug refs #80 --- .../Tests/WebSocket/Version/Hixie76Test.php | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php index 4aacacc..e0a35b5 100644 --- a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php +++ b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php @@ -1,6 +1,7 @@ getMock('\\Ratchet\\ConnectionInterface'); + $mockApp = $this->getMock('\\Ratchet\\MessageComponentInterface'); + + $server = new WsServer($mockApp); + $server->onOpen($mockConn); + $server->onMessage($mockConn, $headers); + + $mockApp->expects($this->once())->method('onOpen'); + $server->onMessage($mockConn, $body . $crlf . $crlf); + } } \ No newline at end of file From 51f46332ad081f8592fd180a701d7bd78f5933ab Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 31 Mar 2013 11:20:00 -0400 Subject: [PATCH 2/6] [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 From 7e58dccdb7d6c3faee48fe80a859736274a0f40f Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 31 Mar 2013 11:44:10 -0400 Subject: [PATCH 3/6] Docs --- src/Ratchet/WebSocket/HttpRequestParser.php | 1 - src/Ratchet/WebSocket/Version/Hixie76/Connection.php | 1 + src/Ratchet/WebSocket/Version/RFC6455/Connection.php | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Ratchet/WebSocket/HttpRequestParser.php b/src/Ratchet/WebSocket/HttpRequestParser.php index e7ee10f..0922ef7 100644 --- a/src/Ratchet/WebSocket/HttpRequestParser.php +++ b/src/Ratchet/WebSocket/HttpRequestParser.php @@ -51,7 +51,6 @@ class HttpRequestParser implements MessageInterface { * @return boolean */ public function isEom($message) { - //return (static::EOM === substr($message, 0 - strlen(static::EOM))); return (boolean)strpos($message, static::EOM); } } \ No newline at end of file diff --git a/src/Ratchet/WebSocket/Version/Hixie76/Connection.php b/src/Ratchet/WebSocket/Version/Hixie76/Connection.php index bb6dcaa..82053eb 100644 --- a/src/Ratchet/WebSocket/Version/Hixie76/Connection.php +++ b/src/Ratchet/WebSocket/Version/Hixie76/Connection.php @@ -4,6 +4,7 @@ use Ratchet\AbstractConnectionDecorator; /** * {@inheritdoc} + * @property \StdClass $WebSocket */ class Connection extends AbstractConnectionDecorator { public function send($msg) { diff --git a/src/Ratchet/WebSocket/Version/RFC6455/Connection.php b/src/Ratchet/WebSocket/Version/RFC6455/Connection.php index 279bd57..2d89493 100644 --- a/src/Ratchet/WebSocket/Version/RFC6455/Connection.php +++ b/src/Ratchet/WebSocket/Version/RFC6455/Connection.php @@ -5,6 +5,7 @@ use Ratchet\WebSocket\Version\DataInterface; /** * {@inheritdoc} + * @property \StdClass $WebSocket */ class Connection extends AbstractConnectionDecorator { public function send($msg) { From 6140c94a339294d5841446bbeea4055082521f2f Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 31 Mar 2013 11:50:39 -0400 Subject: [PATCH 4/6] Cleanup --- CHANGELOG.md | 4 + src/Ratchet/WebSocket/Version/Hixie76.php | 2 +- src/Ratchet/WebSocket/WsServer.php | 92 +++++++++++------------ 3 files changed, 51 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a662e9e..e5c3691 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ CHANGELOG --- +* 0.2.5 (2013-??-??) + + * Fixed Hixie-76 handshake bug + * 0.2.4 (2013-03-09) * Support for Symfony 2.2 and Guzzle 2.3 diff --git a/src/Ratchet/WebSocket/Version/Hixie76.php b/src/Ratchet/WebSocket/Version/Hixie76.php index 4043519..ab34925 100644 --- a/src/Ratchet/WebSocket/Version/Hixie76.php +++ b/src/Ratchet/WebSocket/Version/Hixie76.php @@ -39,7 +39,7 @@ class Hixie76 implements VersionInterface { * @throws \UnderflowException If there hasn't been enough data received */ public function handshake(RequestInterface $request) { - $body = substr($request->getBody(), 0, 8); + $body = substr($request->getBody(), 0, 8); if (8 !== strlen($body)) { throw new \UnderflowException("Not enough data received to issue challenge response"); } diff --git a/src/Ratchet/WebSocket/WsServer.php b/src/Ratchet/WebSocket/WsServer.php index 344e520..281f16c 100644 --- a/src/Ratchet/WebSocket/WsServer.php +++ b/src/Ratchet/WebSocket/WsServer.php @@ -88,54 +88,54 @@ class WsServer implements MessageComponentInterface { * {@inheritdoc} */ public function onMessage(ConnectionInterface $from, $msg) { - if (true !== $from->WebSocket->established) { - 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); - } - - if (!$this->versioner->isVersionEnabled($request)) { - return $this->close($from); - } - - $from->WebSocket->request = $request; - $from->WebSocket->version = $this->versioner->getVersion($request); - } - - try { - $response = $from->WebSocket->version->handshake($from->WebSocket->request); - } catch (\UnderflowException $e) { - return; - } - $response->setHeader('X-Powered-By', \Ratchet\VERSION); - - // This needs to be refactored later on, incorporated with routing - if ('' !== ($agreedSubProtocols = $this->getSubProtocolString($from->WebSocket->request->getTokenizedHeader('Sec-WebSocket-Protocol', ',')))) { - $response->setHeader('Sec-WebSocket-Protocol', $agreedSubProtocols); - } - - $from->send((string)$response); - - if (101 != $response->getStatusCode()) { - return $from->close(); - } - - $upgraded = $from->WebSocket->version->upgradeConnection($from, $this->_decorating); - - $this->connections->attach($from, $upgraded); - - $upgraded->WebSocket->established = true; - - return $this->_decorating->onOpen($upgraded); + if (true === $from->WebSocket->established) { + return $from->WebSocket->version->onMessage($this->connections[$from], $msg); } - $from->WebSocket->version->onMessage($this->connections[$from], $msg); + 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); + } + + if (!$this->versioner->isVersionEnabled($request)) { + return $this->close($from); + } + + $from->WebSocket->request = $request; + $from->WebSocket->version = $this->versioner->getVersion($request); + } + + try { + $response = $from->WebSocket->version->handshake($from->WebSocket->request); + } catch (\UnderflowException $e) { + return; + } + + // This needs to be refactored later on, incorporated with routing + if ('' !== ($agreedSubProtocols = $this->getSubProtocolString($from->WebSocket->request->getTokenizedHeader('Sec-WebSocket-Protocol', ',')))) { + $response->setHeader('Sec-WebSocket-Protocol', $agreedSubProtocols); + } + + $response->setHeader('X-Powered-By', \Ratchet\VERSION); + $from->send((string)$response); + + if (101 != $response->getStatusCode()) { + return $from->close(); + } + + $upgraded = $from->WebSocket->version->upgradeConnection($from, $this->_decorating); + + $this->connections->attach($from, $upgraded); + + $upgraded->WebSocket->established = true; + + return $this->_decorating->onOpen($upgraded); } /** From 3030c81f03fbff77698df4e378f76cb549dc995e Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 31 Mar 2013 14:02:10 -0400 Subject: [PATCH 5/6] [WebSocket] Fixed Hixie handshake bug (for real this time) refs #80 --- src/Ratchet/WebSocket/Version/Hixie76.php | 8 +------- tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php | 4 ---- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/src/Ratchet/WebSocket/Version/Hixie76.php b/src/Ratchet/WebSocket/Version/Hixie76.php index ab34925..536977b 100644 --- a/src/Ratchet/WebSocket/Version/Hixie76.php +++ b/src/Ratchet/WebSocket/Version/Hixie76.php @@ -103,13 +103,7 @@ class Hixie76 implements VersionInterface { } public function generateKeyNumber($key) { - if (0 === substr_count($key, ' ')) { - return ''; - } - - $int = (int)preg_replace('[\D]', '', $key) / substr_count($key, ' '); - - return (is_int($int)) ? $int : ''; + return preg_replace('[\D]', '', $key) / substr_count($key, ' '); } protected function sign($key1, $key2, $code) { diff --git a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php index 7107a1b..fecca81 100644 --- a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php +++ b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php @@ -31,11 +31,7 @@ class Hixie76Test extends \PHPUnit_Framework_TestCase { public static function keyProvider() { return array( array(179922739, '17 9 G`ZD9 2 2b 7X 3 /r90') - , array('', '17 9 G`ZD9 2 2b 7X 3 /r91') , array(906585445, '3e6b263 4 17 80') - , array('', '3e6b263 4 17 80') - , array('', '3e6b63 4 17 80') - , array('', '3e6b6341780') ); } From 92f3844a5357ce9826838d05b7bbc9486d59f531 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sun, 31 Mar 2013 14:16:40 -0400 Subject: [PATCH 6/6] [WebSocket] Prevent division by 0 in Hixie handshake --- src/Ratchet/WebSocket/Version/Hixie76.php | 4 ++++ tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php | 1 + 2 files changed, 5 insertions(+) diff --git a/src/Ratchet/WebSocket/Version/Hixie76.php b/src/Ratchet/WebSocket/Version/Hixie76.php index 536977b..069263a 100644 --- a/src/Ratchet/WebSocket/Version/Hixie76.php +++ b/src/Ratchet/WebSocket/Version/Hixie76.php @@ -103,6 +103,10 @@ class Hixie76 implements VersionInterface { } public function generateKeyNumber($key) { + if (0 === substr_count($key, ' ')) { + return 0; + } + return preg_replace('[\D]', '', $key) / substr_count($key, ' '); } diff --git a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php index fecca81..d31ce64 100644 --- a/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php +++ b/tests/Ratchet/Tests/WebSocket/Version/Hixie76Test.php @@ -32,6 +32,7 @@ class Hixie76Test extends \PHPUnit_Framework_TestCase { return array( array(179922739, '17 9 G`ZD9 2 2b 7X 3 /r90') , array(906585445, '3e6b263 4 17 80') + , array(0, '3e6b26341780') ); }