Skip to content

Commit 54bab66

Browse files
zehnmzehnm
authored
fix: filter log messages may not modify original data (#27)
Fix `filter_log_msg_data` function to not modify passed in message data: - The first WS message is not affected, json to text dump is done before. - But since no deep copy is made on the input data, the original data is still affected. - The original data is usually the internally stored entity. - The modified data is returned if the Remote requests the entity data!
1 parent ae5ca12 commit 54bab66

File tree

4 files changed

+198
-37
lines changed

4 files changed

+198
-37
lines changed

‎.github/workflows/build.yml‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# GitHub Action to run unit tests
2+
---
3+
name: "Unit tests"
4+
5+
on:
6+
push:
7+
paths:
8+
- 'tests/**'
9+
- 'ucapi/**'
10+
- 'requirements.txt'
11+
- 'test-requirements.txt'
12+
- '.github/**/*.yml'
13+
pull_request:
14+
branches: [main]
15+
types: [opened, synchronize, reopened]
16+
17+
permissions:
18+
contents: read
19+
20+
jobs:
21+
test:
22+
runs-on: ubuntu-24.04
23+
steps:
24+
- name: Checkout
25+
uses: actions/checkout@v4
26+
27+
- name: Set up Python
28+
uses: actions/setup-python@v5
29+
with:
30+
python-version: "3.11"
31+
32+
- name: Install pip
33+
run: |
34+
python -m pip install --upgrade pip
35+
36+
- name: Install dependencies
37+
run: |
38+
pip install -r requirements.txt
39+
40+
- name: Unit tests
41+
run: |
42+
python -m unittest discover tests

‎.idea/runConfigurations/Unit_tests.xml‎

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/test_api.py‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import unittest
2+
from copy import deepcopy
3+
4+
from ucapi.api import filter_log_msg_data
5+
from ucapi.media_player import Attributes
6+
7+
8+
class TestFilterLogMsgData(unittest.TestCase):
9+
10+
def test_no_modification_when_no_msg_data(self):
11+
data = {}
12+
result = filter_log_msg_data(data)
13+
self.assertEqual(result, {}, "The result should be an empty dictionary")
14+
15+
def test_no_changes_when_media_image_url_not_present(self):
16+
data = {"msg_data": {"attributes": {"state": "playing", "volume": 50}}}
17+
original = deepcopy(data)
18+
19+
result = filter_log_msg_data(data)
20+
21+
self.assertEqual(
22+
result,
23+
original,
24+
"The result should be the same as the input when no filtered attributes are present",
25+
)
26+
27+
def test_filtering_media_image_url_in_dict(self):
28+
data = {
29+
"msg_data": {
30+
"attributes": {
31+
"state": "playing",
32+
Attributes.MEDIA_IMAGE_URL: "data:image/png;base64,encodeddata",
33+
}
34+
}
35+
}
36+
expected_result = deepcopy(data)
37+
expected_result["msg_data"]["attributes"][
38+
Attributes.MEDIA_IMAGE_URL
39+
] = "data:***"
40+
41+
result = filter_log_msg_data(data)
42+
43+
self.assertEqual(
44+
result, expected_result, "The MEDIA_IMAGE_URL attribute should be filtered"
45+
)
46+
47+
def test_filtering_media_image_url_in_list(self):
48+
data = {
49+
"msg_data": [
50+
{
51+
"attributes": {
52+
"state": "paused",
53+
Attributes.MEDIA_IMAGE_URL: "data:image/png;base64,exampledata",
54+
}
55+
},
56+
{
57+
"attributes": {
58+
"state": "playing",
59+
Attributes.MEDIA_IMAGE_URL: "data:image/jpg;base64,exampledata",
60+
}
61+
},
62+
{"attributes": {"state": "stopped"}},
63+
]
64+
}
65+
expected_result = deepcopy(data)
66+
expected_result["msg_data"][0]["attributes"][
67+
Attributes.MEDIA_IMAGE_URL
68+
] = "data:***"
69+
expected_result["msg_data"][1]["attributes"][
70+
Attributes.MEDIA_IMAGE_URL
71+
] = "data:***"
72+
73+
result = filter_log_msg_data(data)
74+
75+
self.assertEqual(
76+
result,
77+
expected_result,
78+
"All MEDIA_IMAGE_URL attributes in the list should be filtered",
79+
)
80+
81+
def test_input_is_not_modified(self):
82+
data = {
83+
"msg_data": {
84+
"attributes": {
85+
Attributes.MEDIA_IMAGE_URL: "data:image/png;base64,encodeddata"
86+
}
87+
}
88+
}
89+
original_data = deepcopy(data)
90+
91+
filter_log_msg_data(data)
92+
93+
self.assertEqual(
94+
data, original_data, "The input data should not be modified by the function"
95+
)

‎ucapi/api.py‎

Lines changed: 33 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import os
1212
import socket
1313
from asyncio import AbstractEventLoop
14+
from copy import deepcopy
1415
from typing import Any, Callable
1516

1617
import websockets
@@ -25,8 +26,8 @@
2526
from zeroconf.asyncio import AsyncServiceInfo, AsyncZeroconf
2627

2728
import ucapi.api_definitions as uc
28-
from ucapi import media_player
2929
from ucapi.entities import Entities
30+
from ucapi.media_player import Attributes as MediaAttr
3031

3132
try:
3233
from ._version import version as __version__
@@ -249,8 +250,9 @@ async def _send_ws_response(
249250
if websocket in self._clients:
250251
data_dump = json.dumps(data)
251252
if _LOG.isEnabledFor(logging.DEBUG):
252-
data_log = json.dumps(data) if filter_log_msg_data(data) else data_dump
253-
_LOG.debug("[%s] ->: %s", websocket.remote_address, data_log)
253+
_LOG.debug(
254+
"[%s] ->: %s", websocket.remote_address, filter_log_msg_data(data)
255+
)
254256
await websocket.send(data_dump)
255257
else:
256258
_LOG.error("Error sending response: connection no longer established")
@@ -270,14 +272,12 @@ async def _broadcast_ws_event(
270272
"""
271273
data = {"kind": "event", "msg": msg, "msg_data": msg_data, "cat": category}
272274
data_dump = json.dumps(data)
273-
# filter fields
274-
data_log = ""
275-
if _LOG.isEnabledFor(logging.DEBUG):
276-
data_log = json.dumps(data) if filter_log_msg_data(data) else data_dump
277275

278276
for websocket in self._clients:
279277
if _LOG.isEnabledFor(logging.DEBUG):
280-
_LOG.debug("[%s] =>: %s", websocket.remote_address, data_log)
278+
_LOG.debug(
279+
"[%s] =>: %s", websocket.remote_address, filter_log_msg_data(data)
280+
)
281281
try:
282282
await websocket.send(data_dump)
283283
except websockets.exceptions.WebSocketException:
@@ -302,8 +302,9 @@ async def _send_ws_event(
302302

303303
if websocket in self._clients:
304304
if _LOG.isEnabledFor(logging.DEBUG):
305-
data_log = json.dumps(data) if filter_log_msg_data(data) else data_dump
306-
_LOG.debug("[%s] ->: %s", websocket.remote_address, data_log)
305+
_LOG.debug(
306+
"[%s] ->: %s", websocket.remote_address, filter_log_msg_data(data)
307+
)
307308
await websocket.send(data_dump)
308309
else:
309310
_LOG.error("Error sending event: connection no longer established")
@@ -864,47 +865,42 @@ def local_hostname() -> str:
864865
)
865866

866867

867-
def filter_log_msg_data(data: dict[str, Any]) -> bool:
868+
def filter_log_msg_data(data: dict[str, Any]) -> dict[str, Any]:
868869
"""
869870
Filter attribute fields to exclude for log messages in the given msg data dict.
870871
871-
Attention: the dictionary is modified!
872-
873872
- Attributes are filtered in `data["msg_data"]`:
874873
- dict object: key `attributes`
875874
- list object: every list item `attributes`
876875
- Filtered attributes: `MEDIA_IMAGE_URL`
877876
878877
:param data: the message data dict
879-
:return: True if a field was filtered, False otherwise
878+
:return: copy of the message data dict with filtered attributes
880879
"""
880+
# do not modify the original dict
881+
log_upd = deepcopy(data)
882+
if not log_upd:
883+
return {}
884+
881885
# filter out base64 encoded images in the media player's media_image_url attribute
882-
if "msg_data" in data:
886+
if "msg_data" in log_upd:
883887
if (
884-
"attributes" in data["msg_data"]
885-
and media_player.Attributes.MEDIA_IMAGE_URL
886-
in data["msg_data"]["attributes"]
887-
and data["msg_data"]["attributes"][
888-
media_player.Attributes.MEDIA_IMAGE_URL
889-
].startswith("data:")
888+
"attributes" in log_upd["msg_data"]
889+
and MediaAttr.MEDIA_IMAGE_URL in log_upd["msg_data"]["attributes"]
890+
and log_upd["msg_data"]["attributes"][MediaAttr.MEDIA_IMAGE_URL].startswith(
891+
"data:"
892+
)
890893
):
891-
data["msg_data"]["attributes"][
892-
media_player.Attributes.MEDIA_IMAGE_URL
893-
] = "data:***"
894-
return True
895-
896-
if isinstance(data["msg_data"], list):
897-
for item in data["msg_data"]:
894+
log_upd["msg_data"]["attributes"][MediaAttr.MEDIA_IMAGE_URL] = "data:***"
895+
elif isinstance(log_upd["msg_data"], list):
896+
for item in log_upd["msg_data"]:
898897
if (
899898
"attributes" in item
900-
and media_player.Attributes.MEDIA_IMAGE_URL in item["attributes"]
901-
and item["attributes"][
902-
media_player.Attributes.MEDIA_IMAGE_URL
903-
].startswith("data:")
899+
and MediaAttr.MEDIA_IMAGE_URL in item["attributes"]
900+
and item["attributes"][MediaAttr.MEDIA_IMAGE_URL].startswith(
901+
"data:"
902+
)
904903
):
905-
item["attributes"][
906-
media_player.Attributes.MEDIA_IMAGE_URL
907-
] = "data:***"
908-
return True
904+
item["attributes"][MediaAttr.MEDIA_IMAGE_URL] = "data:***"
909905

910-
return False
906+
return log_upd

0 commit comments

Comments
 (0)