Skip to content

Commit 48a8a06

Browse files
authored
fix: make Requirements/Markers pickle-safe (#1171)
* fix: make Requirements/Markers pickle-safe Signed-off-by: Henry Schreiner <henryfs@princeton.edu> * tests: restore full coverage Assisted-by: OpenCode:Qwen3.6-27B Signed-off-by: Henry Schreiner <henryfs@princeton.edu> * fix: address copilot review Assisted-by: Copilot:Kimi-K2.6 Signed-off-by: Henry Schreiner <henryfs@princeton.edu> * docs: fix mentioned version number Signed-off-by: Henry Schreiner <henryfs@princeton.edu> --------- Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
1 parent 823b44e commit 48a8a06

5 files changed

Lines changed: 371 additions & 1 deletion

File tree

src/packaging/_parser.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,34 @@ def __repr__(self) -> str:
2727
def serialize(self) -> str:
2828
raise NotImplementedError
2929

30+
def __getstate__(self) -> str:
31+
# Return just the value string for compactness and stability.
32+
return self.value
33+
34+
def _restore_value(self, value: object) -> None:
35+
if not isinstance(value, str):
36+
raise TypeError(
37+
f"Cannot restore {self.__class__.__name__} value from {value!r}"
38+
)
39+
self.value = value
40+
41+
def __setstate__(self, state: object) -> None:
42+
if isinstance(state, str):
43+
# New format (26.2+): just the value string.
44+
self._restore_value(state)
45+
return
46+
if isinstance(state, tuple) and len(state) == 2:
47+
# Old format (packaging <= 26.0, __slots__): (None, {slot: value}).
48+
_, slot_dict = state
49+
if isinstance(slot_dict, dict) and "value" in slot_dict:
50+
self._restore_value(slot_dict["value"])
51+
return
52+
if isinstance(state, dict) and "value" in state:
53+
# Old format (packaging <= 25.0, no __slots__): plain __dict__.
54+
self._restore_value(state["value"])
55+
return
56+
raise TypeError(f"Cannot restore {self.__class__.__name__} from {state!r}")
57+
3058

3159
class Variable(Node):
3260
__slots__ = ()

src/packaging/markers.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,35 @@ def __eq__(self, other: object) -> bool:
381381

382382
return str(self) == str(other)
383383

384+
def __getstate__(self) -> str:
385+
# Return the marker expression string for compactness and stability.
386+
# Internal Node objects are excluded; the string is re-parsed on load.
387+
return str(self)
388+
389+
def __setstate__(self, state: object) -> None:
390+
if isinstance(state, str):
391+
# New format (26.2+): just the marker expression string.
392+
try:
393+
self._markers = _normalize_extra_values(_parse_marker(state))
394+
except ParserSyntaxError as exc:
395+
raise TypeError(f"Cannot restore Marker from {state!r}") from exc
396+
return
397+
if isinstance(state, dict) and "_markers" in state:
398+
# Old format (packaging <= 26.1, no __slots__): plain __dict__.
399+
markers = state["_markers"]
400+
if isinstance(markers, list):
401+
self._markers = markers
402+
return
403+
if isinstance(state, tuple) and len(state) == 2:
404+
# Old format (packaging <= 26.1, __slots__): (None, {slot: value}).
405+
_, slot_dict = state
406+
if isinstance(slot_dict, dict) and "_markers" in slot_dict:
407+
markers = slot_dict["_markers"]
408+
if isinstance(markers, list):
409+
self._markers = markers
410+
return
411+
raise TypeError(f"Cannot restore Marker from {state!r}")
412+
384413
def __and__(self, other: Marker) -> Marker:
385414
if not isinstance(other, Marker):
386415
return NotImplemented

src/packaging/requirements.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,30 @@ def _iter_parts(self, name: str) -> Iterator[str]:
7373
if self.marker:
7474
yield f"; {self.marker}"
7575

76+
def __getstate__(self) -> str:
77+
# Return the requirement string for compactness and stability.
78+
# Re-parsed on load to reconstruct all fields.
79+
return str(self)
80+
81+
def __setstate__(self, state: object) -> None:
82+
if isinstance(state, str):
83+
# New format (26.2+): just the requirement string.
84+
try:
85+
tmp = Requirement(state)
86+
except InvalidRequirement as exc:
87+
raise TypeError(f"Cannot restore Requirement from {state!r}") from exc
88+
self.name = tmp.name
89+
self.url = tmp.url
90+
self.extras = tmp.extras
91+
self.specifier = tmp.specifier
92+
self.marker = tmp.marker
93+
return
94+
if isinstance(state, dict):
95+
# Old format (packaging <= 26.1, no __slots__): plain __dict__.
96+
self.__dict__.update(state)
97+
return
98+
raise TypeError(f"Cannot restore Requirement from {state!r}")
99+
76100
def __str__(self) -> str:
77101
return "".join(self._iter_parts(self.name))
78102

tests/test_markers.py

Lines changed: 156 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,15 @@
66

77
import itertools
88
import os
9+
import pickle
910
import platform
1011
import sys
1112
from typing import Any, NamedTuple, cast
1213
from unittest import mock
1314

1415
import pytest
1516

16-
from packaging._parser import Node
17+
from packaging._parser import Node, Op, Value, Variable
1718
from packaging.markers import (
1819
InvalidMarker,
1920
Marker,
@@ -564,3 +565,157 @@ def test_evaluation_of_combined_markers() -> None:
564565
& Marker('platform_system == "Linux"')
565566
)
566567
assert m.evaluate(env) is True
568+
569+
570+
@pytest.mark.parametrize(
571+
"marker_str",
572+
[
573+
'python_version >= "3.8"',
574+
'python_version >= "3.8" and os_name == "posix"',
575+
'python_version >= "3.8" or platform_system == "Windows"',
576+
'extra == "security"',
577+
],
578+
)
579+
def test_pickle_marker_roundtrip(marker_str: str) -> None:
580+
# Make sure equality and str() work between a pickle/unpickle round trip.
581+
m = Marker(marker_str)
582+
loaded = pickle.loads(pickle.dumps(m))
583+
assert loaded == m
584+
assert str(loaded) == str(m)
585+
586+
587+
def test_pickle_marker_setstate_rejects_invalid_state() -> None:
588+
# Cover the TypeError branches in __setstate__ for invalid input.
589+
m = Marker.__new__(Marker)
590+
with pytest.raises(TypeError, match="Cannot restore Marker"):
591+
m.__setstate__(12345)
592+
with pytest.raises(TypeError, match="Cannot restore Marker"):
593+
m.__setstate__((1, 2, 3)) # Wrong tuple length
594+
595+
596+
# Pickle bytes generated with packaging==26.1, Python 3.13.1, pickle protocol 2.
597+
# Format: __slots__ (no __getstate__), state is (None, {slot: value}).
598+
_PACKAGING_26_1_PICKLE_MARKER_PYTHON_VERSION_GE_3_8 = (
599+
b"\x80\x02cpackaging.markers\nMarker\nq\x00)\x81q\x01N}q\x02X\x08\x00"
600+
b"\x00\x00_markersq\x03]q\x04cpackaging._parser\nVariable\nq\x05)\x81"
601+
b"q\x06N}q\x07X\x05\x00\x00\x00valueq\x08X\x0e\x00\x00\x00python_vers"
602+
b"ionq\ts\x86q\nbcpackaging._parser\nOp\nq\x0b)\x81q\x0cN}q\rh\x08X\x02"
603+
b"\x00\x00\x00>=q\x0es\x86q\x0fbcpackaging._parser\nValue\nq\x10)\x81q"
604+
b"\x11N}q\x12h\x08X\x03\x00\x00\x003.8q\x13s\x86q\x14b\x87q\x15as\x86"
605+
b"q\x16b."
606+
)
607+
608+
609+
# Pickle bytes generated with packaging==26.0, Python 3.13.1, pickle protocol 2.
610+
# Format: __slots__ (no __getstate__), state is plain __dict__.
611+
_PACKAGING_26_0_PICKLE_MARKER_PYTHON_VERSION_GE_3_8 = (
612+
b"\x80\x02cpackaging.markers\nMarker\nq\x00)\x81q\x01}q\x02X\x08\x00\x00"
613+
b"\x00_markersq\x03]q\x04cpackaging._parser\nVariable\nq\x05)\x81q\x06N}"
614+
b"q\x07X\x05\x00\x00\x00valueq\x08X\x0e\x00\x00\x00python_versionq\ts\x86"
615+
b"q\nbcpackaging._parser\nOp\nq\x0b)\x81q\x0cN}q\rh\x08X\x02\x00\x00"
616+
b"\x00>=q\x0es\x86q\x0fbcpackaging._parser\nValue\nq\x10)\x81q\x11N}q\x12"
617+
b"h\x08X\x03\x00\x00\x003.8q\x13s\x86q\x14b\x87q\x15asb."
618+
)
619+
620+
# Format: __slots__ with Node objects using __dict__ format (packaging <= 25.0).
621+
# Now loadable because Node classes have __getstate__/__setstate__.
622+
_PACKAGING_25_0_PICKLE_MARKER_PYTHON_VERSION_GE_3_8 = (
623+
b"\x80\x02cpackaging.markers\nMarker\nq\x00)\x81q\x01}q\x02X\x08\x00\x00"
624+
b"\x00_markersq\x03]q\x04cpackaging._parser\nVariable\nq\x05)\x81q\x06}q\x07"
625+
b"X\x05\x00\x00\x00valueq\x08X\x0e\x00\x00\x00python_versionq\tsbcpackaging"
626+
b"._parser\nOp\nq\n)\x81q\x0b}q\x0ch\x08X\x02\x00\x00\x00>=q\rsbcpackaging"
627+
b"._parser\nValue\nq\x0e)\x81q\x0f}q\x10h\x08X\x03\x00\x00\x003.8q\x11sb\x87"
628+
b"q\x12asb."
629+
)
630+
631+
632+
def test_pickle_marker_old_format_loads() -> None:
633+
# Verify that Marker pickles created with packaging <= 26.1 (__slots__,
634+
# no __getstate__) can be loaded and produce correct Marker objects.
635+
m = pickle.loads(_PACKAGING_26_1_PICKLE_MARKER_PYTHON_VERSION_GE_3_8)
636+
assert isinstance(m, Marker)
637+
assert str(m) == 'python_version >= "3.8"'
638+
assert m == Marker('python_version >= "3.8"')
639+
640+
641+
def test_pickle_marker_26_0_format_loads() -> None:
642+
# Verify that Marker pickles created with packaging 26.0 (plain __dict__)
643+
# can be loaded and produce correct Marker objects.
644+
m = pickle.loads(_PACKAGING_26_0_PICKLE_MARKER_PYTHON_VERSION_GE_3_8)
645+
assert isinstance(m, Marker)
646+
assert str(m) == 'python_version >= "3.8"'
647+
assert m == Marker('python_version >= "3.8"')
648+
649+
650+
def test_pickle_marker_25_0_format_loads() -> None:
651+
# Verify that Marker pickles created with packaging 25.0 (with Node __dict__)
652+
# can now be loaded thanks to __getstate__/__setstate__ in Node classes.
653+
m = pickle.loads(_PACKAGING_25_0_PICKLE_MARKER_PYTHON_VERSION_GE_3_8)
654+
assert isinstance(m, Marker)
655+
assert str(m) == 'python_version >= "3.8"'
656+
assert m == Marker('python_version >= "3.8"')
657+
658+
659+
def test_pickle_node_roundtrip() -> None:
660+
# Cover Node.__getstate__ and Node.__setstate__ with the new string format.
661+
for node in (Variable("python_version"), Value("3.8"), Op(">=")):
662+
loaded = pickle.loads(pickle.dumps(node))
663+
assert loaded.value == node.value
664+
assert str(loaded) == str(node)
665+
666+
667+
def test_pickle_node_setstate_rejects_invalid_state() -> None:
668+
# Cover the TypeError branch in Node.__setstate__ for invalid input.
669+
node = Variable.__new__(Variable)
670+
with pytest.raises(TypeError, match="Cannot restore Variable"):
671+
node.__setstate__(12345)
672+
673+
node2 = Variable.__new__(Variable)
674+
with pytest.raises(TypeError, match="Cannot restore Variable"):
675+
node2.__setstate__((1, 2, 3)) # Wrong tuple length
676+
677+
# Cover the legacy tuple branch where slot_dict doesn't have "value".
678+
node3 = Variable.__new__(Variable)
679+
with pytest.raises(TypeError, match="Cannot restore Variable"):
680+
node3.__setstate__((None, {"wrong_key": "foo"}))
681+
682+
# Cover the legacy tuple branch where slot_dict has "value" but it's not a str.
683+
node4 = Variable.__new__(Variable)
684+
with pytest.raises(TypeError, match="Cannot restore Variable value from 123"):
685+
node4.__setstate__((None, {"value": 123}))
686+
687+
# Cover the legacy dict branch where "value" exists but it's not a str.
688+
node5 = Value.__new__(Value)
689+
with pytest.raises(TypeError, match="Cannot restore Value value from 456"):
690+
node5.__setstate__({"value": 456})
691+
692+
# Cover the legacy dict branch on Op (different subclass to ensure coverage).
693+
node6 = Op.__new__(Op)
694+
with pytest.raises(TypeError, match="Cannot restore Op value from 789"):
695+
node6.__setstate__({"value": 789})
696+
697+
698+
def test_pickle_marker_setstate_legacy_slot_dict_without_markers_key() -> None:
699+
# Cover Marker.__setstate__ legacy tuple branch where slot_dict has no "_markers".
700+
m = Marker.__new__(Marker)
701+
with pytest.raises(TypeError, match="Cannot restore Marker"):
702+
m.__setstate__((None, {"other_key": "value"}))
703+
704+
705+
def test_pickle_marker_setstate_rejects_invalid_markers_type() -> None:
706+
# Cover the dict branch where "_markers" exists but is not a list.
707+
m1 = Marker.__new__(Marker)
708+
with pytest.raises(TypeError, match="Cannot restore Marker"):
709+
m1.__setstate__({"_markers": "not a list"})
710+
711+
# Cover the tuple branch where "_markers" exists but is not a list.
712+
m2 = Marker.__new__(Marker)
713+
with pytest.raises(TypeError, match="Cannot restore Marker"):
714+
m2.__setstate__((None, {"_markers": "not a list"}))
715+
716+
717+
def test_pickle_marker_setstate_rejects_invalid_marker_string() -> None:
718+
# Cover the string branch where parsing raises ParserSyntaxError.
719+
m = Marker.__new__(Marker)
720+
with pytest.raises(TypeError, match="Cannot restore Marker"):
721+
m.__setstate__("this is not a valid marker")

0 commit comments

Comments
 (0)