Last clean up, ready for merge back to 2.13

cowtowncoder · cowtowncoder · commit f93fd41028b6 · 2021-03-28T13:40:33.000-07:00
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/JsonNodeDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/JsonNodeDeserializer.java
@@ -13,6 +13,9 @@
 /**
  * Deserializer that can build instances of {@link JsonNode} from any
  * JSON content, using appropriate {@link JsonNode} type.
+ *<p>
+ * Rewritten in Jackson 2.13 to avoid recursion and allow handling of
+ * very deeply nested structures.
  */
 @SuppressWarnings("serial")
 public class JsonNodeDeserializer
@@ -48,7 +51,7 @@ public static JsonDeserializer<? extends JsonNode> getDeserializer(Class<?> node
 
     /*
     /**********************************************************************
-    /* Actual deserializer implementations
+    /* Actual deserialization method implementations
     /**********************************************************************
      */
 
@@ -69,16 +72,16 @@ public JsonNode deserialize(JsonParser p, DeserializationContext ctxt) throws IO
         final JsonNodeFactory nodeF = ctxt.getNodeFactory();
         switch (p.currentTokenId()) {
         case JsonTokenId.ID_START_OBJECT:
-            return deserializeContainerNonRecursive(p, ctxt, nodeF, stack, nodeF.objectNode());
+            return _deserializeContainerNoRecursion(p, ctxt, nodeF, stack, nodeF.objectNode());
         case JsonTokenId.ID_END_OBJECT:
             return nodeF.objectNode();
         case JsonTokenId.ID_START_ARRAY:
-            return deserializeContainerNonRecursive(p, ctxt, nodeF, stack, nodeF.arrayNode());
+            return _deserializeContainerNoRecursion(p, ctxt, nodeF, stack, nodeF.arrayNode());
         case JsonTokenId.ID_FIELD_NAME:
-            return deserializeObjectAtName(p, ctxt, nodeF, stack);
+            return _deserializeObjectAtName(p, ctxt, nodeF, stack);
         default:
         }
-        return deserializeAnyScalar(p, ctxt);
+        return _deserializeAnyScalar(p, ctxt);
     }
 
     /*
@@ -87,6 +90,9 @@ public JsonNode deserialize(JsonParser p, DeserializationContext ctxt) throws IO
     /**********************************************************************
      */
 
+    /**
+     * Implementation used when declared type is specifically {@link ObjectNode}.
+     */
     final static class ObjectDeserializer
         extends BaseNodeDeserializer<ObjectNode>
     {
@@ -104,11 +110,11 @@ public ObjectNode deserialize(JsonParser p, DeserializationContext ctxt) throws
             final JsonNodeFactory nodeF = ctxt.getNodeFactory();
             if (p.isExpectedStartObjectToken()) {
                 final ObjectNode root = nodeF.objectNode();
-                deserializeContainerNonRecursive(p, ctxt, nodeF, new ContainerStack(), root);
+                _deserializeContainerNoRecursion(p, ctxt, nodeF, new ContainerStack(), root);
                 return root;
             }
             if (p.hasToken(JsonToken.FIELD_NAME)) {
-                return deserializeObjectAtName(p, ctxt, nodeF, new ContainerStack());
+                return _deserializeObjectAtName(p, ctxt, nodeF, new ContainerStack());
             }
             // 23-Sep-2015, tatu: Ugh. We may also be given END_OBJECT (similar to FIELD_NAME),
             //    if caller has advanced to the first token of Object, but for empty Object
@@ -135,6 +141,9 @@ public ObjectNode deserialize(JsonParser p, DeserializationContext ctxt,
         }
     }
 
+    /**
+     * Implementation used when declared type is specifically {@link ArrayNode}.
+     */
     final static class ArrayDeserializer
         extends BaseNodeDeserializer<ArrayNode>
     {
@@ -152,7 +161,7 @@ public ArrayNode deserialize(JsonParser p, DeserializationContext ctxt) throws I
             if (p.isExpectedStartArrayToken()) {
                 final JsonNodeFactory nodeF = ctxt.getNodeFactory();
                 final ArrayNode arrayNode = nodeF.arrayNode();
-                deserializeContainerNonRecursive(p, ctxt, nodeF,
+                _deserializeContainerNoRecursion(p, ctxt, nodeF,
                         new ContainerStack(), arrayNode);
                 return arrayNode;
             }
@@ -167,7 +176,7 @@ public ArrayNode deserialize(JsonParser p, DeserializationContext ctxt,
                 ArrayNode arrayNode) throws IOException
         {
             if (p.isExpectedStartArrayToken()) {
-                deserializeContainerNonRecursive(p, ctxt, ctxt.getNodeFactory(),
+                _deserializeContainerNoRecursion(p, ctxt, ctxt.getNodeFactory(),
                         new ContainerStack(), arrayNode);
                 return arrayNode;
             }
@@ -177,8 +186,10 @@ public ArrayNode deserialize(JsonParser p, DeserializationContext ctxt,
 }
 
 /**
- * Base class for all actual {@link JsonNode} deserializer
- * implementations
+ * Base class for all actual {@link JsonNode} deserializer implementations.
+ *<p>
+ * Starting with Jackson 2.13 uses iteration instead of recursion: this allows
+ * handling of very deeply nested input structures.
  */
 @SuppressWarnings("serial")
 abstract class BaseNodeDeserializer<T extends JsonNode>
@@ -275,10 +286,8 @@ protected void _handleDuplicateField(JsonParser p, DeserializationContext ctxt,
     /**
      * Alternate deserialization method used when parser already points to first
      * FIELD_NAME and not START_OBJECT.
-     *
-     * @since 2.9
      */
-    protected final ObjectNode deserializeObjectAtName(JsonParser p, DeserializationContext ctxt,
+    protected final ObjectNode _deserializeObjectAtName(JsonParser p, DeserializationContext ctxt,
             final JsonNodeFactory nodeFactory, final ContainerStack stack) throws IOException
     {
         final ObjectNode node = nodeFactory.objectNode();
@@ -291,15 +300,15 @@ protected final ObjectNode deserializeObjectAtName(JsonParser p, Deserialization
             }
             switch (t.id()) {
             case JsonTokenId.ID_START_OBJECT:
-                value = deserializeContainerNonRecursive(p, ctxt, nodeFactory,
+                value = _deserializeContainerNoRecursion(p, ctxt, nodeFactory,
                         stack, nodeFactory.objectNode());
                 break;
             case JsonTokenId.ID_START_ARRAY:
-                value = deserializeContainerNonRecursive(p, ctxt, nodeFactory,
+                value = _deserializeContainerNoRecursion(p, ctxt, nodeFactory,
                         stack, nodeFactory.arrayNode());
                 break;
             default:
-                value = deserializeAnyScalar(p, ctxt);
+                value = _deserializeAnyScalar(p, ctxt);
             }
             JsonNode old = node.replace(key, value);
             if (old != null) {
@@ -352,7 +361,7 @@ protected final JsonNode updateObject(JsonParser p, DeserializationContext ctxt,
                     if (t == JsonToken.START_ARRAY) {
                         // 28-Mar-2021, tatu: We'll only append entries so not very different
                         //    from "regular" deserializeArray...
-                        deserializeContainerNonRecursive(p, ctxt, nodeFactory,
+                        _deserializeContainerNoRecursion(p, ctxt, nodeFactory,
                                 stack, (ArrayNode) old);
                         continue;
                     }
@@ -364,11 +373,11 @@ protected final JsonNode updateObject(JsonParser p, DeserializationContext ctxt,
             JsonNode value;
             switch (t.id()) {
             case JsonTokenId.ID_START_OBJECT:
-                value = deserializeContainerNonRecursive(p, ctxt, nodeFactory,
+                value = _deserializeContainerNoRecursion(p, ctxt, nodeFactory,
                         stack, nodeFactory.objectNode());
                 break;
             case JsonTokenId.ID_START_ARRAY:
-                value = deserializeContainerNonRecursive(p, ctxt, nodeFactory,
+                value = _deserializeContainerNoRecursion(p, ctxt, nodeFactory,
                         stack, nodeFactory.arrayNode());
                 break;
             case JsonTokenId.ID_STRING:
@@ -387,7 +396,7 @@ protected final JsonNode updateObject(JsonParser p, DeserializationContext ctxt,
                 value = nodeFactory.nullNode();
                 break;
             default:
-                value = deserializeRareScalar(p, ctxt);
+                value = _deserializeRareScalar(p, ctxt);
             }
             // 15-Feb-2021, tatu: I don't think this should have been called
             //   on update case (was until 2.12.2) and was simply result of
@@ -405,20 +414,20 @@ protected final JsonNode updateObject(JsonParser p, DeserializationContext ctxt,
 
     // Non-recursive alternative, used beyond certain nesting level
     // @since 2.13.0
-    protected final ContainerNode<?> deserializeContainerNonRecursive(JsonParser p, DeserializationContext ctxt,
-            JsonNodeFactory nodeFactory, ContainerStack stack, ContainerNode<?> root)
+    protected final ContainerNode<?> _deserializeContainerNoRecursion(JsonParser p, DeserializationContext ctxt,
+            JsonNodeFactory nodeFactory, ContainerStack stack, final ContainerNode<?> root)
         throws IOException
     {
         ContainerNode<?> curr = root;
         final int intCoercionFeats = ctxt.getDeserializationFeatures() & F_MASK_INT_COERCIONS;
 
         outer_loop:
-        while (true) {
-//            if (curr.isObject()) {
+        do {
             if (curr instanceof ObjectNode) {
-                final ObjectNode currObject = (ObjectNode) curr;
+                ObjectNode currObject = (ObjectNode) curr;
                 String propName = p.nextFieldName();
 
+                objectLoop:
                 for (; propName != null; propName = p.nextFieldName()) {
                     JsonNode value;
                     JsonToken t = p.nextToken();
@@ -435,9 +444,10 @@ protected final ContainerNode<?> deserializeContainerNonRecursive(JsonParser p,
                                         propName, currObject, old, newOb);
                             }
                             stack.push(curr);
-                            curr = newOb;
+                            curr = currObject = newOb;
+                            // We can actually take a short-cut with nested Objects...
+                            continue objectLoop;
                         }
-                        continue outer_loop;
                     case JsonTokenId.ID_START_ARRAY:
                         {
                             ArrayNode newOb = nodeFactory.arrayNode();
@@ -469,16 +479,17 @@ protected final ContainerNode<?> deserializeContainerNonRecursive(JsonParser p,
                         value = nodeFactory.nullNode();
                         break;
                     default:
-                        value = deserializeRareScalar(p, ctxt);
+                        value = _deserializeRareScalar(p, ctxt);
                     }
                     JsonNode old = currObject.replace(propName, value);
                     if (old != null) {
                         _handleDuplicateField(p, ctxt, nodeFactory,
                                 propName, currObject, old, value);
                     }
                 }
-                // reached not-property-name, should be END_OBJECT
+                // reached not-property-name, should be END_OBJECT (verify?)
             } else {
+                // Otherwise we must have an array
                 final ArrayNode currArray = (ArrayNode) curr;
 
                 arrayLoop:
@@ -519,22 +530,21 @@ protected final ContainerNode<?> deserializeContainerNonRecursive(JsonParser p,
                         currArray.add(nodeFactory.nullNode());
                         continue arrayLoop;
                     default:
-                        currArray.add(deserializeRareScalar(p, ctxt));
+                        currArray.add(_deserializeRareScalar(p, ctxt));
                         continue arrayLoop;
                     }
                 }
                 // Reached end of array (or input), so...
             }
-            if (stack.isEmpty()) {
-                return root;
-            }
-            curr = stack.pop();
-        }
-    }
 
+            // Either way, Object or Array ended, return up nesting level:
+            curr = stack.popOrNull();
+        } while (curr != null);
+        return root;
+    }
 
     // Was called "deserializeAny()" in 2.12 and prior
-    protected final JsonNode deserializeAnyScalar(JsonParser p, DeserializationContext ctxt)
+    protected final JsonNode _deserializeAnyScalar(JsonParser p, DeserializationContext ctxt)
         throws IOException
     {
         final JsonNodeFactory nodeF = ctxt.getNodeFactory();
@@ -562,7 +572,7 @@ protected final JsonNode deserializeAnyScalar(JsonParser p, DeserializationConte
         return (JsonNode) ctxt.handleUnexpectedToken(handledType(), p);
     }
 
-    protected final JsonNode deserializeRareScalar(JsonParser p, DeserializationContext ctxt)
+    protected final JsonNode _deserializeRareScalar(JsonParser p, DeserializationContext ctxt)
         throws IOException
     {
         // 28-Mar-2021, tatu: Only things that caller does not check
@@ -686,29 +696,33 @@ protected final JsonNode _fromEmbedded(JsonParser p, DeserializationContext ctxt
     final static class ContainerStack
     {
         private ContainerNode[] _stack;
-        private int _top;
+        private int _top, _end;
 
         public ContainerStack() { }
 
-        public boolean isEmpty() { return _top == 0; }
-
         // Not used yet but useful for limits (fail at [some high depth])
         public int size() { return _top; }
 
-        public void push(ContainerNode node) {
+        public void push(ContainerNode node)
+        {
+            if (_top < _end) {
+                _stack[_top++] = node;
+                return;
+            }
             if (_stack == null) {
-                _stack = new ContainerNode[10];
-            } else if (_stack.length == _top) {
+                _end = 10;
+                _stack = new ContainerNode[_end];
+            } else {
                 // grow by 50%, for most part
-                final int newSize = _top + Math.min(512, Math.max(10, _top>>1));
-                _stack = Arrays.copyOf(_stack, newSize);
+                _end += Math.min(4000, Math.max(20, _end>>1));
+                _stack = Arrays.copyOf(_stack, _end);
             }
             _stack[_top++] = node;
         }
 
-        public ContainerNode pop() {
+        public ContainerNode popOrNull() {
             if (_top == 0) {
-                throw new IllegalStateException("ContainerStack empty");
+                return null;
             }
             // note: could clean up stack but due to usage pattern, should not make
             // any difference -- all nodes joined during and after construction and
diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/dos/DeepJsonNodeDeser2816Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/dos/DeepJsonNodeDeser2816Test.java
@@ -0,0 +1,46 @@
+package com.fasterxml.jackson.databind.deser.dos;
+
+import com.fasterxml.jackson.databind.*;
+
+// [databind#2816], wrt JsonNode
+public class DeepJsonNodeDeser2816Test extends BaseMapTest
+{
+    // 28-Mar-2021, tatu: Used to fail at 5000 for tree/object,
+    // 8000 for tree/array, before work on iterative JsonNode deserializer
+    // ... currently gets a bit slow at 1M but passes
+//    private final static int TOO_DEEP_NESTING = 1_000_000;
+    private final static int TOO_DEEP_NESTING = 9999;
+
+    private final ObjectMapper MAPPER = newJsonMapper();
+
+    public void testTreeWithArray() throws Exception
+    {
+        final String doc = _nestedDoc(TOO_DEEP_NESTING, "[ ", "] ");
+        JsonNode n = MAPPER.readTree(doc);
+        assertTrue(n.isArray());
+    }
+
+    public void testTreeWithObject() throws Exception
+    {
+        final String doc = "{"+_nestedDoc(TOO_DEEP_NESTING, "\"x\":{", "} ") + "}";
+        JsonNode n = MAPPER.readTree(doc);
+        assertTrue(n.isObject());
+    }
+    
+    private String _nestedDoc(int nesting, String open, String close) {
+        StringBuilder sb = new StringBuilder(nesting * (open.length() + close.length()));
+        for (int i = 0; i < nesting; ++i) {
+            sb.append(open);
+            if ((i & 31) == 0) {
+                sb.append("\n");
+            }
+        }
+        for (int i = 0; i < nesting; ++i) {
+            sb.append(close);
+            if ((i & 31) == 0) {
+                sb.append("\n");
+            }
+        }
+        return sb.toString();
+    }
+}
diff --git a/src/test/java/com/fasterxml/jackson/failing/DeepNestingUntypedDeser2816Test.java b/src/test/java/com/fasterxml/jackson/failing/DeepNestingUntypedDeser2816Test.java
@@ -5,12 +5,12 @@
 
 import com.fasterxml.jackson.databind.*;
 
-// [databind#2816]
-public class DeepNestingDeser2816Test extends BaseMapTest
+// [databind#2816] wrt "untyped" (Maps, Lists)
+public class DeepNestingUntypedDeser2816Test extends BaseMapTest
 {
-    // 2000 passes for all; 3000 fails for untyped, 5000 for tree/object,
-    // 8000 for tree/array too
-    private final static int TOO_DEEP_NESTING = 8000;
+    // 28-Mar-2021, tatu: Currently 3000 fails for untyped/Object,
+    //     4000 for untyped/Array
+    private final static int TOO_DEEP_NESTING = 4000;
 
     private final ObjectMapper MAPPER = newJsonMapper();
 
@@ -28,20 +28,6 @@ public void testUntypedWithObject() throws Exception
         assertTrue(ob instanceof Map<?,?>);
     }
 
-    public void testTreeWithArray() throws Exception
-    {
-        final String doc = _nestedDoc(TOO_DEEP_NESTING, "[ ", "] ");
-        JsonNode n = MAPPER.readTree(doc);
-        assertTrue(n.isArray());
-    }
-
-    public void testTreeWithObject() throws Exception
-    {
-        final String doc = "{"+_nestedDoc(TOO_DEEP_NESTING, "\"x\":{", "} ") + "}";
-        JsonNode n = MAPPER.readTree(doc);
-        assertTrue(n.isObject());
-    }
-    
     private String _nestedDoc(int nesting, String open, String close) {
         StringBuilder sb = new StringBuilder(nesting * (open.length() + close.length()));
         for (int i = 0; i < nesting; ++i) {
