SETTINGS: fix mutual exclusion of ACK and settings body

Author: Kriechi

HISTORY.rst

@@ -15,6 +15,7 @@ Release History
 - Fixed promised stream id parsing for ``PushPromiseFrame``.
 - Fixed unchecked frame length for ``WindowUpdateFrame``. It now correctly raises ``InvalidFrameError``.
 - Fixed window increment value range validation. It must be 1 <= increment <= 2^31-1.
+- Fixed parsing of ``SettingsFrame`` with mutual exclusion of ACK flag and payload.
 
 **Other Changes**
 

hyperframe/frame.py

@@ -425,6 +425,12 @@ def serialize_body(self):
                          for setting, value in self.settings.items()])
 
     def parse_body(self, data):
+        if 'ACK' in self.flags and len(data) > 0:
+            raise InvalidFrameError(
+                "SETTINGS ack frame must not have payload: got %s bytes" %
+                len(data)
+            )
+
         body_len = 0
         for i in range(0, len(data), 6):
             try:

test/test_frames.py

@@ -361,14 +361,26 @@ def test_settings_frame_ack_and_settings(self):
         with pytest.raises(ValueError):
             SettingsFrame(settings=self.settings, flags=('ACK',))
 
+        with pytest.raises(ValueError):
+            decode_frame(self.serialized)
+
     def test_settings_frame_parses_properly(self):
-        f = decode_frame(self.serialized)
+        # unset the ACK flag to allow correct parsing
+        data = self.serialized[:4] + b"\x00" + self.serialized[5:]
+
+        f = decode_frame(data)
 
         assert isinstance(f, SettingsFrame)
-        assert f.flags == set(['ACK'])
+        assert f.flags == set()
         assert f.settings == self.settings
         assert f.body_len == 42
 
+    def test_settings_frame_invalid_body_length(self):
+        with pytest.raises(ValueError):
+            decode_frame(
+                b'\x00\x00\x2A\x04\x00\x00\x00\x00\x00\xFF\xFF\xFF\xFF'
+            )
+
     def test_settings_frames_never_have_streams(self):
         with pytest.raises(ValueError):
             SettingsFrame(stream_id=1)