fix(behavior): reject null children in behavior tree JSON at load time

engine-tests/src/test/java/org/terasology/engine/logic/behavior/core/BehaviorTreeBuilderTest.java

@@ -0,0 +1,50 @@
+// Copyright 2021 The Terasology Foundation
+// SPDX-License-Identifier: Apache-2.0
+package org.terasology.engine.logic.behavior.core;
+
+import com.google.gson.JsonParseException;
+import org.junit.jupiter.api.Test;
+import org.terasology.engine.logic.behavior.actions.InvertAction;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Regression coverage for https://github.com/MovingBlocks/Terasology/issues/5099: a malformed behavior tree
+ * (e.g. a stray/trailing comma producing a null array entry) used to parse "successfully" with a silent
+ * {@code null} child, only to crash later with an unrelated NPE deep in tree execution/copying
+ * ({@link org.terasology.engine.logic.behavior.core.SelectorNode#deepCopy()} or
+ * {@link org.terasology.engine.logic.behavior.DefaultBehaviorTreeRunner}). It should instead fail fast, at
+ * load time, with a message that points at the malformed tree.
+ */
+public class BehaviorTreeBuilderTest {
+
+    private final BehaviorTreeBuilder builder = new BehaviorTreeBuilder();
+
+    @Test
+    public void validTreeStillParses() {
+        BehaviorNode node = builder.fromJson("{ selector: [success, failure, success] }");
+
+        assertEquals(3, node.getChildrenCount());
+    }
+
+    @Test
+    public void nullChildInCompositeArrayFailsLoudlyInsteadOfLater() {
+        JsonParseException exception = assertThrows(JsonParseException.class,
+                () -> builder.fromJson("{ selector: [success, null, success] }"));
+
+        assertTrue(exception.getMessage().contains("selector"));
+        assertTrue(exception.getMessage().contains("index 1"));
+    }
+
+    @Test
+    public void nullChildInDecoratorFailsLoudlyInsteadOfLater() {
+        builder.registerDecorator("invert", InvertAction.class);
+
+        JsonParseException exception = assertThrows(JsonParseException.class,
+                () -> builder.fromJson("{ invert: { child: null } }"));
+
+        assertTrue(exception.getMessage().contains("invert"));
+    }
+}

engine/src/main/java/org/terasology/engine/logic/behavior/core/BehaviorTreeBuilder.java

@@ -231,12 +231,20 @@ private BehaviorNode getCompositeNode(JsonElement json, JsonDeserializationConte
             addAction((ActionNode) node, action);
             JsonElement childJson = jsonElement.getAsJsonObject().get("child");
             BehaviorNode child = context.deserialize(childJson, BehaviorNode.class);
+            if (child == null) {
+                throw new JsonParseException("Malformed behavior tree: decorator '" + type
+                        + "' has no valid child (check for a missing/null \"child\" entry)");
+            }
             node.insertChild(0, child);
         } else if (jsonElement.isJsonArray()) {
             List<BehaviorNode> children = context.deserialize(jsonElement, new TypeToken<List<BehaviorNode>>() {
             }.getType());
             for (int i = 0; i < children.size(); i++) {
                 BehaviorNode child = children.get(i);
+                if (child == null) {
+                    throw new JsonParseException("Malformed behavior tree: '" + type
+                            + "' has a null child at index " + i + " (check for a stray/trailing comma)");
+                }
                 node.insertChild(i, child);
             }
         }