From 858d7e89dd0bb2ebdec01c12fee04688f2f1c6b4 Mon Sep 17 00:00:00 2001 From: akarmakar Date: Wed, 20 May 2026 01:40:14 +0530 Subject: [PATCH 1/4] gh-145856: Fix plistlib.dumps() skipkeys behavior with mixed key types When both skipkeys=True and sort_keys=True were passed to plistlib.dump()/dumps(), the sort ran before the non-string keys were filtered out, raising TypeError on dictionaries with mixed key types. Filter non-string keys before sorting in all three write_dict sites (XML, binary pass 1, binary pass 2). --- Lib/plistlib.py | 23 ++++++------ Lib/test/test_plistlib.py | 36 +++++++++++++------ ...-05-20-14-00-00.gh-issue-145856.Pl5kZx.rst | 4 +++ 3 files changed, 40 insertions(+), 23 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-05-20-14-00-00.gh-issue-145856.Pl5kZx.rst diff --git a/Lib/plistlib.py b/Lib/plistlib.py index 93f3ef5e38af843..2fc584a5df63a66 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -388,15 +388,14 @@ def write_bytes(self, data): def write_dict(self, d): if d: self.begin_element("dict") + items = d.items() + if self._skipkeys: + items = [(k, v) for k, v in items if isinstance(k, str)] if self._sort_keys: - items = sorted(d.items()) - else: - items = d.items() + items = sorted(items) for key, value in items: if not isinstance(key, str): - if self._skipkeys: - continue raise TypeError("keys must be strings") self.simple_element("key", key) self.write_value(value) @@ -719,13 +718,13 @@ def _flatten(self, value): keys = [] values = [] items = value.items() + if self._skipkeys: + items = [(k, v) for k, v in items if isinstance(k, str)] if self._sort_keys: items = sorted(items) for k, v in items: if not isinstance(k, str): - if self._skipkeys: - continue raise TypeError("keys must be strings") keys.append(k) values.append(v) @@ -839,15 +838,15 @@ def _write_object(self, value): elif isinstance(value, (dict, frozendict)): keyRefs, valRefs = [], [] + rootItems = value.items() + if self._skipkeys: + rootItems = [(k, v) for k, v in rootItems + if isinstance(k, str)] if self._sort_keys: - rootItems = sorted(value.items()) - else: - rootItems = value.items() + rootItems = sorted(rootItems) for k, v in rootItems: if not isinstance(k, str): - if self._skipkeys: - continue raise TypeError("keys must be strings") keyRefs.append(self._getrefnum(k)) valRefs.append(self._getrefnum(v)) diff --git a/Lib/test/test_plistlib.py b/Lib/test/test_plistlib.py index b9c261310bb5670..fd6036c033ffb85 100644 --- a/Lib/test/test_plistlib.py +++ b/Lib/test/test_plistlib.py @@ -722,20 +722,34 @@ def test_skipkeys(self): 'snake': 'aWord', } + for fmt in ALL_FORMATS: + for sort_keys in (False, True): + with self.subTest(fmt=fmt, sort_keys=sort_keys): + data = plistlib.dumps( + pl, fmt=fmt, skipkeys=True, sort_keys=sort_keys) + + pl2 = plistlib.loads(data) + self.assertEqual(pl2, {'snake': 'aWord'}) + + fp = BytesIO() + plistlib.dump( + pl, fp, fmt=fmt, skipkeys=True, sort_keys=sort_keys) + data = fp.getvalue() + pl2 = plistlib.loads(fp.getvalue()) + self.assertEqual(pl2, {'snake': 'aWord'}) + + def test_skipkeys_with_sort_keys_mixed_types(self): + # gh-145856: skipkeys=True + sort_keys=True with mixed key types + # used to raise TypeError because the sort ran before the filter. + pl = {1: 'a', 'z': 'b', 'a': 'c'} + for fmt in ALL_FORMATS: with self.subTest(fmt=fmt): data = plistlib.dumps( - pl, fmt=fmt, skipkeys=True, sort_keys=False) - - pl2 = plistlib.loads(data) - self.assertEqual(pl2, {'snake': 'aWord'}) - - fp = BytesIO() - plistlib.dump( - pl, fp, fmt=fmt, skipkeys=True, sort_keys=False) - data = fp.getvalue() - pl2 = plistlib.loads(fp.getvalue()) - self.assertEqual(pl2, {'snake': 'aWord'}) + pl, fmt=fmt, skipkeys=True, sort_keys=True) + pl2 = plistlib.loads(data, dict_type=collections.OrderedDict) + self.assertEqual(dict(pl2), {'z': 'b', 'a': 'c'}) + self.assertEqual(list(pl2.keys()), ['a', 'z']) def test_tuple_members(self): pl = { diff --git a/Misc/NEWS.d/next/Library/2026-05-20-14-00-00.gh-issue-145856.Pl5kZx.rst b/Misc/NEWS.d/next/Library/2026-05-20-14-00-00.gh-issue-145856.Pl5kZx.rst new file mode 100644 index 000000000000000..9f8cc32fd81c88c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-05-20-14-00-00.gh-issue-145856.Pl5kZx.rst @@ -0,0 +1,4 @@ +Fix :func:`plistlib.dumps` and :func:`plistlib.dump` so that ``skipkeys=True`` +together with ``sort_keys=True`` correctly drops non-string keys when the +dictionary contains a mix of string and non-string keys. Previously the sort +ran before the filter and raised :exc:`TypeError`. From bf56b2beee48ecc17d9cd3cee4bf805bfe8dc5e7 Mon Sep 17 00:00:00 2001 From: akarmakar Date: Wed, 20 May 2026 02:30:24 +0530 Subject: [PATCH 2/4] Remove duplicate iteration checking for non-string keys --- Lib/plistlib.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/plistlib.py b/Lib/plistlib.py index 2fc584a5df63a66..1a7c8822d978f35 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -390,13 +390,13 @@ def write_dict(self, d): self.begin_element("dict") items = d.items() if self._skipkeys: + # skip keys that are not strings + # https://github.com/python/cpython/issues/145856 items = [(k, v) for k, v in items if isinstance(k, str)] if self._sort_keys: items = sorted(items) for key, value in items: - if not isinstance(key, str): - raise TypeError("keys must be strings") self.simple_element("key", key) self.write_value(value) self.end_element("dict") @@ -719,13 +719,13 @@ def _flatten(self, value): values = [] items = value.items() if self._skipkeys: + # skip keys that are not strings + # https://github.com/python/cpython/issues/145856 items = [(k, v) for k, v in items if isinstance(k, str)] if self._sort_keys: items = sorted(items) for k, v in items: - if not isinstance(k, str): - raise TypeError("keys must be strings") keys.append(k) values.append(v) @@ -840,14 +840,14 @@ def _write_object(self, value): rootItems = value.items() if self._skipkeys: + # skip keys that are not strings + # https://github.com/python/cpython/issues/145856 rootItems = [(k, v) for k, v in rootItems if isinstance(k, str)] if self._sort_keys: rootItems = sorted(rootItems) for k, v in rootItems: - if not isinstance(k, str): - raise TypeError("keys must be strings") keyRefs.append(self._getrefnum(k)) valRefs.append(self._getrefnum(v)) From 9d970c826c317b96df1cf83c22e22f295143af09 Mon Sep 17 00:00:00 2001 From: akarmakar Date: Wed, 20 May 2026 09:56:09 +0530 Subject: [PATCH 3/4] Revert "Remove duplicate iteration checking for non-string keys" This reverts commit 0fc854521cb90da7cee67350dceb114f8d82f157. --- Lib/plistlib.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Lib/plistlib.py b/Lib/plistlib.py index 1a7c8822d978f35..2fc584a5df63a66 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -390,13 +390,13 @@ def write_dict(self, d): self.begin_element("dict") items = d.items() if self._skipkeys: - # skip keys that are not strings - # https://github.com/python/cpython/issues/145856 items = [(k, v) for k, v in items if isinstance(k, str)] if self._sort_keys: items = sorted(items) for key, value in items: + if not isinstance(key, str): + raise TypeError("keys must be strings") self.simple_element("key", key) self.write_value(value) self.end_element("dict") @@ -719,13 +719,13 @@ def _flatten(self, value): values = [] items = value.items() if self._skipkeys: - # skip keys that are not strings - # https://github.com/python/cpython/issues/145856 items = [(k, v) for k, v in items if isinstance(k, str)] if self._sort_keys: items = sorted(items) for k, v in items: + if not isinstance(k, str): + raise TypeError("keys must be strings") keys.append(k) values.append(v) @@ -840,14 +840,14 @@ def _write_object(self, value): rootItems = value.items() if self._skipkeys: - # skip keys that are not strings - # https://github.com/python/cpython/issues/145856 rootItems = [(k, v) for k, v in rootItems if isinstance(k, str)] if self._sort_keys: rootItems = sorted(rootItems) for k, v in rootItems: + if not isinstance(k, str): + raise TypeError("keys must be strings") keyRefs.append(self._getrefnum(k)) valRefs.append(self._getrefnum(v)) From eeb96fabdd26265e1fea0d8387b89646db1d8684 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 5 Sep 2026 17:58:16 +0300 Subject: [PATCH 4/4] Only copy the items if they are sorted, and check the keys before sorting The three writers now use a common helper. It builds a list only if the items are sorted, and if skipkeys is false it checks the type of keys before sorting, so that a dict with mixed keys is reported as "keys must be strings" instead of failing to compare the keys. Co-Authored-By: Claude Opus 5 (1M context) --- Lib/plistlib.py | 47 ++++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/Lib/plistlib.py b/Lib/plistlib.py index 2fc584a5df63a66..822fbcf17b16d08 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -159,6 +159,25 @@ def _date_to_string(d, aware_datetime): d.hour, d.minute, d.second ) +def _dict_items(d, sort_keys, skipkeys): + """Return the (key, value) pairs of a dict, sorted if needed. + + Sorting fails for keys of different types, so non-string keys are + removed or reported before sorting. + """ + items = d.items() + if sort_keys: + if skipkeys: + items = [item for item in items if isinstance(item[0], str)] + items.sort() + else: + for key in d: + if not isinstance(key, str): + raise TypeError("keys must be strings") + items = sorted(items) + return items + + def _escape(text): m = _controlCharPat.search(text) if m is not None: @@ -388,14 +407,11 @@ def write_bytes(self, data): def write_dict(self, d): if d: self.begin_element("dict") - items = d.items() - if self._skipkeys: - items = [(k, v) for k, v in items if isinstance(k, str)] - if self._sort_keys: - items = sorted(items) - + items = _dict_items(d, self._sort_keys, self._skipkeys) for key, value in items: if not isinstance(key, str): + if self._skipkeys: + continue raise TypeError("keys must be strings") self.simple_element("key", key) self.write_value(value) @@ -717,14 +733,11 @@ def _flatten(self, value): if isinstance(value, (dict, frozendict)): keys = [] values = [] - items = value.items() - if self._skipkeys: - items = [(k, v) for k, v in items if isinstance(k, str)] - if self._sort_keys: - items = sorted(items) - + items = _dict_items(value, self._sort_keys, self._skipkeys) for k, v in items: if not isinstance(k, str): + if self._skipkeys: + continue raise TypeError("keys must be strings") keys.append(k) values.append(v) @@ -838,15 +851,11 @@ def _write_object(self, value): elif isinstance(value, (dict, frozendict)): keyRefs, valRefs = [], [] - rootItems = value.items() - if self._skipkeys: - rootItems = [(k, v) for k, v in rootItems - if isinstance(k, str)] - if self._sort_keys: - rootItems = sorted(rootItems) - + rootItems = _dict_items(value, self._sort_keys, self._skipkeys) for k, v in rootItems: if not isinstance(k, str): + if self._skipkeys: + continue raise TypeError("keys must be strings") keyRefs.append(self._getrefnum(k)) valRefs.append(self._getrefnum(v))