fix(protocol): return only the error body when a Block2 transfer fails

smartthings_local/protocol/coap.py

@@ -490,15 +490,19 @@ def add_response(self, message):
         if message.code == 0:
             raise BlockwiseError()
 
-        # A non-success response terminates the logical GET immediately while
-        # preserving the connected-session contract of returning bytes already
-        # accumulated before and in the error response.
+        # A non-success response terminates the logical GET. Its body is a
+        # diagnostic in the server's own format rather than a continuation of
+        # the representation, so it replaces whatever blocks arrived instead
+        # of extending them: get() returns (code, payload) with no boundary
+        # marker between the two, so concatenating leaves the caller one
+        # buffer holding two content types and no way to split it. Callers
+        # gate on 2.05 before decoding, so the accumulated bytes have no
+        # reader once the code is not 2.xx.
         if message.code >> 5 != 2:
-            if len(self._payload) + len(message.payload) > \
-                    self._max_payload_bytes:
+            if len(message.payload) > self._max_payload_bytes:
                 raise BlockwiseError()
             self._code = message.code
-            self._payload.extend(message.payload)
+            self._payload = bytearray(message.payload)
             self._blocks_received += 1
             self._complete = True
             return BLOCK2_COMPLETE

tests/test_coap_wire.py

@@ -434,7 +434,7 @@ def test_block2_continuation_requires_an_explicit_block2_option():
         ))
 
 
-def test_mid_transfer_error_preserves_connected_session_contract_by_default():
+def test_mid_transfer_error_replaces_the_partial_representation():
     error = _message(
         code=0x80,
         payload=b'error',
@@ -450,7 +450,20 @@ def test_mid_transfer_error_preserves_connected_session_contract_by_default():
     ))
     assert accumulator.add_response(error) == BLOCK2_COMPLETE
     assert accumulator.code == 0x80
-    assert accumulator.payload == b'a' * 16 + b'error'
+    assert accumulator.payload == b'error'
+
+
+def test_error_before_any_block_returns_the_diagnostic_body():
+    accumulator = Block2Accumulator(b'token')
+    assert accumulator.add_response(_message(
+        code=0x80,
+        payload=b'error',
+        include_block=False,
+        etag=None,
+        content_format=None,
+    )) == BLOCK2_COMPLETE
+    assert accumulator.code == 0x80
+    assert accumulator.payload == b'error'
 
 
 def test_block2_accumulator_enforces_exact_block_and_payload_bounds():
@@ -499,6 +512,34 @@ def test_block2_accumulator_enforces_exact_block_and_payload_bounds():
             content_format=None,
         ))
 
+    # An error body is bounded on its own length, since it replaces the
+    # accumulated blocks rather than extending them: a diagnostic that fits
+    # is accepted however many bytes arrived before it.
+    after_blocks = Block2Accumulator(b'token', max_payload_bytes=16)
+    after_blocks.add_response(_message(
+        number=0,
+        more=True,
+        payload=b'a' * 16,
+    ))
+    assert after_blocks.add_response(_message(
+        code=0x80,
+        payload=b'x' * 16,
+        include_block=False,
+        etag=None,
+        content_format=None,
+    )) == BLOCK2_COMPLETE
+    assert after_blocks.payload == b'x' * 16
+
+    oversized_error = Block2Accumulator(b'token', max_payload_bytes=16)
+    with pytest.raises(BlockwiseError):
+        oversized_error.add_response(_message(
+            code=0x80,
+            payload=b'x' * 17,
+            include_block=False,
+            etag=None,
+            content_format=None,
+        ))
+
 
 def test_block2_accumulator_default_payload_bound_is_exactly_64_kib():
     exact = Block2Accumulator(b'token')

tests/test_dtls_session_reader_death.py

@@ -500,7 +500,7 @@ def wait_for_block(_event, per_wait):
     assert sess._pending_mids == {}
 
 
-def test_get_preserves_mid_transfer_error_payload_contract():
+def test_get_returns_only_the_error_body_when_a_transfer_fails_mid_way():
     sess = _make_session()
 
     def respond(datagram):
@@ -527,7 +527,7 @@ def respond(datagram):
 
     sess._send_dgram = respond
     sess.pace = lambda: None
-    assert sess.get(['oic', 'res']) == (0x80, b'a' * 16 + b'error')
+    assert sess.get(['oic', 'res']) == (0x80, b'error')
 
 
 @pytest.mark.parametrize('method', ('get', 'post'))