Skip to content

Commit 218a8d0

Browse files
fix: defensive-copy column_names and column_types in RowsResponse
RowsResponse used to store caller-supplied column_names and column_types lists by reference, creating silent aliasing: 1. decode_body and decode_rows_continuation stored column_types via `if not column_types: column_types = types` where `types` was ALSO stored as all_row_types[0]. The returned response therefore had `r.column_types is r.row_types[0] == True`, so `r.column_types.append(...)` silently rewrote the first row's type list. 2. MessageDecoder.decode_continuation passed the caller's column_names list directly into every continuation RowsResponse. A caller who sorted or mutated `msg.column_names` on one continuation silently mutated every sibling continuation AND the original response. 3. User code constructing RowsResponse with lists they intend to keep mutating elsewhere would see those mutations reflected in the returned object. Add __post_init__ to RowsResponse that shallow-copies column_names and column_types. Catches all three sites uniformly with one code change and survives future construction sites. Cost: two list allocations per response, negligible compared to the row payload. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 94da6ad commit 218a8d0

2 files changed

Lines changed: 107 additions & 0 deletions

File tree

src/dqlitewire/messages/responses.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,31 @@ class RowsResponse(Message):
219219
rows: list[list[Any]] = field(default_factory=list)
220220
has_more: bool = False
221221

222+
def __post_init__(self) -> None:
223+
# Defensive copies (issue 042). Three sources of aliasing
224+
# motivate this:
225+
#
226+
# 1. ``decode_body`` / ``decode_rows_continuation`` store
227+
# ``column_types = types`` where ``types`` is also stored
228+
# as ``all_row_types[0]``, so without a copy
229+
# ``self.column_types is self.row_types[0]`` and mutating
230+
# one silently rewrites the other.
231+
#
232+
# 2. ``MessageDecoder.decode_continuation`` and
233+
# ``decode_rows_continuation`` pass the caller's
234+
# ``column_names`` list by reference into every
235+
# continuation response; any mutation on one response
236+
# propagates to every sibling.
237+
#
238+
# 3. User code constructing ``RowsResponse`` directly with a
239+
# list they intend to keep mutating elsewhere.
240+
#
241+
# Copying here catches all three sites uniformly and survives
242+
# future construction sites. The cost is two list allocations
243+
# per response — negligible compared to the row payload.
244+
self.column_names = list(self.column_names)
245+
self.column_types = list(self.column_types)
246+
222247
def _get_row_types(self, row_idx: int, row: list[Any]) -> list[ValueType]:
223248
"""Get types for a row: from row_types, column_types, or inferred."""
224249
if self.row_types and row_idx < len(self.row_types):

tests/test_messages_responses.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,88 @@ def test_zero_values(self) -> None:
150150
assert decoded.rows_affected == 0
151151

152152

153+
class TestRowsResponseAliasing:
154+
"""Regression tests for issue 042.
155+
156+
``RowsResponse`` used to store caller-supplied ``column_names`` and
157+
``column_types`` lists by reference, creating two kinds of silent
158+
aliasing:
159+
160+
1. ``column_types`` was stored as ``all_row_types[0]`` inside
161+
``decode_body`` / ``decode_rows_continuation`` via
162+
``if not column_types: column_types = types``. The returned
163+
object then had ``r.column_types is r.row_types[0]``, so
164+
mutating one list silently rewrote the other.
165+
2. ``column_names`` was passed through ``decode_rows_continuation``
166+
unchanged and attached directly to every continuation
167+
``RowsResponse``. Callers who mutated ``msg.column_names`` on
168+
one continuation silently mutated every sibling continuation
169+
AND the original response.
170+
171+
The fix copies both lists on construction via ``__post_init__``.
172+
"""
173+
174+
def test_column_types_is_not_aliased_to_row_types_first(self) -> None:
175+
"""After decoding, ``column_types`` must be a distinct list
176+
object from ``row_types[0]`` so mutation of one does not
177+
propagate to the other.
178+
"""
179+
msg = RowsResponse(
180+
column_names=["id", "name"],
181+
column_types=[ValueType.INTEGER, ValueType.TEXT],
182+
rows=[[1, "alice"], [2, "bob"]],
183+
has_more=False,
184+
)
185+
encoded = msg.encode()
186+
decoded = RowsResponse.decode_body(encoded[HEADER_SIZE:])
187+
188+
assert decoded.row_types # sanity: rows were decoded with types
189+
assert decoded.column_types is not decoded.row_types[0], (
190+
"column_types must not share identity with row_types[0]"
191+
)
192+
193+
# Prove independence by mutation.
194+
decoded.column_types[0] = ValueType.NULL
195+
assert decoded.row_types[0][0] != ValueType.NULL
196+
197+
def test_column_names_defensive_copy_on_construct(self) -> None:
198+
"""A caller who supplies ``column_names`` to RowsResponse and
199+
later mutates the source list must not see the change
200+
reflected in the returned object.
201+
"""
202+
names_in = ["a", "b", "c"]
203+
msg = RowsResponse(
204+
column_names=names_in,
205+
column_types=[ValueType.INTEGER, ValueType.TEXT, ValueType.BLOB],
206+
row_types=[],
207+
rows=[],
208+
has_more=False,
209+
)
210+
211+
names_in.append("d")
212+
213+
assert msg.column_names == ["a", "b", "c"], (
214+
"RowsResponse.column_names must not alias the caller's list"
215+
)
216+
217+
def test_column_types_defensive_copy_on_construct(self) -> None:
218+
"""Same for column_types passed into the constructor."""
219+
types_in = [ValueType.INTEGER, ValueType.TEXT]
220+
msg = RowsResponse(
221+
column_names=["a", "b"],
222+
column_types=types_in,
223+
row_types=[],
224+
rows=[],
225+
has_more=False,
226+
)
227+
228+
types_in[0] = ValueType.BLOB
229+
230+
assert msg.column_types[0] == ValueType.INTEGER, (
231+
"RowsResponse.column_types must not alias the caller's list"
232+
)
233+
234+
153235
class TestRowsResponse:
154236
def test_empty_result(self) -> None:
155237
msg = RowsResponse(

0 commit comments

Comments
 (0)