Skip to content

Protect state.as_dict from mutation #65693

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
balloob merged 11 commits into from
Feb 4, 2022
Merged

Protect state.as_dict from mutation #65693

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
cb3d52e
Protect state.as_dict from mutation
balloob Feb 4, 2022
6cb6951
Fix type
balloob Feb 4, 2022
ec868f3
Fix react_data typing
balloob Feb 4, 2022
ccf1896
Also redact mappings stored in dicts
balloob Feb 4, 2022
6b2d532
Revert tuya
balloob Feb 4, 2022
9d05e04
Add ReadOnlyDict
balloob Feb 4, 2022
45e18fb
Add test
balloob Feb 4, 2022
4e9037b
improve core test
balloob Feb 4, 2022
f5a8a3b
Cleanup types
balloob Feb 4, 2022
6d3d600
Shelly types
balloob Feb 4, 2022
89fcca9
Fix restore cache mocking
balloob Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions homeassistant/components/homekit_controller/diagnostics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""Diagnostics support for HomeKit Controller."""
from __future__ import annotations

from typing import Any
from typing import Any, cast

from aiohomekit.model.characteristics.characteristic_types import CharacteristicsTypes

Expand Down Expand Up @@ -66,7 +66,7 @@ def _async_get_diagnostics_for_device(
state = hass.states.get(entity_entry.entity_id)
state_dict = None
if state:
state_dict = async_redact_data(state.as_dict(), REDACTED_STATE)
state_dict = cast(dict, async_redact_data(state.as_dict(), REDACTED_STATE))

@Jc2k Jc2k Feb 4, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the typing changes in cc6b0cc are wrong. async_redact_data is returning a dict, not a MappingProxyType, so the cast here is papering over a bug in async_redact_data IMO.

@balloob balloob Feb 4, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async_redact_data will convert any type inheriting from Mapping to a dict. It's typed to have input = output type.

Let me see if I can fix that.

@balloob balloob Feb 4, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial attempt gives me an error.

@overload
def async_redact_data(data: Mapping, to_redact: Iterable[Any]) -> dict:
    ...


@overload
def async_redact_data(data: T, to_redact: Iterable[Any]) -> T:
    ...

homeassistant/components/diagnostics/util.py:15: error: Overloaded function signatures 1 and 2 overlap with incompatible return types [misc]
Found 1 error in 1 file (checked 2 source files)

@balloob balloob Feb 4, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignored that error ec868f3 馃し

state_dict.pop("context", None)

entities.append(
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/tuya/diagnostics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def _async_device_as_dict(hass: HomeAssistant, device: TuyaDevice) -> dict[str,
state = hass.states.get(entity_entry.entity_id)
state_dict = None
if state:
state_dict = state.as_dict()
state_dict = dict(state.as_dict())

# Redact the `entity_picture` attribute as it contains a token.
if "entity_picture" in state_dict["attributes"]:
Expand Down
22 changes: 12 additions & 10 deletions homeassistant/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ def __init__(
self.last_changed = last_changed or self.last_updated
self.context = context or Context()
self.domain, self.object_id = split_entity_id(self.entity_id)
self._as_dict: dict[str, Collection[Any]] | None = None
self._as_dict: MappingProxyType[str, Collection[Any]] | None = None

@property
def name(self) -> str:
Expand All @@ -1063,7 +1063,7 @@ def name(self) -> str:
"_", " "
)

def as_dict(self) -> dict[str, Collection[Any]]:
def as_dict(self) -> MappingProxyType[str, Collection[Any]]:
"""Return a dict representation of the State.

Async friendly.
Expand All @@ -1077,14 +1077,16 @@ def as_dict(self) -> dict[str, Collection[Any]]:
last_updated_isoformat = last_changed_isoformat
else:
last_updated_isoformat = self.last_updated.isoformat()
self._as_dict = {
"entity_id": self.entity_id,
"state": self.state,
"attributes": dict(self.attributes),
"last_changed": last_changed_isoformat,
"last_updated": last_updated_isoformat,
"context": self.context.as_dict(),
}
self._as_dict = MappingProxyType(
{
"entity_id": self.entity_id,
"state": self.state,
"attributes": dict(self.attributes),
"last_changed": last_changed_isoformat,
"last_updated": last_updated_isoformat,
"context": MappingProxyType(self.context.as_dict()),
}
)
return self._as_dict

@classmethod
Expand Down
13 changes: 11 additions & 2 deletions tests/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,19 @@ def test_state_as_dict():
"last_updated": last_time.isoformat(),
"state": "on",
}
assert state.as_dict() == expected
as_dict_1 = state.as_dict()
assert as_dict_1 == expected
# 2nd time to verify cache
assert state.as_dict() == expected
assert state.as_dict() is state.as_dict()
assert state.as_dict() is as_dict_1

# Verify it's immutable
with pytest.raises(AttributeError):
as_dict_1.pop("state")
with pytest.raises(TypeError):
as_dict_1["state"] = "yo"
with pytest.raises(TypeError):
as_dict_1["context"]["user_id"] = None


async def test_eventbus_add_remove_listener(hass):
Expand Down