Repository: postlund/pyatv
Pull request #2033 (2023-06-07 06:49:36)

Fix intermittent companion proxy connection issues
Companion protocol appears to bee sensitive to the byte-size of opack encoded integers. Previously proxy connection could sometimes fail if the size of the response "_x" field did not match the size of the request "_x" field.

This PR preserves  integer sizes when decoding/re-encoding with opack

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/postlund/pyatv/pull/2033?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) Report
Patch coverage: **`100.00`**% and project coverage change: **`-0.13`** :warning:
> Comparison is base [(`d508f35`)](https://app.codecov.io/gh/postlund/pyatv/commit/d508f35d98a1af411877db5dd483e433e208a0ac?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) 87.69% compared to head [(`5bd80df`)](https://app.codecov.io/gh/postlund/pyatv/pull/2033?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) 87.57%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2033      +/-   ##
==========================================
- Coverage   87.69%   87.57%   -0.13%     
==========================================
  Files         155      155              
  Lines        9977     9985       +8     
==========================================
- Hits         8749     8744       -5     
- Misses       1228     1241      +13     
```


| [Impacted Files](https://app.codecov.io/gh/postlund/pyatv/pull/2033?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) | Coverage Δ | |
|---|---|---|
| [pyatv/support/opack.py](https://app.codecov.io/gh/postlund/pyatv/pull/2033?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvc3VwcG9ydC9vcGFjay5weQ==) | `92.81% <100.00%> (+0.36%)` | :arrow_up: |

... and [4 files with indirect coverage changes](https://app.codecov.io/gh/postlund/pyatv/pull/2033/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/postlund/pyatv/pull/2033?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).

postlund: Peculiar behavior on Apples part, but OK!

Patch:
diff --git a/docs/documentation/atvproxy.md b/docs/documentation/atvproxy.md
index fad48c62..704792bc 100644
--- a/docs/documentation/atvproxy.md
+++ b/docs/documentation/atvproxy.md
@@ -93,13 +93,7 @@ work and behave as expected and all traffic should be logged to console.
 
 # Companion Proxy
 
-There is also support for the Companion protocol. Occasionally iOS devices will
-fail to complete the connection ("Connecting" visible in the remote app despite
-seeing initialization traffic in the proxy), possibly due to some kind of IP
-address & port or identifier caching. In such situations rebooting the iOS
-device, restarting the proxy, restarting the Apple TV Remote app (in newer iOS
-versions), or a combination of these has been known to bring it back to a
-working state.
+There is also support for the Companion protocol.
 
 ## Device Credentials
 
diff --git a/pyatv/support/opack.py b/pyatv/support/opack.py
index c4226b27..e0768a8c 100644
--- a/pyatv/support/opack.py
+++ b/pyatv/support/opack.py
@@ -9,9 +9,25 @@ from datetime import datetime
 
 # pylint: disable=too-many-branches,too-many-return-statements,too-many-statements
 import struct
-from typing import Tuple
+from typing import Dict, Tuple, Type
 from uuid import UUID
 
+_SIZED_INT_TYPES: Dict[int, Type] = {}
+
+
+def _sized_int(value: int, size: int) -> int:
+    """Return an int subclass with a size attribute.
+
+    This preserves the original encoded size, and allows re-encoding to the same
+    size.
+    """
+    if size in _SIZED_INT_TYPES:
+        type_ = _SIZED_INT_TYPES[size]
+    else:
+        type_ = type(f"int_{size}b", (int,), {"size": size})
+        _SIZED_INT_TYPES[size] = type_
+    return type_(value)
+
 
 def pack(data: object) -> bytes:
     """Pack data structure with OPACK and return bytes."""
@@ -29,13 +45,14 @@ def _pack(data, object_list):
     elif isinstance(data, datetime):
         raise NotImplementedError("absolute time")
     elif isinstance(data, int):
-        if data < 0x28:
+        size_hint = getattr(data, "size", None)  # if created with _sized_int()
+        if data < 0x28 and not size_hint:
             packed_bytes = bytes([data + 8])
-        elif data <= 0xFF:
+        elif (data <= 0xFF and not size_hint) or size_hint == 1:
             packed_bytes = bytes([0x30]) + data.to_bytes(1, byteorder="little")
-        elif data <= 0xFFFF:
+        elif (data <= 0xFFFF and not size_hint) or size_hint == 2:
             packed_bytes = bytes([0x31]) + data.to_bytes(2, byteorder="little")
-        elif data <= 0xFFFFFFFF:
+        elif (data <= 0xFFFFFFFF and not size_hint) or size_hint == 4:
             packed_bytes = bytes([0x32]) + data.to_bytes(4, byteorder="little")
         elif data <= 0xFFFFFFFFFFFFFFFF:
             packed_bytes = bytes([0x33]) + data.to_bytes(8, byteorder="little")
@@ -148,7 +165,9 @@ def _unpack(data, object_list):
     elif (data[0] & 0xF0) == 0x30:
         noof_bytes = 2 ** (data[0] & 0xF)
         value, remaining = (
-            int.from_bytes(data[1 : 1 + noof_bytes], byteorder="little"),
+            _sized_int(
+                int.from_bytes(data[1 : 1 + noof_bytes], byteorder="little"), noof_bytes
+            ),
             data[1 + noof_bytes :],
         )
     elif 0x40 <= data[0] <= 0x60:


Test patch:
diff --git a/tests/support/test_opack.py b/tests/support/test_opack.py
index c7c82f8a..6495374a 100644
--- a/tests/support/test_opack.py
+++ b/tests/support/test_opack.py
@@ -8,7 +8,7 @@ from uuid import UUID
 from deepdiff import DeepDiff
 import pytest
 
-from pyatv.support.opack import pack, unpack
+from pyatv.support.opack import _sized_int, pack, unpack
 
 # pack
 
@@ -52,6 +52,13 @@ def test_pack_larger_integers():
     assert pack(0x1FFFFFFFFFFFFFF) == b"\x33\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x01"
 
 
+def test_pack_sized_integers():
+    assert pack(_sized_int(0x1, 1)) == b"\x30\x01"
+    assert pack(_sized_int(0x1, 2)) == b"\x31\x01\x00"
+    assert pack(_sized_int(0x1, 4)) == b"\x32\x01\x00\x00\x00"
+    assert pack(_sized_int(0x1, 8)) == b"\x33\x01\x00\x00\x00\x00\x00\x00\x00"
+
+
 def test_pack_float64():
     assert pack(1.0) == b"\x36\x00\x00\x00\x00\x00\x00\xF0\x3F"
 
@@ -227,9 +234,16 @@ def test_unpack_small_integers():
 
 def test_unpack_larger_integers():
     assert unpack(b"\x30\x28") == (0x28, b"")
-    assert unpack(b"\x31\xFf\x01") == (0x1FF, b"")
-    assert unpack(b"\x32\xFF\xFF\x01") == (0x1FFFF, b"")
-    assert unpack(b"\x33\xFF\xFF\xFF\x01") == (0x1FFFFFF, b"")
+    assert unpack(b"\x31\xFF\x01") == (0x1FF, b"")
+    assert unpack(b"\x32\xFF\xFF\xFF\x01") == (0x1FFFFFF, b"")
+    assert unpack(b"\x33\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x01") == (0x1FFFFFFFFFFFFFF, b"")
+
+
+def test_unpack_sized_integers():
+    assert getattr(unpack(b"\x30\x01")[0], "size") == 1
+    assert getattr(unpack(b"\x31\x01\x00")[0], "size") == 2
+    assert getattr(unpack(b"\x32\x01\x00\x00\x00")[0], "size") == 4
+    assert getattr(unpack(b"\x33\x01\x00\x00\x00\x00\x00\x00\x00")[0], "size") == 8
 
 
 def test_pack_unfloat32():