Repository: tornadoweb/tornado

Issue:
AttributeError if Websocket client misses required header
If the client misses required header for websocket handshake, the server raises AttributeError.

Minimal code for reproduce

### Client

```python
import socket

REQ_1 = ('GET /ws HTTP/1.1\r\n'
         'Host: example.com:9221\r\n'
         'Upgrade: websocket\r\n'
         'Connection: Upgrade\r\n'
        #  'Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n'
         'Sec-WebSocket-Version: 13\r\n'
         '\r\n')
conn = socket.create_connection(('127.0.0.1', 9221))
conn.send(REQ_1.encode('utf-8'))
resp_1 = conn.recv(10 * 1024)
```

### Server

```python
import tornado.ioloop
import tornado.web
import tornado.websocket

class WsHandler(tornado.websocket.WebSocketHandler):
    pass

def make_app():
    return tornado.web.Application([
        (r'/ws', WsHandler),
    ])

if __name__ == "__main__":
    app = make_app()
    app.listen(9221)
    tornado.ioloop.IOLoop.current().start()
```

### Traceback

```
ERROR:tornado.application:Uncaught exception GET /ws (127.0.0.1)
HTTPServerRequest(protocol='http', host='example.com:8000', method='GET', uri='/ws', version='HTTP/1.1', remote_ip='127.0.0.1', headers={'Host': 'example.com:8000', 'Upgrade': 'websocket', 'Connection': 'Upgrade', 'Sec-Websocket-Version': '13'})
Traceback (most recent call last):
  File "/home/pjknkda/test/ws-invalid/python-env/lib/python3.6/site-packages/tornado/websocket.py", line 618, in accept_connection
    self._handle_websocket_headers()
  File "/home/pjknkda/test/ws-invalid/python-env/lib/python3.6/site-packages/tornado/websocket.py", line 634, in _handle_websocket_headers
    raise ValueError("Missing/Invalid WebSocket headers")
ValueError: Missing/Invalid WebSocket headers

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/pjknkda/test/ws-invalid/python-env/lib/python3.6/site-packages/tornado/web.py", line 1467, in _stack_context_handle_exception
    raise_exc_info((type, value, traceback))
  File "<string>", line 4, in raise_exc_info
  File "/home/pjknkda/test/ws-invalid/python-env/lib/python3.6/site-packages/tornado/web.py", line 1669, in wrapper
    result = method(self, *args, **kwargs)
  File "/home/pjknkda/test/ws-invalid/python-env/lib/python3.6/site-packages/tornado/websocket.py", line 196, in get
    self.ws_connection.accept_connection()
  File "/home/pjknkda/test/ws-invalid/python-env/lib/python3.6/site-packages/tornado/websocket.py", line 623, in accept_connection
    self._abort()
  File "/home/pjknkda/test/ws-invalid/python-env/lib/python3.6/site-packages/tornado/websocket.py", line 512, in _abort
    self.stream.close()  # forcibly tear down the connection
AttributeError: 'NoneType' object has no attribute 'close'
ERROR:tornado.access:500 GET /ws (127.0.0.1) 4.13ms
```

It seems that `WebSocketProtocol13.accept_connection` calls `WebSocketProtocol._abort` immediately if there is missing required headers, however, it is before the handshake, thus there is yet no `self.stream` whereas the _abort function tries to `self.stream.close()`. Also, the _abort function calls `self.close()` and there is also the same buggy code which calls `self.stream.close()` without checking the nullity of `self.stream`.

Discussion:

Patch:
diff --git a/tornado/websocket.py b/tornado/websocket.py
index d5a7fa89..c6804ca0 100644
--- a/tornado/websocket.py
+++ b/tornado/websocket.py
@@ -616,6 +616,14 @@ class WebSocketProtocol13(WebSocketProtocol):
     def accept_connection(self):
         try:
             self._handle_websocket_headers()
+        except ValueError:
+            self.handler.set_status(400)
+            log_msg = "Missing/Invalid WebSocket headers"
+            self.handler.finish(log_msg)
+            gen_log.debug(log_msg)
+            return
+
+        try:
             self._accept_connection()
         except ValueError:
             gen_log.debug("Malformed WebSocket request received",


Test patch:
diff --git a/tornado/test/websocket_test.py b/tornado/test/websocket_test.py
index 5a2a6577..54734d81 100644
--- a/tornado/test/websocket_test.py
+++ b/tornado/test/websocket_test.py
@@ -193,6 +193,13 @@ class WebSocketTest(WebSocketBaseTestCase):
         response = self.fetch('/echo')
         self.assertEqual(response.code, 400)
 
+    def test_missing_websocket_key(self):
+        response = self.fetch('/echo',
+                              headers={'Connection': 'Upgrade',
+                                       'Upgrade': 'WebSocket',
+                                       'Sec-WebSocket-Version': '13'})
+        self.assertEqual(response.code, 400)
+
     def test_bad_websocket_version(self):
         response = self.fetch('/echo',
                               headers={'Connection': 'Upgrade',