Report container nesting past the recursion limit as IonException

src-python/amazon/ion/simpleion.py

@@ -56,6 +56,7 @@
 """
 import io
 import warnings
+from contextlib import contextmanager
 from datetime import datetime
 from decimal import Decimal
 from enum import IntFlag
@@ -306,13 +307,14 @@ def dump_python(obj, fp, imports=None, binary=True, sequence_as_stream=False,
     from_type = _FROM_TYPE_TUPLE_AS_SEXP if tuple_as_sexp else _FROM_TYPE
     if binary or not omit_version_marker:
         writer.send(ION_VERSION_MARKER_EVENT)  # The IVM is emitted automatically in binary; it's optional in text.
-    if sequence_as_stream and isinstance(obj, (list, tuple)) or isinstance(obj, GeneratorType):
-        # Treat this top-level sequence as a stream; serialize its elements as top-level values, but don't serialize the
-        # sequence itself.
-        for top_level in obj:
-            _dump(top_level, writer, from_type)
-    else:
-        _dump(obj, writer, from_type)
+    with _translate_recursion_error():
+        if sequence_as_stream and isinstance(obj, (list, tuple)) or isinstance(obj, GeneratorType):
+            # Treat this top-level sequence as a stream; serialize its elements as top-level values, but don't
+            # serialize the sequence itself.
+            for top_level in obj:
+                _dump(top_level, writer, from_type)
+        else:
+            _dump(obj, writer, from_type)
     writer.send(ION_STREAM_END_EVENT)
 
 
@@ -361,8 +363,11 @@ def _dump(obj, writer, from_type, field=None, in_struct=False, depth=0):
         ion_type = obj.ion_type
         ion_nature = True
     except AttributeError:
-        ion_type = _ion_type(obj, from_type)
         ion_nature = False
+    if not ion_nature:
+        # Resolved outside the handler above: a RecursionError raised here while the AttributeError
+        # is still in flight has to record it as its context, which PyPy cannot do out of stack.
+        ion_type = _ion_type(obj, from_type)
     if ion_type is None:
         raise IonException('Value must have a non-None ion_type: %s, depth: %d, field: %s' % (repr(obj), depth, field))
     if not null and ion_type.is_container:
@@ -387,6 +392,16 @@ def _dump(obj, writer, from_type, field=None, in_struct=False, depth=0):
     writer.send(event)
 
 
+@contextmanager
+def _translate_recursion_error():
+    """Reports container nesting too deep to process as an IonException, like other malformed
+    input, keeping the RecursionError as the cause."""
+    try:
+        yield
+    except RecursionError as e:
+        raise IonException('Container nesting exceeded the maximum supported recursion depth.') from e
+
+
 def load_python(fp, catalog=None, single_value=True, parse_eagerly=True):
     """'pure' Python implementation. Users should prefer to call ``load``."""
     if isinstance(fp, _TEXT_TYPES):
@@ -402,7 +417,8 @@ def load_python(fp, catalog=None, single_value=True, parse_eagerly=True):
     reader = blocking_reader(managed_reader(raw_reader, catalog), fp)
     if parse_eagerly:
         out = []  # top-level
-        _load(out, reader)
+        with _translate_recursion_error():
+            _load(out, reader)
         if single_value:
             if len(out) != 1:
                 raise IonException('Stream contained %d values; expected a single value.' % (len(out),))
@@ -411,7 +427,8 @@ def load_python(fp, catalog=None, single_value=True, parse_eagerly=True):
     else:
         out = _load_iteratively(reader)
         if single_value:
-            result = next(out)
+            with _translate_recursion_error():
+                result = next(out)
             try:
                 next(out)
                 raise IonException('Stream contained more than 1 values; expected a single value.')
@@ -443,7 +460,8 @@ def _load_iteratively(reader, end_type=IonEventType.STREAM_END):
         ion_type = event.ion_type
         if event.event_type is IonEventType.CONTAINER_START:
             container = _FROM_ION_TYPE[ion_type].from_event(event)
-            _load(container, reader, IonEventType.CONTAINER_END, ion_type is IonType.STRUCT)
+            with _translate_recursion_error():
+                _load(container, reader, IonEventType.CONTAINER_END, ion_type is IonType.STRUCT)
             yield container
         elif event.event_type is IonEventType.SCALAR:
             if event.value is None or ion_type is IonType.NULL or ion_type.is_container:
@@ -480,7 +498,8 @@ def add(obj):
 def dump_extension(obj, fp, binary=True, sequence_as_stream=False, tuple_as_sexp=False, omit_version_marker=False):
     """C-extension implementation. Users should prefer to call ``dump``."""
 
-    res = ionc.ionc_write(obj, binary, sequence_as_stream, tuple_as_sexp)
+    with _translate_recursion_error():
+        res = ionc.ionc_write(obj, binary, sequence_as_stream, tuple_as_sexp)
 
     # TODO: support "omit_version_marker" rather than hacking.
     # TODO: support "trailing_commas" (support is not included in the C code).
@@ -489,21 +508,37 @@ def dump_extension(obj, fp, binary=True, sequence_as_stream=False, tuple_as_sexp
     fp.write(res)
 
 
+def _translate_recursion_error_iter(iterator):
+    """Yields from ``iterator``, reporting nesting too deep to process as an IonException."""
+    while True:
+        with _translate_recursion_error():
+            try:
+                value = next(iterator)
+            except StopIteration:
+                return
+        yield value
+
+
 def load_extension(fp, single_value=True, parse_eagerly=True,
                    text_buffer_size_limit=None, value_model=IonPyValueModel.ION_PY):
     """C-extension implementation. Users should prefer to call ``load``."""
     iterator = ionc.ionc_read(fp, value_model=value_model.value, text_buffer_size_limit=text_buffer_size_limit)
     if single_value:
-        try:
-            value = next(iterator)
-        except StopIteration:
-            return None
-        try:
-            next(iterator)
-            raise IonException('Stream contained more than 1 values; expected a single value.')
-        except StopIteration:
-            pass
+        with _translate_recursion_error():
+            try:
+                value = next(iterator)
+            except StopIteration:
+                return None
+        with _translate_recursion_error():
+            try:
+                next(iterator)
+                raise IonException('Stream contained more than 1 values; expected a single value.')
+            except StopIteration:
+                pass
         return value
     if parse_eagerly:
-        return list(iterator)
-    return iterator
+        with _translate_recursion_error():
+            return list(iterator)
+    # Wrapped so that a caller advancing the iterator themselves also gets an IonException. The
+    # wrapper is a generator, which ionc_write writes as a stream just like the raw iterator.
+    return _translate_recursion_error_iter(iterator)

src/ioncmodule.c

@@ -27,6 +27,10 @@ static char _err_msg[ERR_MSG_MAX_LEN];
 
 #define _FAILWITHMSG(x, msg) { err = x; snprintf(_err_msg, ERR_MSG_MAX_LEN, msg); goto fail; }
 
+// Unwinds when a Python C-API call returns NULL, having already set an exception. Yields a
+// non-zero iERR to stop the IONCHECK chain while leaving that exception intact for the caller.
+#define _FAILWITH_PENDING_PYERR() { err = IERR_INTERNAL_ERROR; goto fail; }
+
 #define IONC_BYTES_FORMAT "y#"
 #define IONC_READ_ARGS_FORMAT "ObO"
 
@@ -355,7 +359,11 @@ static iERR ionc_write_sequence(hWRITER writer, PyObject* sequence, PyObject* tu
         child_obj = PySequence_Fast_GET_ITEM(sequence, i);
         Py_INCREF(child_obj);
 
-        IONCHECK(Py_EnterRecursiveCall(" while writing an Ion sequence"));
+        // Returns a plain int, not an iERR; see ionc_read_into_container.
+        if (Py_EnterRecursiveCall(" while writing an Ion sequence")) {
+            err = IERR_STACK_OVERFLOW;
+            goto fail;
+        }
         err = ionc_write_value(writer, child_obj, tuple_as_sexp);
         Py_LeaveRecursiveCall();
         IONCHECK(err);
@@ -378,15 +386,19 @@ static iERR ionc_write_sequence(hWRITER writer, PyObject* sequence, PyObject* tu
  */
 
 static iERR write_struct_field(hWRITER writer, PyObject* key, PyObject* val, PyObject* tuple_as_sexp) {
-    iERR err;
+    iENTER;
     if (PyUnicode_Check(key)) {
         ION_STRING field_name;
         ion_string_from_py(key, &field_name);
         IONCHECK(ion_writer_write_field_name(writer, &field_name));
     } else if (key == Py_None) {
         IONCHECK(_ion_writer_write_field_sid_helper(writer, 0));
     }
-    IONCHECK(Py_EnterRecursiveCall(" while writing an Ion struct"));
+    // Returns a plain int, not an iERR; see ionc_read_into_container.
+    if (Py_EnterRecursiveCall(" while writing an Ion struct")) {
+        err = IERR_STACK_OVERFLOW;
+        goto fail;
+    }
     err = ionc_write_value(writer, val, tuple_as_sexp);
     Py_LeaveRecursiveCall();
     IONCHECK(err);
@@ -798,7 +810,9 @@ static PyObject* ionc_write(PyObject *self, PyObject *args, PyObject *kwds) {
     options.max_annotation_count = ANNOTATION_MAX_LEN;
     IONCHECK(ion_writer_open(&writer, ion_stream, &options));
 
-    if (Py_TYPE(obj) == &ionc_read_IteratorType) {
+    // Generators are written as a stream of top-level values, matching dump_python, and covering
+    // the read iterator once simpleion has wrapped it to translate errors.
+    if (Py_TYPE(obj) == &ionc_read_IteratorType || PyGen_Check(obj)) {
         PyObject *item;
         while ((item = PyIter_Next(obj)) != NULL) {
             err = ionc_write_value(writer, item, tuple_as_sexp);
@@ -863,14 +877,17 @@ static PyObject* ionc_write(PyObject *self, PyObject *args, PyObject *kwds) {
     Py_XDECREF(sequence_as_stream);
     Py_XDECREF(tuple_as_sexp);
 
+    // Reported as-is and translated in simpleion.py; see ionc_read_iter_next.
+    if (PyErr_ExceptionMatches(PyExc_RecursionError)) {
+        return NULL;
+    }
     PyObject* exception = NULL;
     if (err == IERR_INVALID_STATE) {
         exception = PyErr_Format(PyExc_TypeError, "%s", _err_msg);
     }
     else {
         exception = PyErr_Format(_ion_exception_cls, "%s %s", ion_error_to_str(err), _err_msg);
     }
-
     _err_msg[0] = '\0';
     return exception;
 }
@@ -1005,7 +1022,12 @@ static iERR ionc_read_timestamp(hREADER hreader, PyObject** timestamp_out) {
 static iERR ionc_read_into_container(hREADER hreader, PyObject* container, enum ContainerType parent_type, uint8_t value_model) {
     iENTER;
     IONCHECK(ion_reader_step_in(hreader));
-    IONCHECK(Py_EnterRecursiveCall(" while reading an Ion container"));
+    // Py_EnterRecursiveCall reports failure with a plain non-zero int and sets a RecursionError.
+    // That value is not an iERR, so it is checked here rather than handed to IONCHECK.
+    if (Py_EnterRecursiveCall(" while reading an Ion container")) {
+        err = IERR_STACK_OVERFLOW;
+        goto fail;
+    }
     err = ionc_read_all(hreader, container, parent_type, value_model);
     Py_LeaveRecursiveCall();
     IONCHECK(err);
@@ -1022,32 +1044,49 @@ static iERR ionc_read_into_container(hREADER hreader, PyObject* container, enum
  *      container_type: Type of container to add to.
  *      field_name:  The field name of the element if it is inside a struct
  */
-static void ionc_add_to_container(PyObject* pyContainer, PyObject* element, enum ContainerType container_type, PyObject* field_name) {
+static iERR ionc_add_to_container(PyObject* pyContainer, PyObject* element, enum ContainerType container_type, PyObject* field_name) {
+    iENTER;
+    // A NULL element means its construction already failed and set an exception. Reporting that
+    // exception is more useful than the SystemError PyList_Append/PyDict_SetItem would raise.
+    if (element == NULL) {
+        _FAILWITH_PENDING_PYERR();
+    }
     switch (container_type) {
         case MULTIMAP:
         {
             // this builds the "hash-map of lists" structure that the IonPyDict object
             // expects for its __store
             PyObject* empty = PyList_New(0);
+            if (empty == NULL) {
+                _FAILWITH_PENDING_PYERR();
+            }
             // SetDefault performs get|set with a single hash of the key
             PyObject* found = PyDict_SetDefault(pyContainer, field_name, empty);
-            PyList_Append(found, element);
-
+            if (found == NULL || PyList_Append(found, element) < 0) {
+                Py_DECREF(empty);
+                _FAILWITH_PENDING_PYERR();
+            }
             Py_DECREF(empty);
             break;
         }
         case STD_DICT:
         {
-            PyDict_SetItem(pyContainer, field_name, element);
+            if (PyDict_SetItem(pyContainer, field_name, element) < 0) {
+                _FAILWITH_PENDING_PYERR();
+            }
             break;
         }
         case LIST:
         {
-            PyList_Append(pyContainer, (PyObject*)element);
+            if (PyList_Append(pyContainer, (PyObject*)element) < 0) {
+                _FAILWITH_PENDING_PYERR();
+            }
             break;
         }
     }
+fail:
     Py_XDECREF(element);
+    cRETURN;
 }
 
 /*
@@ -1285,6 +1324,10 @@ iERR ionc_read_value(hREADER hreader, ION_TYPE t, PyObject* container, enum Cont
                 wrap_py_value = TRUE;
                 container_type = MULTIMAP;
             }
+            // Runs Python code once per nesting level, so it can fail with a RecursionError set.
+            if (py_value == NULL) {
+                _FAILWITH_PENDING_PYERR();
+            }
 
             IONCHECK(ionc_read_into_container(hreader, py_value, container_type, value_model));
             break;
@@ -1309,6 +1352,10 @@ iERR ionc_read_value(hREADER hreader, ION_TYPE t, PyObject* container, enum Cont
             } else {
                 py_value = PyList_New(0);
             }
+            // Runs Python code once per nesting level, so it can fail with a RecursionError set.
+            if (py_value == NULL) {
+                _FAILWITH_PENDING_PYERR();
+            }
             IONCHECK(ionc_read_into_container(hreader, py_value, LIST, value_model));
             ion_nature_constructor = _ionpylist_fromvalue;
             break;
@@ -1328,9 +1375,14 @@ iERR ionc_read_value(hREADER hreader, ION_TYPE t, PyObject* container, enum Cont
             NULL
         );
         Py_XDECREF(py_value);
+        py_value = NULL;
+        // Wrapping runs Python code too, so it can fail with a RecursionError set.
+        if (final_py_value == NULL) {
+            _FAILWITH_PENDING_PYERR();
+        }
     }
 
-    ionc_add_to_container(container, final_py_value, parent_type, py_field_name);
+    IONCHECK(ionc_add_to_container(container, final_py_value, parent_type, py_field_name));
 
 fail:
     Py_XDECREF(py_annotations);
@@ -1456,6 +1508,11 @@ PyObject* ionc_read_iter_next(PyObject *self) {
 
 fail:
     Py_XDECREF(container);
+    // A RecursionError is reported as-is; simpleion.py translates it into an IonException. Any
+    // other pending exception is superseded by the IonException built below.
+    if (PyErr_ExceptionMatches(PyExc_RecursionError)) {
+        return NULL;
+    }
     PyObject* exception = PyErr_Format(_ion_exception_cls, "%s %s", ion_error_to_str(err), _err_msg);
     _err_msg[0] = '\0';
     return exception;