From 4de9caaa78b6769d116181cd8095a79b3306754b Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Fri, 25 Nov 2011 09:41:11 -0500 Subject: [PATCH 1/4] Bug Fixes Fixed bug where WebSocket protocols were being created for every connection Enabled Garbage Collection in server --- lib/Ratchet/Application/Server/App.php | 1 + lib/Ratchet/Application/WebSocket/App.php | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Ratchet/Application/Server/App.php b/lib/Ratchet/Application/Server/App.php index 8f62237..4b2306a 100644 --- a/lib/Ratchet/Application/Server/App.php +++ b/lib/Ratchet/Application/Server/App.php @@ -70,6 +70,7 @@ class App implements ApplicationInterface { $this->_connections[$host->getResource()] = new Connection($host); $this->_resources[] = $host->getResource(); + gc_enable(); set_time_limit(0); ob_implicit_flush(); diff --git a/lib/Ratchet/Application/WebSocket/App.php b/lib/Ratchet/Application/WebSocket/App.php index 46aaba3..ed8b9f5 100644 --- a/lib/Ratchet/Application/WebSocket/App.php +++ b/lib/Ratchet/Application/WebSocket/App.php @@ -16,7 +16,6 @@ use Ratchet\Application\WebSocket\Version; * @todo Make sure this works both ways (client/server) as stack needs to exist on client for framing * @todo Learn about closing the socket. A message has to be sent prior to closing - does the message get sent onClose event or CloseConnection command? * @todo Consider chaning this class to a State Pattern. If a WS App interface is passed use different state for additional methods used - * @todo I think I need to overhaul the architecture of this...more onus should be on the VersionInterfaces in case of changes...let them handle more decisions, not just parsing */ class App implements ApplicationInterface, ConfiguratorInterface { /** @@ -206,8 +205,8 @@ class App implements ApplicationInterface, ConfiguratorInterface { } else { $ns = __NAMESPACE__ . "\\Version\\{$name}"; if ($ns::isProtocol($headers)) { - $this->_version[$name] = new $ns; - return $this->_version[$name]; + $this->_versions[$name] = new $ns; + return $this->_versions[$name]; } } } From e6012d16850cfb7b57ca096bde9eb31a8ee520fc Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Fri, 25 Nov 2011 10:42:35 -0500 Subject: [PATCH 2/4] No Mask on Frame HyBi spec says server shouldn't mask payloads when delivering to client - now allow user to specify to mask or not; WebSocket by default will not mask, Framing on its own will --- lib/Ratchet/Application/WebSocket/App.php | 14 +++++++++++++- .../Application/WebSocket/Version/Hixie76.php | 2 +- .../Application/WebSocket/Version/HyBi10.php | 4 ++-- .../WebSocket/Version/VersionInterface.php | 3 ++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/Ratchet/Application/WebSocket/App.php b/lib/Ratchet/Application/WebSocket/App.php index ed8b9f5..7e70461 100644 --- a/lib/Ratchet/Application/WebSocket/App.php +++ b/lib/Ratchet/Application/WebSocket/App.php @@ -39,6 +39,8 @@ class App implements ApplicationInterface, ConfiguratorInterface { , 'Hixie76' => null ); + protected $_mask_payload = false; + public function __construct(ApplicationInterface $app = null) { if (null === $app) { throw new \UnexpectedValueException("WebSocket requires an application to run"); @@ -171,7 +173,7 @@ class App implements ApplicationInterface, ConfiguratorInterface { } $version = $command->getConnection()->WebSocket->version; - return $command->setMessage($version->frame($command->getMessage())); + return $command->setMessage($version->frame($command->getMessage(), $this->_mask_payload)); } if ($command instanceof \Traversable) { @@ -226,4 +228,14 @@ class App implements ApplicationInterface, ConfiguratorInterface { unset($this->_versions[$name]); } + + /** + * Set the option to mask the payload upon sending to client + * If WebSocket is used as server, this should be false, client to true + * @param bool + * @todo User shouldn't have to know/set this, need to figure out how to do this automatically + */ + public function setMaskPayload($opt) { + $this->_mask_payload = (boolean)$opt; + } } \ No newline at end of file diff --git a/lib/Ratchet/Application/WebSocket/Version/Hixie76.php b/lib/Ratchet/Application/WebSocket/Version/Hixie76.php index 38e94d8..03fbe78 100644 --- a/lib/Ratchet/Application/WebSocket/Version/Hixie76.php +++ b/lib/Ratchet/Application/WebSocket/Version/Hixie76.php @@ -53,7 +53,7 @@ class Hixie76 implements VersionInterface { return new Hixie76\Frame; } - public function frame($message) { + public function frame($message, $mask = true) { return chr(0) . $message . chr(255); } diff --git a/lib/Ratchet/Application/WebSocket/Version/HyBi10.php b/lib/Ratchet/Application/WebSocket/Version/HyBi10.php index 80316b3..78efb88 100644 --- a/lib/Ratchet/Application/WebSocket/Version/HyBi10.php +++ b/lib/Ratchet/Application/WebSocket/Version/HyBi10.php @@ -61,10 +61,10 @@ class HyBi10 implements VersionInterface { * @param string * @return string */ - public function frame($message) { + public function frame($message, $mask = true) { $payload = $message; $type = 'text'; - $masked = true; + $masked = $mask; $frameHead = array(); $frame = ''; diff --git a/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php b/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php index b6078d9..461e149 100644 --- a/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php +++ b/lib/Ratchet/Application/WebSocket/Version/VersionInterface.php @@ -35,8 +35,9 @@ interface VersionInterface { /** * @param string + * @param bool * @return string * @todo Change to use other classes, this will be removed eventually */ - function frame($message); + function frame($message, $mask = true); } \ No newline at end of file From 28bccc4c0768f5cc5804d22a98bced4f22002c20 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Fri, 25 Nov 2011 12:01:04 -0500 Subject: [PATCH 3/4] Typo Fixed spelling error in docs --- lib/Ratchet/Application/WebSocket/Version/Hixie76.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Ratchet/Application/WebSocket/Version/Hixie76.php b/lib/Ratchet/Application/WebSocket/Version/Hixie76.php index 03fbe78..0777701 100644 --- a/lib/Ratchet/Application/WebSocket/Version/Hixie76.php +++ b/lib/Ratchet/Application/WebSocket/Version/Hixie76.php @@ -7,7 +7,7 @@ namespace Ratchet\Application\WebSocket\Version; * 1) The handshake is done in HTTP, which includes a key for signing in the body... * BUT there is no Length defined in the header (as per HTTP spec) so the TCP buffer can't tell when the message is done! * 2) By nature it's insecure. Google did a test study where they were able to do a - * man-in-the-middle attack on 10%-15% of the people who saw their add who had a browser (currently only Safari) supporting the Hixie76 protocol. + * man-in-the-middle attack on 10%-15% of the people who saw their ad who had a browser (currently only Safari) supporting the Hixie76 protocol. * This was exploited by taking advantage of proxy servers in front of the user who ignored some HTTP headers in the handshake * The Hixie76 is currently implemented by Safari * Handshake from Andrea Giammarchi (http://webreflection.blogspot.com/2010/06/websocket-handshake-76-simplified.html) From bd954fae5dfd94271ee48584095561b80760b76c Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Fri, 25 Nov 2011 16:49:56 -0500 Subject: [PATCH 4/4] Fixed memory leak High number of connections caused memory issues, was parsing every outgoing message even though it was the same for many clients --- lib/Ratchet/Application/WebSocket/App.php | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/Ratchet/Application/WebSocket/App.php b/lib/Ratchet/Application/WebSocket/App.php index 7e70461..4600473 100644 --- a/lib/Ratchet/Application/WebSocket/App.php +++ b/lib/Ratchet/Application/WebSocket/App.php @@ -167,18 +167,36 @@ class App implements ApplicationInterface, ConfiguratorInterface { * @return Ratchet\Resource\Command\CommandInterface|NULL */ protected function prepareCommand(CommandInterface $command = null) { + $cache = array(); + return $this->mungCommand($command, $cache); + } + + /** + * Does the actual work of prepareCommand + * Separated to pass the cache array by reference, so we're not framing the same stirng over and over + * @param Ratchet\Resource\Command\CommandInterface|NULL + * @param array + * @return Ratchet\Resource\Command\CommandInterface|NULL + */ + protected function mungCommand(CommandInterface $command = null, &$cache) { if ($command instanceof SendMessage) { if (!isset($command->getConnection()->WebSocket->version)) { // Client could close connection before handshake complete or invalid handshake return $command; } $version = $command->getConnection()->WebSocket->version; - return $command->setMessage($version->frame($command->getMessage(), $this->_mask_payload)); + $hash = md5($command->getMessage()) . '-' . spl_object_hash($version); + + if (!isset($cache[$hash])) { + $cache[$hash] = $version->frame($command->getMessage(), $this->_mask_payload); + } + + return $command->setMessage($cache[$hash]); } if ($command instanceof \Traversable) { foreach ($command as $cmd) { - $cmd = $this->prepareCommand($cmd); + $cmd = $this->mungCommand($cmd, $cache); } }