Correct Responses in ServerNegotiator add tests

src/Handshake/ServerNegotiator.php

@@ -13,12 +13,15 @@ class ServerNegotiator implements NegotiatorInterface {
      */
     private $verifier;
 
-    private $_supportedSubProtocols = [];
+    private $_supportedSubProtocols;
 
-    private $_strictSubProtocols = false;
+    private $_strictSubProtocols;
 
-    public function __construct(RequestVerifier $requestVerifier) {
+    public function __construct(RequestVerifier $requestVerifier, array $supportedSubProtocols = [], $strictSubProtocol = false) {
         $this->verifier = $requestVerifier;
+
+        $this->_supportedSubProtocols = $supportedSubProtocols;
+        $this->_strictSubProtocols = $strictSubProtocol;
     }
 
     /**
@@ -55,20 +58,38 @@ class ServerNegotiator implements NegotiatorInterface {
             return new Response(400);
         }
 
+        $upgradeSuggestion = [
+            'Connection'             => 'Upgrade',
+            'Upgrade'                => 'websocket',
+            'Sec-WebSocket-Version'  => $this->getVersionNumber(),
+            'Sec-WebSocket-Protocol' => implode(', ', $this->_supportedSubProtocols)
+        ];
         if (true !== $this->verifier->verifyUpgradeRequest($request->getHeader('Upgrade'))) {
-            return new Response(400, [], '1.1', null, 'Upgrade header MUST be provided');
+            return new Response(
+                426,
+                $upgradeSuggestion,
+                null,
+                '1.1',
+                'Upgrade header MUST be provided'
+            );
         }
 
         if (true !== $this->verifier->verifyConnection($request->getHeader('Connection'))) {
-            return new Response(400, [], '1.1', null, 'Connection header MUST be provided');
+            return new Response(400, [], null, '1.1', 'Connection Upgrade MUST be requested');
         }
 
         if (true !== $this->verifier->verifyKey($request->getHeader('Sec-WebSocket-Key'))) {
-            return new Response(400, [], '1.1', null, 'Invalid Sec-WebSocket-Key');
+            return new Response(400, [], null, '1.1', 'Invalid Sec-WebSocket-Key');
         }
 
         if (true !== $this->verifier->verifyVersion($request->getHeader('Sec-WebSocket-Version'))) {
-            return new Response(426, ['Sec-WebSocket-Version' => $this->getVersionNumber()]);
+            /*
+             * https://tools.ietf.org/html/rfc7230#section-6.7
+             * A server that sends a 426 (Upgrade Required) response MUST send an
+             * Upgrade header field to indicate the acceptable protocols, in order
+             * of descending preference
+             */
+            return new Response(426, $upgradeSuggestion);
         }
 
         $headers = [];
@@ -81,7 +102,7 @@ class ServerNegotiator implements NegotiatorInterface {
             }, null);
 
             if ($this->_strictSubProtocols && null === $match) {
-                return new Response(400, [], '1.1', null ,'No Sec-WebSocket-Protocols requested supported');
+                return new Response(426, $upgradeSuggestion);
             }
 
             if (null !== $match) {
@@ -107,6 +128,10 @@ class ServerNegotiator implements NegotiatorInterface {
         return base64_encode(sha1($key . static::GUID, true));
     }
 
+    /**
+     * @param array $protocols
+     * @deprecated
+     */
     function setSupportedSubProtocols(array $protocols) {
         $this->_supportedSubProtocols = array_flip($protocols);
     }
@@ -118,6 +143,7 @@ class ServerNegotiator implements NegotiatorInterface {
      * @todo Consider extending this interface and moving this there.
      *       The spec does says the server can fail for this reason, but
      * it is not a requirement. This is an implementation detail.
+     * @deprecated
      */
     function setStrictSubProtocolCheck($enable) {
         $this->_strictSubProtocols = (boolean)$enable;

tests/unit/Handshake/ServerNegotiatorTest.php

@@ -0,0 +1,143 @@
+<?php
+
+namespace Ratchet\RFC6455\Test\Unit\Handshake;
+
+use function GuzzleHttp\Psr7\parse_request;
+use Ratchet\RFC6455\Handshake\RequestVerifier;
+use Ratchet\RFC6455\Handshake\ServerNegotiator;
+
+class ServerNegotiatorTest extends \PHPUnit_Framework_TestCase
+{
+    public function testNoUpgradeRequested() {
+        $negotiator = new ServerNegotiator(new RequestVerifier());
+
+        $requestText = 'GET / HTTP/1.1
+Host: 127.0.0.1:6789
+Connection: keep-alive
+Pragma: no-cache
+Cache-Control: no-cache
+Upgrade-Insecure-Requests: 1
+User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
+Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
+Accept-Encoding: gzip, deflate, sdch, br
+Accept-Language: en-US,en;q=0.8';
+
+        $request = parse_request($requestText);
+
+        $response = $negotiator->handshake($request);
+
+        $this->assertEquals('1.1', $response->getProtocolVersion());
+        $this->assertEquals(426, $response->getStatusCode());
+        $this->assertEquals('Upgrade header MUST be provided', $response->getReasonPhrase());
+        $this->assertEquals('Upgrade', $response->getHeaderLine('Connection'));
+        $this->assertEquals('websocket', $response->getHeaderLine('Upgrade'));
+        $this->assertEquals('13', $response->getHeaderLine('Sec-WebSocket-Version'));
+    }
+
+    public function testNoConnectionUpgradeRequested() {
+        $negotiator = new ServerNegotiator(new RequestVerifier());
+
+        $requestText = 'GET / HTTP/1.1
+Host: 127.0.0.1:6789
+Connection: keep-alive
+Pragma: no-cache
+Cache-Control: no-cache
+Upgrade: websocket
+Upgrade-Insecure-Requests: 1
+User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
+Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
+Accept-Encoding: gzip, deflate, sdch, br
+Accept-Language: en-US,en;q=0.8';
+
+        $request = parse_request($requestText);
+
+        $response = $negotiator->handshake($request);
+
+        $this->assertEquals('1.1', $response->getProtocolVersion());
+        $this->assertEquals(400, $response->getStatusCode());
+        $this->assertEquals('Connection Upgrade MUST be requested', $response->getReasonPhrase());
+    }
+
+    public function testInvalidSecWebsocketKey() {
+        $negotiator = new ServerNegotiator(new RequestVerifier());
+
+        $requestText = 'GET / HTTP/1.1
+Host: 127.0.0.1:6789
+Connection: Upgrade
+Pragma: no-cache
+Cache-Control: no-cache
+Upgrade: websocket
+Sec-WebSocket-Key: 12345
+Upgrade-Insecure-Requests: 1
+User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
+Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
+Accept-Encoding: gzip, deflate, sdch, br
+Accept-Language: en-US,en;q=0.8';
+
+        $request = parse_request($requestText);
+
+        $response = $negotiator->handshake($request);
+
+        $this->assertEquals('1.1', $response->getProtocolVersion());
+        $this->assertEquals(400, $response->getStatusCode());
+        $this->assertEquals('Invalid Sec-WebSocket-Key', $response->getReasonPhrase());
+    }
+
+    public function testInvalidSecWebsocketVersion() {
+        $negotiator = new ServerNegotiator(new RequestVerifier());
+
+        $requestText = 'GET / HTTP/1.1
+Host: 127.0.0.1:6789
+Connection: Upgrade
+Pragma: no-cache
+Cache-Control: no-cache
+Upgrade: websocket
+Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
+Upgrade-Insecure-Requests: 1
+User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
+Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
+Accept-Encoding: gzip, deflate, sdch, br
+Accept-Language: en-US,en;q=0.8';
+
+        $request = parse_request($requestText);
+
+        $response = $negotiator->handshake($request);
+
+        $this->assertEquals('1.1', $response->getProtocolVersion());
+        $this->assertEquals(426, $response->getStatusCode());
+        $this->assertEquals('Upgrade Required', $response->getReasonPhrase());
+        $this->assertEquals('Upgrade', $response->getHeaderLine('Connection'));
+        $this->assertEquals('websocket', $response->getHeaderLine('Upgrade'));
+        $this->assertEquals('13', $response->getHeaderLine('Sec-WebSocket-Version'));
+    }
+
+    public function testBadSubprotocolResponse() {
+        $negotiator = new ServerNegotiator(new RequestVerifier(), [], true);
+
+        $requestText = 'GET / HTTP/1.1
+Host: 127.0.0.1:6789
+Connection: Upgrade
+Pragma: no-cache
+Cache-Control: no-cache
+Upgrade: websocket
+Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
+Sec-WebSocket-Version: 13
+Sec-WebSocket-Protocol: someprotocol
+Upgrade-Insecure-Requests: 1
+User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36
+Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
+Accept-Encoding: gzip, deflate, sdch, br
+Accept-Language: en-US,en;q=0.8';
+
+        $request = parse_request($requestText);
+
+        $response = $negotiator->handshake($request);
+
+        $this->assertEquals('1.1', $response->getProtocolVersion());
+        $this->assertEquals(426, $response->getStatusCode());
+        $this->assertEquals('Upgrade Required', $response->getReasonPhrase());
+        $this->assertEquals('Upgrade', $response->getHeaderLine('Connection'));
+        $this->assertEquals('websocket', $response->getHeaderLine('Upgrade'));
+        $this->assertEquals('13', $response->getHeaderLine('Sec-WebSocket-Version'));
+    }
+}