Merge commit 'a031605' into pr135

Author: Lukasa

hyper/http20/connection.py

@@ -12,7 +12,7 @@
 from ..packages.hyperframe.frame import (
     FRAMES, DataFrame, HeadersFrame, PushPromiseFrame, RstStreamFrame,
     SettingsFrame, Frame, WindowUpdateFrame, GoAwayFrame, PingFrame,
-    BlockedFrame
+    BlockedFrame, FRAME_MAX_LEN, FRAME_MAX_ALLOWED_LEN
 )
 from ..packages.hpack.hpack_compat import Encoder, Decoder
 from .stream import Stream
@@ -123,6 +123,7 @@ def __init_state(self):
         # Values for the settings used on an HTTP/2 connection.
         self._settings = {
             SettingsFrame.INITIAL_WINDOW_SIZE: 65535,
+            SettingsFrame.SETTINGS_MAX_FRAME_SIZE: FRAME_MAX_LEN,
         }
 
         # The socket used to send data.
@@ -247,16 +248,17 @@ def _send_preamble(self):
         # The server will also send an initial settings frame, so get it.
         self._recv_cb()
 
-    def close(self):
+    def close(self, error_code=None):
         """
         Close the connection to the server.
 
+        :param error_code: (optional) The error code to reset all streams with.
         :returns: Nothing.
         """
         # Close all streams
         for stream in list(self.streams.values()):
             log.debug("Close stream %d" % stream.stream_id)
-            stream.close()
+            stream.close(error_code)
 
         # Send GoAway frame to the server
         try:
@@ -388,10 +390,14 @@ def receive_frame(self, frame):
             if 'ACK' not in frame.flags:
                 self._update_settings(frame)
 
-                # Need to return an ack.
-                f = SettingsFrame(0)
-                f.flags.add('ACK')
-                self._send_cb(f)
+                # When the setting containing the max frame size value is out
+                # of range, the spec dictates to tear down the connection.
+                # Therefore we make sure the socket is still alive before
+                # returning the ack.
+                if self._sock is not None:
+                    f = SettingsFrame(0)
+                    f.flags.add('ACK')
+                    self._send_cb(f)
         elif frame.type == GoAwayFrame.type:
             # If we get GoAway with error code zero, we are doing a graceful
             # shutdown and all is well. Otherwise, throw an exception.
@@ -449,6 +455,20 @@ def _update_settings(self, frame):
 
             self._settings[SettingsFrame.INITIAL_WINDOW_SIZE] = newsize
 
+        if SettingsFrame.SETTINGS_MAX_FRAME_SIZE in frame.settings:
+            new_size = frame.settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE]
+            if FRAME_MAX_LEN <= new_size <= FRAME_MAX_ALLOWED_LEN:
+                self._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] = new_size
+            else:
+                log.warning(
+                    "Frame size %d is outside of allowed range",
+                    new_size)
+                # Tear the connection down with error code PROTOCOL_ERROR
+                self.close(1)
+                error_string = ("Advertised frame size %d is outside of range" %
+                                (new_size))
+                raise ConnectionError(error_string)
+
     def _new_stream(self, stream_id=None, local_closed=False):
         """
         Returns a new stream object for this connection.
@@ -469,12 +489,7 @@ def _close_stream(self, stream_id, error_code=None):
         """
         Called by a stream when it would like to be 'closed'.
         """
-        if error_code is not None:
-            f = RstStreamFrame(stream_id)
-            f.error_code = error_code
-            self._send_cb(f)
-
-        del self.streams[stream_id]
+        self._send_rst_frame(stream_id, error_code)
 
     def _send_cb(self, frame, tolerate_peer_gone=False):
         """
@@ -495,6 +510,13 @@ def _send_cb(self, frame, tolerate_peer_gone=False):
 
         data = frame.serialize()
 
+        if frame.body_len > self._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE]:
+            raise ValueError(
+                     "Frame size %d exceeds maximum frame size setting %d" %
+                     (frame.body_len,
+                     self._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE])
+            )
+
         log.info(
             "Sending frame %s on stream %d",
             frame.__class__.__name__,
@@ -535,6 +557,15 @@ def _consume_single_frame(self):
         # Parse the header. We can use the returned memoryview directly here.
         frame, length = Frame.parse_frame_header(header)
 
+        if (length > FRAME_MAX_LEN):
+            log.warning(
+                "Frame size exceeded on stream %d (received: %d, max: %d)",
+                frame.stream_id,
+                length,
+                FRAME_MAX_LEN
+            )
+            self._send_rst_frame(frame.stream_id, 6) # 6 = FRAME_SIZE_ERROR
+
         # Read the remaining data from the socket.
         data = self._recv_payload(length)
         self._consume_frame_payload(frame, data)
@@ -595,9 +626,7 @@ def _consume_frame_payload(self, frame, data):
                 # the ENABLE_PUSH setting is 0, but the spec leaves the client
                 # action undefined when they do it anyway. So we just refuse
                 # the stream and go about our business.
-                f = RstStreamFrame(frame.promised_stream_id)
-                f.error_code = 7 # REFUSED_STREAM
-                self._send_cb(f)
+                self._send_rst_frame(frame.promised_stream_id, 7)
 
         # Work out to whom this frame should go.
         if frame.stream_id != 0:
@@ -606,9 +635,7 @@ def _consume_frame_payload(self, frame, data):
             except KeyError:
                 # If we receive an unexpected stream identifier then we
                 # cancel the stream with an error of type PROTOCOL_ERROR
-                f = RstStreamFrame(frame.stream_id)
-                f.error_code = 1 # PROTOCOL_ERROR
-                self._send_cb(f)
+                self._send_rst_frame(frame.stream_id, 1)
                 log.warning(
                     "Unexpected stream identifier %d" % (frame.stream_id)
                 )
@@ -637,6 +664,20 @@ def _recv_cb(self):
             except ConnectionResetError:
                 break
 
+    def _send_rst_frame(self, stream_id, error_code):
+        """
+        Send reset stream frame with error code and remove stream from map.
+        """
+        f = RstStreamFrame(stream_id)
+        f.error_code = error_code
+        self._send_cb(f)
+
+        try:
+            del self.streams[stream_id]
+        except KeyError as e:  # pragma: no cover
+            log.warn(
+                "Stream with id %d does not exist: %s",
+                stream_id, e)
 
     # The following two methods are the implementation of the context manager
     # protocol.

hyper/http20/stream.py

@@ -368,8 +368,9 @@ def close(self, error_code=None):
         :returns: Nothing.
         """
         # Right now let's not bother with grace, let's just call close on the
-        # connection.
-        self._close_cb(self.stream_id, error_code)
+        # connection. If not error code is provided then assume it is a
+        # gracefull shutdown.
+        self._close_cb(self.stream_id, error_code or 0)
 
     def _handle_header_block(self, headers):
         """

test/test_hyper.py

@@ -2,7 +2,7 @@
 from hyper.packages.hyperframe.frame import (
     Frame, DataFrame, RstStreamFrame, SettingsFrame,
     PushPromiseFrame, PingFrame, WindowUpdateFrame, HeadersFrame,
-    ContinuationFrame, BlockedFrame, GoAwayFrame,
+    ContinuationFrame, BlockedFrame, GoAwayFrame, FRAME_MAX_LEN, FRAME_MAX_ALLOWED_LEN
 )
 from hyper.packages.hpack.hpack_compat import Encoder, Decoder
 from hyper.http20.connection import HTTP20Connection
@@ -27,7 +27,7 @@
 import socket
 import zlib
 from io import BytesIO
-
+import hyper
 
 def decode_frame(frame_data):
     f, length = Frame.parse_frame_header(frame_data[:9])
@@ -195,6 +195,7 @@ def test_closed_connections_are_reset(self):
         assert c.decoder is not decoder
         assert c._settings == {
             SettingsFrame.INITIAL_WINDOW_SIZE: 65535,
+            SettingsFrame.SETTINGS_MAX_FRAME_SIZE: FRAME_MAX_LEN,
         }
         assert c._out_flow_control_window == 65535
         assert c.window_manager is not wm
@@ -245,6 +246,71 @@ def test_connections_increment_send_window_properly(self):
 
         assert c._out_flow_control_window == 65535 + 1000
 
+    def test_connections_handle_resizing_max_frame_size_properly(self):
+        sock = DummySocket()
+        f = SettingsFrame(0)
+        f.settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] = 65536 # 2^16
+        c = HTTP20Connection('www.google.com')
+        c._sock = sock
+
+        # 'Receive' the SETTINGS frame.
+        c.receive_frame(f)
+
+        # Confirm that the setting is stored and the max frame size increased.
+        assert c._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] == 65536
+
+        # Confirm we got a SETTINGS ACK.
+        f2 = decode_frame(sock.queue[0])
+        assert isinstance(f2, SettingsFrame)
+        assert f2.stream_id == 0
+        assert f2.flags == set(['ACK'])
+
+    def test_connections_handle_too_small_max_frame_size_properly(self):
+        sock = DummySocket()
+        f = SettingsFrame(0)
+        f.settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] = 1024
+        c = HTTP20Connection('www.google.com')
+        c._sock = sock
+
+        # 'Receive' the SETTINGS frame.
+        with pytest.raises(ConnectionError):
+            c.receive_frame(f)
+
+        # The value advertised by an endpoint MUST be between 2^14 and
+        # 2^24-1 octets. Confirm that the max frame size did not increase.
+        assert c._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] == FRAME_MAX_LEN
+
+        # When the setting containing the max frame size value is out of range,
+        # the spec dictates to tear down the connection.
+        assert c._sock == None
+
+        # Check if GoAway frame was correctly sent to the endpoint
+        f = decode_frame(sock.queue[0])
+        assert isinstance(f, GoAwayFrame)
+
+    def test_connections_handle_too_big_max_frame_size_properly(self):
+        sock = DummySocket()
+        f = SettingsFrame(0)
+        f.settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] = 67108864 # 2^26
+        c = HTTP20Connection('www.google.com')
+        c._sock = sock
+
+        # 'Receive' the SETTINGS frame.
+        with pytest.raises(ConnectionError):
+            c.receive_frame(f)
+
+        # The value advertised by an endpoint MUST be between 2^14 and
+        # 2^24-1 octets. Confirm that the max frame size did not increase.
+        assert c._settings[SettingsFrame.SETTINGS_MAX_FRAME_SIZE] == FRAME_MAX_LEN
+
+        # When the setting containing the max frame size value is out of range,
+        # the spec dictates to tear down the connection.
+        assert c._sock == None
+
+        # Check if GoAway frame was correctly sent to the endpoint
+        f = decode_frame(sock.queue[0])
+        assert isinstance(f, GoAwayFrame)
+
     def test_connections_handle_resizing_header_tables_properly(self):
         sock = DummySocket()
         f = SettingsFrame(0)
@@ -1282,6 +1348,65 @@ def data_callback(frame):
         assert isinstance(f, RstStreamFrame)
         assert f.error_code == 1 # PROTOCOL_ERROR
 
+    def test_connection_sends_rst_frame_if_frame_size_too_large(self):
+        sock = DummySocket()
+        d = DataFrame(1)
+        # Receive oversized frame on the client side.
+        # Create huge data frame that exceeds the FRAME_MAX_LEN value in order
+        # to trigger the reset frame with error code 6 (FRAME_SIZE_ERROR).
+        # FRAME_MAX_LEN is a constant value for the hyper client and cannot 
+        # be updated as of now.
+        d.data = b''.join([b"hi there client" for x in range(40)])
+        sock.buffer = BytesIO(d.serialize())
+
+        frames = []
+
+        def send_rst_frame(stream_id, error_code):
+            f = RstStreamFrame(stream_id)
+            f.error_code = error_code
+            frames.append(f)
+
+        c = HTTP20Connection('www.google.com')
+        c._sock = sock
+        c._send_rst_frame = send_rst_frame
+        c.request('GET', '/')
+        c._recv_cb()
+
+        assert len(frames) == 1
+        f = frames[0]
+        assert isinstance(f, RstStreamFrame)
+        assert f.stream_id == 1
+        assert f.error_code == 6 #FRAME_SIZE_ERROR
+
+    def test_connection_stream_is_removed_when_receiving_out_of_range_frame(self):
+        sock = DummySocket()
+        d = DataFrame(1)
+        d.data = b''.join([b"hi there sir" for x in range(40)])
+        sock.buffer = BytesIO(d.serialize())
+
+        c = HTTP20Connection('www.google.com')
+        c._sock = sock
+        c.request('GET', '/')
+
+        # Make sure the stream gets removed from the map
+        # after receiving an out of size data frame
+        assert len(c.streams) == 1
+        c._recv_cb()
+        assert len(c.streams) == 0
+
+    def test_connection_error_when_send_out_of_range_frame(self):
+        # Send oversized frame to the server side.
+        # Create huge data frame that exceeds the intitial FRAME_MAX_LEN setting
+        # in order to trigger a value error when sending it.
+        # Note that the value of the FRAME_MAX_LEN setting can be updated
+        # by the server through a settings frame.
+        d = DataFrame(1)
+        d.data = b''.join([b"hi there server" for x in range(1500)])
+
+        c = HTTP20Connection('www.google.com')
+        c._sock = DummySocket()
+        with pytest.raises(ValueError):
+            c._send_cb(d)
 
 # Some utility classes for the tests.
 class NullEncoder(object):