Merge lp:~gz/brz/bencode_recursion into lp:brz

Proposed by Martin Packman
Status: Merged
Approved by: Martin Packman
Approved revision: no longer in the source branch.
Merge reported by: The Breezy Bot
Merged at revision: not available
Proposed branch: lp:~gz/brz/bencode_recursion
Merge into: lp:brz
Diff against target: 230 lines (+50/-37)
4 files modified
breezy/_bencode_pyx.pyx (+20/-24)
breezy/python-compat.h (+2/-0)
breezy/tests/test__bencode.py (+26/-13)
python3.passing (+2/-0)
To merge this branch: bzr merge lp:~gz/brz/bencode_recursion
Reviewer Review Type Date Requested Status
Jelmer Vernooij Approve
Review via email: mp+353451@code.launchpad.net

Commit message

Fix recursion handling in bencode extension

Description of the change

This conflicts with part of the fix in the proposed fix-c-extensions branch:

https://code.launchpad.net/~jelmer/brz/fix-c-extensions/+merge/353369

I think the right change is a mix of the two? Should bare raise but with the (uncovered?) encode site also updated.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

This seems simpler than my solution and also fix other stuff (e.g. the warnings).

review: Approve
Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/394/

Revision history for this message
The Breezy Bot (the-breezy-bot) wrote :

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/405/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'breezy/_bencode_pyx.pyx'
--- breezy/_bencode_pyx.pyx 2017-06-27 01:29:30 +0000
+++ breezy/_bencode_pyx.pyx 2018-08-21 00:57:35 +0000
@@ -18,37 +18,37 @@
1818
19from __future__ import absolute_import19from __future__ import absolute_import
2020
21from cpython.bool cimport (
22 PyBool_Check,
23 )
21from cpython.bytes cimport (24from cpython.bytes cimport (
22 PyBytes_CheckExact,25 PyBytes_CheckExact,
23 PyBytes_FromStringAndSize,26 PyBytes_FromStringAndSize,
24 PyBytes_AS_STRING,27 PyBytes_AS_STRING,
25 PyBytes_GET_SIZE,28 PyBytes_GET_SIZE,
26 )29 )
27from cpython.long cimport (30from cpython.dict cimport (
28 PyLong_CheckExact,31 PyDict_CheckExact,
29 )32 )
30from cpython.int cimport (33from cpython.int cimport (
31 PyInt_CheckExact,34 PyInt_CheckExact,
32 PyInt_FromString,35 PyInt_FromString,
33 )36 )
34from cpython.tuple cimport (
35 PyTuple_CheckExact,
36 )
37from cpython.list cimport (37from cpython.list cimport (
38 PyList_CheckExact,38 PyList_CheckExact,
39 PyList_Append,39 PyList_Append,
40 )40 )
41from cpython.dict cimport (41from cpython.long cimport (
42 PyDict_CheckExact,42 PyLong_CheckExact,
43 )
44from cpython.bool cimport (
45 PyBool_Check,
46 )43 )
47from cpython.mem cimport (44from cpython.mem cimport (
48 PyMem_Free,45 PyMem_Free,
49 PyMem_Malloc,46 PyMem_Malloc,
50 PyMem_Realloc,47 PyMem_Realloc,
51 )48 )
49from cpython.tuple cimport (
50 PyTuple_CheckExact,
51 )
5252
53from libc.stdlib cimport (53from libc.stdlib cimport (
54 strtol,54 strtol,
@@ -57,15 +57,14 @@
57 memcpy,57 memcpy,
58 )58 )
5959
60cdef extern from "python-compat.h":
61 int snprintf(char* buffer, size_t nsize, char* fmt, ...)
62 # Use wrapper with inverted error return so Cython can propogate
63 int BrzPy_EnterRecursiveCall(char *) except 0
64
60cdef extern from "Python.h":65cdef extern from "Python.h":
61 # There is no cython module for ceval.h for some reason
62 int Py_GetRecursionLimit()
63 int Py_EnterRecursiveCall(char *)
64 void Py_LeaveRecursiveCall()66 void Py_LeaveRecursiveCall()
6567
66cdef extern from "python-compat.h":
67 int snprintf(char* buffer, size_t nsize, char* fmt, ...)
68
69cdef class Decoder68cdef class Decoder
70cdef class Encoder69cdef class Encoder
7170
@@ -114,8 +113,7 @@
114 if 0 == self.size:113 if 0 == self.size:
115 raise ValueError('stream underflow')114 raise ValueError('stream underflow')
116115
117 if Py_EnterRecursiveCall("_decode_object"):116 BrzPy_EnterRecursiveCall(" while bencode decoding")
118 raise RuntimeError("too deeply nested")
119 try:117 try:
120 ch = self.tail[0]118 ch = self.tail[0]
121 if c'0' <= ch <= c'9':119 if c'0' <= ch <= c'9':
@@ -129,10 +127,9 @@
129 elif ch == c'd':127 elif ch == c'd':
130 D_UPDATE_TAIL(self, 1)128 D_UPDATE_TAIL(self, 1)
131 return self._decode_dict()129 return self._decode_dict()
132 else:
133 raise ValueError('unknown object type identifier %r' % ch)
134 finally:130 finally:
135 Py_LeaveRecursiveCall()131 Py_LeaveRecursiveCall()
132 raise ValueError('unknown object type identifier %r' % ch)
136133
137 cdef int _read_digits(self, char stop_char) except -1:134 cdef int _read_digits(self, char stop_char) except -1:
138 cdef int i135 cdef int i
@@ -342,7 +339,7 @@
342 cdef Py_ssize_t x_len339 cdef Py_ssize_t x_len
343 x_len = PyBytes_GET_SIZE(x)340 x_len = PyBytes_GET_SIZE(x)
344 self._ensure_buffer(x_len + INT_BUF_SIZE)341 self._ensure_buffer(x_len + INT_BUF_SIZE)
345 n = snprintf(self.tail, INT_BUF_SIZE, b'%d:', x_len)342 n = snprintf(self.tail, INT_BUF_SIZE, b'%ld:', x_len)
346 if n < 0:343 if n < 0:
347 raise MemoryError('string %s too big to encode' % x)344 raise MemoryError('string %s too big to encode' % x)
348 memcpy(<void *>(self.tail+n), PyBytes_AS_STRING(x), x_len)345 memcpy(<void *>(self.tail+n), PyBytes_AS_STRING(x), x_len)
@@ -378,9 +375,8 @@
378 E_UPDATE_TAIL(self, 1)375 E_UPDATE_TAIL(self, 1)
379 return 1376 return 1
380377
381 def process(self, object x):378 cpdef object process(self, object x):
382 if Py_EnterRecursiveCall("encode"):379 BrzPy_EnterRecursiveCall(" while bencode encoding")
383 raise RuntimeError("too deeply nested")
384 try:380 try:
385 if PyBytes_CheckExact(x):381 if PyBytes_CheckExact(x):
386 self._encode_string(x)382 self._encode_string(x)
387383
=== modified file 'breezy/python-compat.h'
--- breezy/python-compat.h 2018-07-12 01:43:49 +0000
+++ breezy/python-compat.h 2018-08-21 00:57:35 +0000
@@ -71,6 +71,8 @@
7171
72#endif72#endif
7373
74#define BrzPy_EnterRecursiveCall(where) (Py_EnterRecursiveCall(where) == 0)
75
74#if defined(_WIN32) || defined(WIN32)76#if defined(_WIN32) || defined(WIN32)
75 /* Defining WIN32_LEAN_AND_MEAN makes including windows quite a bit77 /* Defining WIN32_LEAN_AND_MEAN makes including windows quite a bit
76 * lighter weight.78 * lighter weight.
7779
=== modified file 'breezy/tests/test__bencode.py'
--- breezy/tests/test__bencode.py 2017-06-10 22:09:37 +0000
+++ breezy/tests/test__bencode.py 2018-08-21 00:57:35 +0000
@@ -26,6 +26,21 @@
26 return suite26 return suite
2727
2828
29class RecursionLimit(object):
30 """Context manager that lowers recursion limit for testing."""
31
32 def __init__(self, limit=100):
33 self._new_limit = limit
34 self._old_limit = sys.getrecursionlimit()
35
36 def __enter__(self):
37 sys.setrecursionlimit(self._new_limit)
38 return self
39
40 def __exit__(self, *exc_info):
41 sys.setrecursionlimit(self._old_limit)
42
43
29class TestBencodeDecode(tests.TestCase):44class TestBencodeDecode(tests.TestCase):
3045
31 module = None46 module = None
@@ -90,7 +105,8 @@
90 self._check([[b'Alice', b'Bob'], [2, 3]], b'll5:Alice3:Bobeli2ei3eee')105 self._check([[b'Alice', b'Bob'], [2, 3]], b'll5:Alice3:Bobeli2ei3eee')
91106
92 def test_list_deepnested(self):107 def test_list_deepnested(self):
93 self._run_check_error(RuntimeError, (b"l" * 10000) + (b"e" * 10000))108 with RecursionLimit():
109 self._run_check_error(RuntimeError, (b"l" * 100) + (b"e" * 100))
94110
95 def test_malformed_list(self):111 def test_malformed_list(self):
96 self._run_check_error(ValueError, b'l')112 self._run_check_error(ValueError, b'l')
@@ -107,14 +123,9 @@
107 b'd8:spam.mp3d6:author5:Alice6:lengthi100000eee')123 b'd8:spam.mp3d6:author5:Alice6:lengthi100000eee')
108124
109 def test_dict_deepnested(self):125 def test_dict_deepnested(self):
110 # The recursion here provokes CPython into emitting a warning on126 with RecursionLimit():
111 # stderr, "maximum recursion depth exceeded in __subclasscheck__", due127 self._run_check_error(
112 # to running out of stack space while evaluating "except (...):" in128 RuntimeError, (b"d0:" * 1000) + b'i1e' + (b"e" * 1000))
113 # _bencode_py. This is harmless, so we temporarily override stderr to
114 # avoid distracting noise in the test output.
115 self.overrideAttr(sys, 'stderr', self._log_file)
116 self._run_check_error(
117 RuntimeError, (b"d0:" * 10000) + b'i1e' + (b"e" * 10000))
118129
119 def test_malformed_dict(self):130 def test_malformed_dict(self):
120 self._run_check_error(ValueError, b'd')131 self._run_check_error(ValueError, b'd')
@@ -184,10 +195,11 @@
184 def test_list_deep_nested(self):195 def test_list_deep_nested(self):
185 top = []196 top = []
186 l = top197 l = top
187 for i in range(10000):198 for i in range(1000):
188 l.append([])199 l.append([])
189 l = l[0]200 l = l[0]
190 self.assertRaises(RuntimeError, self.module.bencode, top)201 with RecursionLimit():
202 self.assertRaises(RuntimeError, self.module.bencode, top)
191203
192 def test_dict(self):204 def test_dict(self):
193 self._check(b'de', {})205 self._check(b'de', {})
@@ -197,10 +209,11 @@
197209
198 def test_dict_deep_nested(self):210 def test_dict_deep_nested(self):
199 d = top = {}211 d = top = {}
200 for i in range(10000):212 for i in range(1000):
201 d[b''] = {}213 d[b''] = {}
202 d = d[b'']214 d = d[b'']
203 self.assertRaises(RuntimeError, self.module.bencode, top)215 with RecursionLimit():
216 self.assertRaises(RuntimeError, self.module.bencode, top)
204217
205 def test_bencached(self):218 def test_bencached(self):
206 self._check(b'i3e', self.module.Bencached(self.module.bencode(3)))219 self._check(b'i3e', self.module.Bencached(self.module.bencode(3)))
207220
=== modified file 'python3.passing'
--- python3.passing 2018-08-21 00:20:23 +0000
+++ python3.passing 2018-08-21 00:57:35 +0000
@@ -22151,6 +22151,7 @@
22151breezy.tests.test__bencode.TestBencodeDecode.test_decoder_type_error(C)22151breezy.tests.test__bencode.TestBencodeDecode.test_decoder_type_error(C)
22152breezy.tests.test__bencode.TestBencodeDecode.test_decoder_type_error(python)22152breezy.tests.test__bencode.TestBencodeDecode.test_decoder_type_error(python)
22153breezy.tests.test__bencode.TestBencodeDecode.test_dict(C)22153breezy.tests.test__bencode.TestBencodeDecode.test_dict(C)
22154breezy.tests.test__bencode.TestBencodeDecode.test_dict_deepnested(C)
22154breezy.tests.test__bencode.TestBencodeDecode.test_dict_deepnested(python)22155breezy.tests.test__bencode.TestBencodeDecode.test_dict_deepnested(python)
22155breezy.tests.test__bencode.TestBencodeDecode.test_dict(python)22156breezy.tests.test__bencode.TestBencodeDecode.test_dict(python)
22156breezy.tests.test__bencode.TestBencodeDecode.test_empty_string(C)22157breezy.tests.test__bencode.TestBencodeDecode.test_empty_string(C)
@@ -22162,6 +22163,7 @@
22162breezy.tests.test__bencode.TestBencodeDecode.test_large_string(C)22163breezy.tests.test__bencode.TestBencodeDecode.test_large_string(C)
22163breezy.tests.test__bencode.TestBencodeDecode.test_large_string(python)22164breezy.tests.test__bencode.TestBencodeDecode.test_large_string(python)
22164breezy.tests.test__bencode.TestBencodeDecode.test_list(C)22165breezy.tests.test__bencode.TestBencodeDecode.test_list(C)
22166breezy.tests.test__bencode.TestBencodeDecode.test_list_deepnested(C)
22165breezy.tests.test__bencode.TestBencodeDecode.test_list_deepnested(python)22167breezy.tests.test__bencode.TestBencodeDecode.test_list_deepnested(python)
22166breezy.tests.test__bencode.TestBencodeDecode.test_list(python)22168breezy.tests.test__bencode.TestBencodeDecode.test_list(python)
22167breezy.tests.test__bencode.TestBencodeDecode.test_long(C)22169breezy.tests.test__bencode.TestBencodeDecode.test_long(C)

Subscribers

People subscribed via source and target branches