Skip to content

Commit cced98b

Browse files
ashm-devashm-dev
committed
Revert "Address review: remove redundant comments, use nonlocal, add list mutation test"
This reverts commit b4e50c9.
1 parent 37b25e9 commit cced98b

2 files changed

Lines changed: 71 additions & 123 deletions

File tree

‎Lib/test/test_json/test_speedups.py‎

Lines changed: 66 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,21 @@
11
from test.test_json import CTest
2-
from test.support import gc_collect
32

43

54
class BadBool:
65
def __bool__(self):
7-
1 / 0
6+
1/0
87

98

109
class TestSpeedups(CTest):
1110
def test_scanstring(self):
1211
self.assertEqual(self.json.decoder.scanstring.__module__, "_json")
13-
self.assertIs(
14-
self.json.decoder.scanstring, self.json.decoder.c_scanstring
15-
)
12+
self.assertIs(self.json.decoder.scanstring, self.json.decoder.c_scanstring)
1613

1714
def test_encode_basestring_ascii(self):
18-
self.assertEqual(
19-
self.json.encoder.encode_basestring_ascii.__module__, "_json"
20-
)
21-
self.assertIs(
22-
self.json.encoder.encode_basestring_ascii,
23-
self.json.encoder.c_encode_basestring_ascii,
24-
)
15+
self.assertEqual(self.json.encoder.encode_basestring_ascii.__module__,
16+
"_json")
17+
self.assertIs(self.json.encoder.encode_basestring_ascii,
18+
self.json.encoder.c_encode_basestring_ascii)
2519

2620

2721
class TestDecode(CTest):
@@ -31,170 +25,119 @@ def test_make_scanner(self):
3125
def test_bad_bool_args(self):
3226
def test(value):
3327
self.json.decoder.JSONDecoder(strict=BadBool()).decode(value)
34-
3528
self.assertRaises(ZeroDivisionError, test, '""')
36-
self.assertRaises(ZeroDivisionError, test, "{}")
29+
self.assertRaises(ZeroDivisionError, test, '{}')
3730

3831

3932
class TestEncode(CTest):
4033
def test_make_encoder(self):
4134
# bpo-6986: The interpreter shouldn't crash in case c_make_encoder()
4235
# receives invalid arguments.
43-
self.assertRaises(
44-
TypeError,
45-
self.json.encoder.c_make_encoder,
36+
self.assertRaises(TypeError, self.json.encoder.c_make_encoder,
4637
(True, False),
47-
b"\xcd\x7d\x3d\x4e\x12\x4c\xf9\x79\xd7\x52\xba\x82\xf2\x27\x4a\x7d\xa0\xca\x75",
48-
None,
49-
)
38+
b"\xCD\x7D\x3D\x4E\x12\x4C\xF9\x79\xD7\x52\xBA\x82\xF2\x27\x4A\x7D\xA0\xCA\x75",
39+
None)
5040

5141
def test_bad_str_encoder(self):
5242
# Issue #31505: There shouldn't be an assertion failure in case
5343
# c_make_encoder() receives a bad encoder() argument.
5444
def bad_encoder1(*args):
5545
return None
56-
57-
enc = self.json.encoder.c_make_encoder(
58-
None,
59-
lambda obj: str(obj),
60-
bad_encoder1,
61-
None,
62-
": ",
63-
", ",
64-
False,
65-
False,
66-
False,
67-
)
46+
enc = self.json.encoder.c_make_encoder(None, lambda obj: str(obj),
47+
bad_encoder1, None, ': ', ', ',
48+
False, False, False)
6849
with self.assertRaises(TypeError):
69-
enc("spam", 4)
50+
enc('spam', 4)
7051
with self.assertRaises(TypeError):
71-
enc({"spam": 42}, 4)
52+
enc({'spam': 42}, 4)
7253

7354
def bad_encoder2(*args):
74-
1 / 0
75-
76-
enc = self.json.encoder.c_make_encoder(
77-
None,
78-
lambda obj: str(obj),
79-
bad_encoder2,
80-
None,
81-
": ",
82-
", ",
83-
False,
84-
False,
85-
False,
86-
)
55+
1/0
56+
enc = self.json.encoder.c_make_encoder(None, lambda obj: str(obj),
57+
bad_encoder2, None, ': ', ', ',
58+
False, False, False)
8759
with self.assertRaises(ZeroDivisionError):
88-
enc("spam", 4)
60+
enc('spam', 4)
8961

9062
def test_bad_markers_argument_to_encoder(self):
9163
# https://bugs.python.org/issue45269
9264
with self.assertRaisesRegex(
9365
TypeError,
94-
r"make_encoder\(\) argument 1 must be dict or None, not int",
66+
r'make_encoder\(\) argument 1 must be dict or None, not int',
9567
):
96-
self.json.encoder.c_make_encoder(
97-
1, None, None, None, ": ", ", ", False, False, False
98-
)
68+
self.json.encoder.c_make_encoder(1, None, None, None, ': ', ', ',
69+
False, False, False)
9970

10071
def test_bad_bool_args(self):
10172
def test(name):
102-
self.json.encoder.JSONEncoder(**{name: BadBool()}).encode({"a": 1})
103-
104-
self.assertRaises(ZeroDivisionError, test, "skipkeys")
105-
self.assertRaises(ZeroDivisionError, test, "ensure_ascii")
106-
self.assertRaises(ZeroDivisionError, test, "check_circular")
107-
self.assertRaises(ZeroDivisionError, test, "allow_nan")
108-
self.assertRaises(ZeroDivisionError, test, "sort_keys")
73+
self.json.encoder.JSONEncoder(**{name: BadBool()}).encode({'a': 1})
74+
self.assertRaises(ZeroDivisionError, test, 'skipkeys')
75+
self.assertRaises(ZeroDivisionError, test, 'ensure_ascii')
76+
self.assertRaises(ZeroDivisionError, test, 'check_circular')
77+
self.assertRaises(ZeroDivisionError, test, 'allow_nan')
78+
self.assertRaises(ZeroDivisionError, test, 'sort_keys')
10979

11080
def test_unsortable_keys(self):
11181
with self.assertRaises(TypeError):
112-
self.json.encoder.JSONEncoder(sort_keys=True).encode(
113-
{"a": 1, 1: "a"}
114-
)
82+
self.json.encoder.JSONEncoder(sort_keys=True).encode({'a': 1, 1: 'a'})
11583

11684
def test_current_indent_level(self):
11785
enc = self.json.encoder.c_make_encoder(
11886
markers=None,
11987
default=str,
12088
encoder=self.json.encoder.c_encode_basestring,
121-
indent="\t",
122-
key_separator=": ",
123-
item_separator=", ",
89+
indent='\t',
90+
key_separator=': ',
91+
item_separator=', ',
12492
sort_keys=False,
12593
skipkeys=False,
126-
allow_nan=False,
127-
)
128-
expected = '[\n\t"spam", \n\t{\n\t\t"ham": "eggs"\n\t}\n]'
129-
self.assertEqual(enc(["spam", {"ham": "eggs"}], 0)[0], expected)
130-
self.assertEqual(enc(["spam", {"ham": "eggs"}], -3)[0], expected)
94+
allow_nan=False)
95+
expected = (
96+
'[\n'
97+
'\t"spam", \n'
98+
'\t{\n'
99+
'\t\t"ham": "eggs"\n'
100+
'\t}\n'
101+
']')
102+
self.assertEqual(enc(['spam', {'ham': 'eggs'}], 0)[0], expected)
103+
self.assertEqual(enc(['spam', {'ham': 'eggs'}], -3)[0], expected)
131104
expected2 = (
132-
"[\n"
105+
'[\n'
133106
'\t\t\t\t"spam", \n'
134-
"\t\t\t\t{\n"
107+
'\t\t\t\t{\n'
135108
'\t\t\t\t\t"ham": "eggs"\n'
136-
"\t\t\t\t}\n"
137-
"\t\t\t]"
138-
)
139-
self.assertEqual(enc(["spam", {"ham": "eggs"}], 3)[0], expected2)
140-
self.assertRaises(TypeError, enc, ["spam", {"ham": "eggs"}], 3.0)
141-
self.assertRaises(TypeError, enc, ["spam", {"ham": "eggs"}])
109+
'\t\t\t\t}\n'
110+
'\t\t\t]')
111+
self.assertEqual(enc(['spam', {'ham': 'eggs'}], 3)[0], expected2)
112+
self.assertRaises(TypeError, enc, ['spam', {'ham': 'eggs'}], 3.0)
113+
self.assertRaises(TypeError, enc, ['spam', {'ham': 'eggs'}])
114+
115+
def test_mutate_items_during_encode(self):
116+
c_make_encoder = getattr(self.json.encoder, 'c_make_encoder', None)
117+
if c_make_encoder is None:
118+
self.skipTest("c_make_encoder not available")
142119

143-
def test_mutate_dict_items_during_encode(self):
144-
items = None
120+
cache = []
145121

146122
class BadDict(dict):
147123
def items(self):
148-
nonlocal items
149-
items = [("boom", object())]
150-
return items
124+
entries = [("boom", object())]
125+
cache.append(entries)
126+
return entries
151127

152128
def encode_str(obj):
153-
nonlocal items
154-
if items is not None:
155-
items.clear()
156-
items = None
157-
gc_collect()
129+
if cache:
130+
cache.pop().clear()
158131
return '"x"'
159132

160-
encoder = self.json.encoder.c_make_encoder(
161-
None,
162-
lambda o: "null",
163-
encode_str,
164-
None,
165-
": ",
166-
", ",
167-
False,
168-
False,
169-
True,
133+
encoder = c_make_encoder(
134+
None, lambda o: "null",
135+
encode_str, None,
136+
": ", ", ", False,
137+
False, True
170138
)
171139

172140
try:
173141
encoder(BadDict(real=1), 0)
174142
except (ValueError, RuntimeError):
175143
pass
176-
177-
def test_mutate_list_during_encode(self):
178-
lst = [object() for _ in range(10)]
179-
180-
def default(obj):
181-
lst.clear()
182-
gc_collect()
183-
return None
184-
185-
encoder = self.json.encoder.c_make_encoder(
186-
None,
187-
default,
188-
self.json.encoder.c_encode_basestring,
189-
None,
190-
": ",
191-
", ",
192-
False,
193-
False,
194-
True,
195-
)
196-
197-
try:
198-
encoder(lst, 0)
199-
except (ValueError, RuntimeError):
200-
pass

‎Modules/_json.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,6 +1767,8 @@ _encoder_iterate_dict_lock_held(PyEncoderObject *s, PyUnicodeWriter *writer,
17671767
PyObject *key, *value;
17681768
Py_ssize_t pos = 0;
17691769
while (PyDict_Next(dct, &pos, &key, &value)) {
1770+
// GH-142831: The key and value must be strong-referenced to avoid
1771+
// use-after-free if the user code modifies the dict during iteration.
17701772
Py_INCREF(key);
17711773
Py_INCREF(value);
17721774

@@ -1882,6 +1884,9 @@ _encoder_iterate_fast_seq_lock_held(PyEncoderObject *s, PyUnicodeWriter *writer,
18821884
{
18831885
for (Py_ssize_t i = 0; i < PySequence_Fast_GET_SIZE(s_fast); i++) {
18841886
PyObject *obj = PySequence_Fast_GET_ITEM(s_fast, i);
1887+
1888+
// GH-142831: The object must be strong-referenced to avoid use-after-free
1889+
// if the user code modifies the sequence during iteration.
18851890
Py_INCREF(obj);
18861891

18871892
if (i) {

0 commit comments

Comments
 (0)