From 5e795984487132820e96d8ec18cf6826b92a48d1 Mon Sep 17 00:00:00 2001 From: Chris Boden Date: Sat, 29 Nov 2014 13:08:04 -0500 Subject: [PATCH] Separate negotiation and validation --- src/Handshake/Negotiator.php | 32 +----- src/Handshake/NegotiatorInterface.php | 2 - src/Handshake/RequestVerifier.php | 9 +- src/Messaging/Validation/MessageValidator.php | 105 ++++++++++++++++++ 4 files changed, 111 insertions(+), 37 deletions(-) create mode 100644 src/Messaging/Validation/MessageValidator.php diff --git a/src/Handshake/Negotiator.php b/src/Handshake/Negotiator.php index ccddb07..69a5fe7 100644 --- a/src/Handshake/Negotiator.php +++ b/src/Handshake/Negotiator.php @@ -54,7 +54,7 @@ class Negotiator implements NegotiatorInterface { * {@inheritdoc} */ public function getVersionNumber() { - return 13; + return RequestVerifier::VERSION; } /** @@ -111,25 +111,7 @@ class Negotiator implements NegotiatorInterface { if ($from->WebSocket->frame->isCoalesced()) { $frame = $from->WebSocket->frame; - if (false !== $frame->getRsv1() || - false !== $frame->getRsv2() || - false !== $frame->getRsv3() - ) { - return $from->close($frame::CLOSE_PROTOCOL); - } - - // This is server-side specific logic - if (!$frame->isMasked()) { - return $from->close($frame::CLOSE_PROTOCOL); - } - - $opcode = $frame->getOpcode(); - if ($opcode > 2) { - if ($frame->getPayloadLength() > 125 || !$frame->isFinal()) { - return $from->close($frame::CLOSE_PROTOCOL); - } - switch ($opcode) { case $frame::OP_CLOSE: $closeCode = 0; @@ -177,14 +159,6 @@ class Negotiator implements NegotiatorInterface { $overflow = $from->WebSocket->frame->extractOverflow(); - if ($frame::OP_CONTINUE == $frame->getOpcode() && 0 == count($from->WebSocket->message)) { - return $from->close($frame::CLOSE_PROTOCOL); - } - - if (count($from->WebSocket->message) > 0 && $frame::OP_CONTINUE != $frame->getOpcode()) { - return $from->close($frame::CLOSE_PROTOCOL); - } - $from->WebSocket->message->addFrame($from->WebSocket->frame); unset($from->WebSocket->frame); } @@ -193,10 +167,6 @@ class Negotiator implements NegotiatorInterface { $parsed = $from->WebSocket->message->getPayload(); unset($from->WebSocket->message); - if (!$this->validator->checkEncoding($parsed, 'UTF-8')) { - return $from->close(Frame::CLOSE_BAD_PAYLOAD); - } - $from->WebSocket->coalescedCallback->onMessage($from, $parsed); } diff --git a/src/Handshake/NegotiatorInterface.php b/src/Handshake/NegotiatorInterface.php index 624772d..718d760 100644 --- a/src/Handshake/NegotiatorInterface.php +++ b/src/Handshake/NegotiatorInterface.php @@ -15,7 +15,6 @@ interface NegotiatorInterface { * Given an HTTP header, determine if this version should handle the protocol * @param \Guzzle\Http\Message\RequestInterface $request * @return bool - * @throws \UnderflowException If the protocol thinks the headers are still fragmented */ function isProtocol(RequestInterface $request); @@ -29,7 +28,6 @@ interface NegotiatorInterface { * Perform the handshake and return the response headers * @param \Guzzle\Http\Message\RequestInterface $request * @return \Guzzle\Http\Message\Response - * @throws \UnderflowException If the message hasn't finished buffering (not yet implemented, theoretically will only happen with Hixie version) */ function handshake(RequestInterface $request); diff --git a/src/Handshake/RequestVerifier.php b/src/Handshake/RequestVerifier.php index 1e9e2a6..e9f8d6d 100644 --- a/src/Handshake/RequestVerifier.php +++ b/src/Handshake/RequestVerifier.php @@ -8,6 +8,8 @@ use Guzzle\Http\Message\RequestInterface; * @todo Currently just returning invalid - should consider returning appropriate HTTP status code error #s */ class RequestVerifier { + const VERSION = 13; + /** * Given an array of the headers this method will run through all verification methods * @param \Guzzle\Http\Message\RequestInterface $request @@ -23,9 +25,9 @@ class RequestVerifier { $passes += (int)$this->verifyUpgradeRequest((string)$request->getHeader('Upgrade')); $passes += (int)$this->verifyConnection((string)$request->getHeader('Connection')); $passes += (int)$this->verifyKey((string)$request->getHeader('Sec-WebSocket-Key')); - //$passes += (int)$this->verifyVersion($headers['Sec-WebSocket-Version']); // Temporarily breaking functionality + $passes += (int)$this->verifyVersion($headers['Sec-WebSocket-Version']); - return (7 === $passes); + return (8 === $passes); } /** @@ -117,10 +119,9 @@ class RequestVerifier { * Verify the version passed matches this RFC * @param string|int MUST equal 13|"13" * @return bool - * @todo Ran in to a problem here...I'm having HyBi use the RFC files, this breaks it! oops */ public function verifyVersion($val) { - return (13 === (int)$val); + return (static::VERSION === (int)$val); } /** diff --git a/src/Messaging/Validation/MessageValidator.php b/src/Messaging/Validation/MessageValidator.php new file mode 100644 index 0000000..b15b726 --- /dev/null +++ b/src/Messaging/Validation/MessageValidator.php @@ -0,0 +1,105 @@ +validator = $validator; + } + + /** + * 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 + */ + 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->checkFrame($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; + } + + if (count($message) > 0 && $frame::OP_CONTINUE !== $frame->getOpcode()) { + return $frame::CLOSE_PROTOCOL; + } + + $parsed = $message->getPayload(); + if (!$this->validator->checkEncoding($parsed, 'UTF-8')) { + return $frame::CLOSE_BAD_PAYLOAD; + } + + return true; + } + + public function validateFrame(FrameInterface $frame) { + if (false !== $frame->getRsv1() || + false !== $frame->getRsv2() || + false !== $frame->getRsv3() + ) { + return $frame::CLOSE_PROTOCOL; + } + + // Should be checking all frames + if ($this->checkForMask && !$frame->isMasked()) { + return $frame::CLOSE_PROTOCOL; + } + + $opcode = $frame->getOpcode(); + + if ($opcode > 2) { + if ($frame->getPayloadLength() > 125 || !$frame->isFinal()) { + return $frame::CLOSE_PROTOCOL; + } + + switch ($opcode) { + case $frame::OP_CLOSE: + $closeCode = 0; + + $bin = $frame->getPayload(); + + if (empty($bin)) { + return $frame::CLOSE_NORMAL; + } + + if (strlen($bin) >= 2) { + list($closeCode) = array_merge(unpack('n*', substr($bin, 0, 2))); + } + + if (!$this->isValidCloseCode($closeCode)) { + return $frame::CLOSE_PROTOCOL; + } + + if (!$this->validator->checkEncoding(substr($bin, 2), 'UTF-8')) { + return $frame::CLOSE_BAD_PAYLOAD; + } + + return $frame::CLOSE_NORMAL; + break; + case $frame::OP_PING: + case $frame::OP_PONG: + break; + default: + return $frame::CLOSE_PROTOCOL; + break; + } + } + + return true; + } +}