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

[Martin Packman]

Fix recursion handling in bencode extension

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.

[Jelmer Vernooij (jelmer)]

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

review: Approve

[The Breezy Bot (the-breezy-bot)]

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

[The Breezy Bot (the-breezy-bot)]

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

=== 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 @@
 
 from __future__ import absolute_import
 
+from cpython.bool cimport (
+    PyBool_Check,
+    )
 from cpython.bytes cimport (
     PyBytes_CheckExact,
     PyBytes_FromStringAndSize,
     PyBytes_AS_STRING,
     PyBytes_GET_SIZE,
     )
-from cpython.long cimport (
-    PyLong_CheckExact,
+from cpython.dict cimport (
+    PyDict_CheckExact,
     )
 from cpython.int cimport (
     PyInt_CheckExact,
     PyInt_FromString,
     )
-from cpython.tuple cimport (
-    PyTuple_CheckExact,
-    )
 from cpython.list cimport (
     PyList_CheckExact,
     PyList_Append,
     )
-from cpython.dict cimport (
-    PyDict_CheckExact,
-    )
-from cpython.bool cimport (
-    PyBool_Check,
+from cpython.long cimport (
+    PyLong_CheckExact,
     )
 from cpython.mem cimport (
     PyMem_Free,
     PyMem_Malloc,
     PyMem_Realloc,
     )
+from cpython.tuple cimport (
+    PyTuple_CheckExact,
+    )
 
 from libc.stdlib cimport (
     strtol,
@@ -57,15 +57,14 @@
     memcpy,
     )
 
+cdef extern from "python-compat.h":
+    int snprintf(char* buffer, size_t nsize, char* fmt, ...)
+    # Use wrapper with inverted error return so Cython can propogate
+    int BrzPy_EnterRecursiveCall(char *) except 0
+
 cdef extern from "Python.h":
-    # There is no cython module for ceval.h for some reason
-    int Py_GetRecursionLimit()
-    int Py_EnterRecursiveCall(char *)
     void Py_LeaveRecursiveCall()
 
-cdef extern from "python-compat.h":
-    int snprintf(char* buffer, size_t nsize, char* fmt, ...)
-
 cdef class Decoder
 cdef class Encoder
 
@@ -114,8 +113,7 @@
         if 0 == self.size:
             raise ValueError('stream underflow')
 
-        if Py_EnterRecursiveCall("_decode_object"):
-            raise RuntimeError("too deeply nested")
+        BrzPy_EnterRecursiveCall(" while bencode decoding")
         try:
             ch = self.tail[0]
             if c'0' <= ch <= c'9':
@@ -129,10 +127,9 @@
             elif ch == c'd':
                 D_UPDATE_TAIL(self, 1)
                 return self._decode_dict()
-            else:
-                raise ValueError('unknown object type identifier %r' % ch)
         finally:
             Py_LeaveRecursiveCall()
+        raise ValueError('unknown object type identifier %r' % ch)
 
     cdef int _read_digits(self, char stop_char) except -1:
         cdef int i
@@ -342,7 +339,7 @@
         cdef Py_ssize_t x_len
         x_len = PyBytes_GET_SIZE(x)
         self._ensure_buffer(x_len + INT_BUF_SIZE)
-        n = snprintf(self.tail, INT_BUF_SIZE, b'%d:', x_len)
+        n = snprintf(self.tail, INT_BUF_SIZE, b'%ld:', x_len)
         if n < 0:
             raise MemoryError('string %s too big to encode' % x)
         memcpy(<void *>(self.tail+n), PyBytes_AS_STRING(x), x_len)
@@ -378,9 +375,8 @@
         E_UPDATE_TAIL(self, 1)
         return 1
 
-    def process(self, object x):
-        if Py_EnterRecursiveCall("encode"):
-            raise RuntimeError("too deeply nested")
+    cpdef object process(self, object x):
+        BrzPy_EnterRecursiveCall(" while bencode encoding")
         try:
             if PyBytes_CheckExact(x):
                 self._encode_string(x)

=== 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 @@
 
 #endif
 
+#define BrzPy_EnterRecursiveCall(where) (Py_EnterRecursiveCall(where) == 0)
+
 #if defined(_WIN32) || defined(WIN32)
     /* Defining WIN32_LEAN_AND_MEAN makes including windows quite a bit
      * lighter weight.

=== 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 @@
     return suite
 
 
+class RecursionLimit(object):
+    """Context manager that lowers recursion limit for testing."""
+
+    def __init__(self, limit=100):
+        self._new_limit = limit
+        self._old_limit = sys.getrecursionlimit()
+
+    def __enter__(self):
+        sys.setrecursionlimit(self._new_limit)
+        return self
+
+    def __exit__(self, *exc_info):
+        sys.setrecursionlimit(self._old_limit)
+
+
 class TestBencodeDecode(tests.TestCase):
 
     module = None
@@ -90,7 +105,8 @@
         self._check([[b'Alice', b'Bob'], [2, 3]], b'll5:Alice3:Bobeli2ei3eee')
 
     def test_list_deepnested(self):
-        self._run_check_error(RuntimeError, (b"l" * 10000) + (b"e" * 10000))
+        with RecursionLimit():
+            self._run_check_error(RuntimeError, (b"l" * 100) + (b"e" * 100))
 
     def test_malformed_list(self):
         self._run_check_error(ValueError, b'l')
@@ -107,14 +123,9 @@
             b'd8:spam.mp3d6:author5:Alice6:lengthi100000eee')
 
     def test_dict_deepnested(self):
-        # The recursion here provokes CPython into emitting a warning on
-        # stderr, "maximum recursion depth exceeded in __subclasscheck__", due
-        # to running out of stack space while evaluating "except (...):" in
-        # _bencode_py.  This is harmless, so we temporarily override stderr to
-        # avoid distracting noise in the test output.
-        self.overrideAttr(sys, 'stderr', self._log_file)
-        self._run_check_error(
-            RuntimeError, (b"d0:" * 10000) + b'i1e' + (b"e" * 10000))
+        with RecursionLimit():
+            self._run_check_error(
+                RuntimeError, (b"d0:" * 1000) + b'i1e' + (b"e" * 1000))
 
     def test_malformed_dict(self):
         self._run_check_error(ValueError, b'd')
@@ -184,10 +195,11 @@
     def test_list_deep_nested(self):
         top = []
         l = top
-        for i in range(10000):
+        for i in range(1000):
             l.append([])
             l = l[0]
-        self.assertRaises(RuntimeError, self.module.bencode, top)
+        with RecursionLimit():
+            self.assertRaises(RuntimeError, self.module.bencode, top)
 
     def test_dict(self):
         self._check(b'de', {})
@@ -197,10 +209,11 @@
 
     def test_dict_deep_nested(self):
         d = top = {}
-        for i in range(10000):
+        for i in range(1000):
             d[b''] = {}
             d = d[b'']
-        self.assertRaises(RuntimeError, self.module.bencode, top)
+        with RecursionLimit():
+            self.assertRaises(RuntimeError, self.module.bencode, top)
 
     def test_bencached(self):
         self._check(b'i3e', self.module.Bencached(self.module.bencode(3)))

=== 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 @@
 breezy.tests.test__bencode.TestBencodeDecode.test_decoder_type_error(C)
 breezy.tests.test__bencode.TestBencodeDecode.test_decoder_type_error(python)
 breezy.tests.test__bencode.TestBencodeDecode.test_dict(C)
+breezy.tests.test__bencode.TestBencodeDecode.test_dict_deepnested(C)
 breezy.tests.test__bencode.TestBencodeDecode.test_dict_deepnested(python)
 breezy.tests.test__bencode.TestBencodeDecode.test_dict(python)
 breezy.tests.test__bencode.TestBencodeDecode.test_empty_string(C)
@@ -22162,6 +22163,7 @@
 breezy.tests.test__bencode.TestBencodeDecode.test_large_string(C)
 breezy.tests.test__bencode.TestBencodeDecode.test_large_string(python)
 breezy.tests.test__bencode.TestBencodeDecode.test_list(C)
+breezy.tests.test__bencode.TestBencodeDecode.test_list_deepnested(C)
 breezy.tests.test__bencode.TestBencodeDecode.test_list_deepnested(python)
 breezy.tests.test__bencode.TestBencodeDecode.test_list(python)
 breezy.tests.test__bencode.TestBencodeDecode.test_long(C)