Skip to content

Commit eba88a7

Browse files
committed
frozen_dataclass(docs/test): Enhance documentation and test precision
why: Improve clarity, security awareness, and test robustness what: - Simplify doctests with conventional exception handling patterns - Add explicit example of using immutable collections for deep immutability - Enhance security warnings about _frozen flag bypass vulnerability - Use precise regex patterns in test assertions for better error detection - Improve documentation of design choices and implementation limitations security: Explicitly document the _frozen attribute vulnerability with clear examples and suggested mitigations
1 parent 6513c74 commit eba88a7

File tree

2 files changed

+38
-29
lines changed

2 files changed

+38
-29
lines changed

src/libtmux/_internal/frozen_dataclass.py

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,10 @@ def frozen_dataclass(cls: type[_T]) -> type[_T]:
4747
>>> user = User(id=1, name="Alice")
4848
>>> user.name
4949
'Alice'
50-
>>> try:
51-
... user.name = "Bob"
52-
... except AttributeError as e:
53-
... print(f"Error: {e}")
54-
Error: User is immutable: cannot modify field 'name'
50+
>>> user.name = "Bob"
51+
Traceback (most recent call last):
52+
...
53+
AttributeError: User is immutable: cannot modify field 'name'
5554
5655
Mutating internal attributes (_-prefixed):
5756
@@ -68,6 +67,13 @@ def frozen_dataclass(cls: type[_T]) -> type[_T]:
6867
>>> c.items.append(3) # allowed; mutable field itself isn't protected
6968
>>> c.items
7069
[1, 2, 3]
70+
>>> # For deep immutability, use immutable collections (tuple, frozenset)
71+
>>> @frozen_dataclass
72+
... class ImmutableContainer:
73+
... items: tuple[int, ...] = (1, 2)
74+
>>> ic = ImmutableContainer()
75+
>>> ic.items
76+
(1, 2)
7177
7278
Inheritance from mutable base classes:
7379
@@ -81,26 +87,25 @@ def frozen_dataclass(cls: type[_T]) -> type[_T]:
8187
>>> obj = ImmutableSub(42)
8288
>>> obj.value
8389
42
84-
>>> try:
85-
... obj.value = 100
86-
... except AttributeError as e:
87-
... print(f"Error: {e}")
88-
Error: ImmutableSub is immutable: cannot modify field 'value'
90+
>>> obj.value = 100
91+
Traceback (most recent call last):
92+
...
93+
AttributeError: ImmutableSub is immutable: cannot modify field 'value'
8994
9095
Security consideration - modifying the _frozen flag:
9196
9297
>>> @frozen_dataclass
9398
... class SecureData:
9499
... secret: str
95100
>>> data = SecureData(secret="password123")
96-
>>> try:
97-
... data.secret = "hacked"
98-
... except AttributeError as e:
99-
... print(f"Protected: {e}")
100-
Protected: SecureData is immutable: cannot modify field 'secret'
101-
>>> # However, the _frozen flag can be modified to bypass protection:
102-
>>> data._frozen = False
103101
>>> data.secret = "hacked"
102+
Traceback (most recent call last):
103+
...
104+
AttributeError: SecureData is immutable: cannot modify field 'secret'
105+
>>> # CAUTION: The _frozen attribute can be modified to bypass immutability protection
106+
>>> # This is a known limitation of this implementation
107+
>>> data._frozen = False # intentionally bypassing immutability
108+
>>> data.secret = "hacked" # now works because object is no longer frozen
104109
>>> data.secret
105110
'hacked'
106111
"""

tests/_internal/test_frozen_dataclass.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,22 +80,20 @@ def test_immutability() -> None:
8080
pane_id="pane123", width=80, height=24, captured_content=["Line1"]
8181
)
8282

83-
# Attempting to modify a field should raise AttributeError
84-
with pytest.raises(AttributeError, match="immutable.*cannot modify field 'width'"):
83+
# Attempting to modify a field should raise AttributeError with precise error message
84+
with pytest.raises(AttributeError, match=r"PaneSnapshot is immutable: cannot modify field 'width'"):
8585
snapshot.width = 200 # type: ignore
8686

87-
# Attempting to add a new field should raise AttributeError
88-
with pytest.raises(
89-
AttributeError, match="immutable.*cannot modify field 'new_field'"
90-
):
87+
# Attempting to add a new field should raise AttributeError with precise error message
88+
with pytest.raises(AttributeError, match=r"PaneSnapshot is immutable: cannot modify field 'new_field'"):
9189
snapshot.new_field = "value" # type: ignore
9290

93-
# Attempting to delete a field should raise AttributeError
94-
with pytest.raises(AttributeError, match="immutable.*cannot delete field 'width'"):
91+
# Attempting to delete a field should raise AttributeError with precise error message
92+
with pytest.raises(AttributeError, match=r"PaneSnapshot is immutable: cannot delete field 'width'"):
9593
del snapshot.width
9694

9795
# Calling a method that tries to modify state should fail
98-
with pytest.raises(NotImplementedError, match="immutable"):
96+
with pytest.raises(NotImplementedError, match=r"Snapshot is immutable. resize\(\) not allowed."):
9997
snapshot.resize(200, 50)
10098

10199

@@ -189,7 +187,12 @@ def test_bidirectional_references() -> None:
189187

190188

191189
class DimensionTestCase(t.NamedTuple):
192-
"""Test fixture for validating dimensions in PaneSnapshot."""
190+
"""Test fixture for validating dimensions in PaneSnapshot.
191+
192+
Note: This implementation intentionally allows any dimension values, including
193+
negative or extremely large values. In a real-world application, you might want
194+
to add validation to the class constructor if certain dimension ranges are required.
195+
"""
193196

194197
test_id: str
195198
width: int
@@ -284,11 +287,12 @@ def test_frozen_flag(
284287
error_match: str | None,
285288
) -> None:
286289
"""Test behavior when attempting to manipulate the _frozen flag.
287-
290+
288291
Note: We discovered that setting _frozen=False actually allows mutation,
289292
which could be a potential security issue if users know about this behavior.
290293
In a more secure implementation, the _frozen attribute might need additional
291-
protection to prevent this bypass mechanism.
294+
protection to prevent this bypass mechanism, such as making it a property with
295+
a setter that raises an exception.
292296
"""
293297
# Create a frozen dataclass
294298
pane = PaneSnapshot(pane_id="test_frozen", width=80, height=24)

0 commit comments

Comments
 (0)