Better guard the parsing logic for malformed data from the SDK (#248)

Author: marcelveldt

matter_server/common/helpers/json.py

@@ -1,5 +1,6 @@
 """Helpers to work with (de)serializing of json."""
 
+from base64 import b64encode
 from dataclasses import is_dataclass
 from typing import Any
 
@@ -30,7 +31,7 @@ def json_encoder_default(obj: Any) -> Any:
     if isinstance(obj, Nullable):
         return None
     if isinstance(obj, bytes):
-        return str(obj)
+        return b64encode(obj).decode("utf-8")
     if isinstance(obj, Exception):
         return str(obj)
     if type(obj) == type:

matter_server/common/helpers/util.py

@@ -2,6 +2,7 @@
 from __future__ import annotations
 
 from base64 import b64decode, b64encode
+import binascii
 from dataclasses import MISSING, asdict, fields, is_dataclass
 from datetime import datetime
 from enum import Enum
@@ -78,7 +79,7 @@ def _convert_value(value: Any) -> Any:
         if isinstance(value, Enum):
             return value.value
         if isinstance(value, bytes):
-            return b64encode(value).decode()
+            return b64encode(value).decode("utf-8")
         if isinstance(value, float32):
             return float(value)
         if type(value) == type:
@@ -115,11 +116,8 @@ def parse_value(name: str, value: Any, value_type: Any, default: Any = MISSING)
         if hasattr(value_type, "from_dict"):
             return value_type.from_dict(value)
         # handle a parse error in the sdk which is returned as:
-        # {'TLVValue': None, 'Reason': None}
-        if (
-            value.get("TLVValue", MISSING) is None
-            and value.get("Reason", MISSING) is None
-        ):
+        # {'TLVValue': None, 'Reason': None} or {'TLVValue': None}
+        if value.get("TLVValue", MISSING) is None:
             if value_type in (None, Nullable, Any):
                 return None
             value = None
@@ -132,13 +130,14 @@ def parse_value(name: str, value: Any, value_type: Any, default: Any = MISSING)
         return None
     if is_dataclass(value_type) and isinstance(value, dict):
         return dataclass_from_dict(value_type, value)
-    origin = get_origin(value_type)
-    if origin is list and isinstance(value, list):
-        return [
+    # get origin value type and inspect one-by-one
+    origin: Any = get_origin(value_type)
+    if origin in (list, tuple) and isinstance(value, list | tuple):
+        return origin(
             parse_value(name, subvalue, get_args(value_type)[0])
             for subvalue in value
             if subvalue is not None
-        ]
+        )
     # handle dictionary where we should inspect all values
     elif origin is dict:
         subkey_type = get_args(value_type)[0]
@@ -175,13 +174,19 @@ def parse_value(name: str, value: Any, value_type: Any, default: Any = MISSING)
         return None
     elif origin is type:
         return get_type_hints(value, globals(), locals())
+    # handle Any as value type (which is basically unprocessable)
     if value_type is Any:
         return value
+    # raise if value is None and the value is required according to annotations
     if value is None and value_type is not NoneType:
         raise KeyError(f"`{name}` of type `{value_type}` is required.")
 
     try:
         if issubclass(value_type, Enum):
+            # handle enums from the SDK that have a value that does not exist in the enum (sigh)
+            if value not in value_type._value2member_map_:
+                # we do not want to crash so we return the raw value
+                return value
             return value_type(value)
         if issubclass(value_type, datetime):
             return parse_utc_timestamp(value)
@@ -194,8 +199,14 @@ def parse_value(name: str, value: Any, value_type: Any, default: Any = MISSING)
         return float(value)
     if value_type is int and isinstance(value, str) and value.isnumeric():
         return int(value)
+    # handle bytes values (sent over the wire as base64 encoded strings)
     if value_type is bytes and isinstance(value, str):
-        return b64decode(value.encode())
+        try:
+            return b64decode(value.encode("utf-8"))
+        except binascii.Error:
+            # unfortunately sometimes the data is malformed
+            # as it is not super important we ignore it (for now)
+            return b""
 
     # Matter SDK specific types
     if value_type is uint and (

tests/common/test_parser.py

@@ -0,0 +1,105 @@
+"""Test parser functions that converts the incoming json from API into dataclass models."""
+import datetime
+from dataclasses import dataclass
+from typing import Optional
+from enum import IntEnum, Enum
+
+import pytest
+
+from matter_server.common.helpers.util import dataclass_from_dict
+
+class MatterIntEnum(IntEnum):
+    """Basic Matter Test IntEnum"""
+    A = 0
+    B = 1
+    C = 2
+
+
+class MatterEnum(Enum):
+    """Basic Matter Test Enum"""
+    A = "a"
+    B = "b"
+    C = "c"
+
+
+@dataclass
+class BasicModelChild:
+    """Basic test model."""
+
+    a: int
+    b: str
+    c: str
+    d: Optional[int]
+
+
+@dataclass
+class BasicModel:
+    """Basic test model."""
+
+    a: int
+    b: float
+    c: str
+    d: Optional[int]
+    e: BasicModelChild
+    f: datetime.datetime
+    g: MatterEnum
+    h: MatterIntEnum
+    i: str = "default"
+
+
+def test_dataclass_from_dict():
+    """Test dataclass from dict parsing."""
+    raw = {
+        "a": 1,
+        "b": 1.0,
+        "c": "hello",
+        "d": 1,
+        "e": {"a": 2, "b": "test", "c": "test", "d": None},
+        "f": "2022-12-09T06:58:00Z",
+        "g": "a",
+        "h": 2
+    }
+    res = dataclass_from_dict(BasicModel, raw)
+    # test the basic values
+    assert isinstance(res, BasicModel)
+    assert res.a == 1
+    assert res.b == 1.0
+    assert res.d == 1
+    # test recursive parsing
+    assert isinstance(res.e, BasicModelChild)
+    # test default value
+    assert res.i == "default"
+    # test int gets converted to float
+    raw["b"] = 2
+    res = dataclass_from_dict(BasicModel, raw)
+    assert res.b == 2.0
+    # test datetime string
+    assert isinstance(res.f, datetime.datetime)
+    assert res.f.month == 12
+    assert res.f.day == 9
+    # test parse (valid) MatterEnum
+    assert res.g == MatterEnum.A
+    # test parse (valid) MatterIntEnum
+    assert res.h == MatterIntEnum.C
+    # test parse invalid enum value returns raw value
+    raw2 = {**raw}
+    raw2["h"] = 5
+    res2 = dataclass_from_dict(BasicModel, raw2)
+    assert res2.h == 5
+    # test string doesn't match int
+    with pytest.raises(TypeError):
+        raw2 = {**raw}
+        raw2["a"] = "blah"
+        dataclass_from_dict(BasicModel, raw2)
+    # test missing key result in keyerror
+    with pytest.raises(KeyError):
+        raw2 = {**raw}
+        del raw2["a"]
+        dataclass_from_dict(BasicModel, raw2)
+    # test extra keys silently ignored in non-strict mode
+    raw2 = {**raw}
+    raw2["extrakey"] = "something"
+    dataclass_from_dict(BasicModel, raw2, strict=False)
+    # test extra keys not silently ignored in strict mode
+    with pytest.raises(KeyError):
+        dataclass_from_dict(BasicModel, raw2, strict=True)