XRENDERING-771: Null pointer exception when list items occur without lists (#327)

* Protect against list items without wrapping list.
* Log a debug message to make it possible to debug this problem.
* Add tests.

(cherry picked from commit 5f70b11)

Author: michitux

xwiki-rendering-api/src/main/java/org/xwiki/rendering/listener/chaining/BlockStateChainingListener.java

@@ -23,6 +23,8 @@
 import java.util.Deque;
 import java.util.Map;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.xwiki.rendering.listener.Format;
 import org.xwiki.rendering.listener.HeaderLevel;
 import org.xwiki.rendering.listener.ListType;
@@ -38,6 +40,8 @@
  */
 public class BlockStateChainingListener extends AbstractChainingListener implements StackableChainingListener
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(BlockStateChainingListener.class);
+
     public enum Event
     {
         NONE,
@@ -278,7 +282,12 @@ public void beginDocument(MetaData metadata)
     public void beginDefinitionDescription()
     {
         ++this.inlineDepth;
-        ++this.definitionListDepth.peek().definitionListItemIndex;
+        if (!this.definitionListDepth.isEmpty()) {
+            ++this.definitionListDepth.peek().definitionListItemIndex;
+        } else if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Invalid nesting: definition description outside definition list.",
+                new IllegalStateException());
+        }
 
         super.beginDefinitionDescription();
 
@@ -304,7 +313,11 @@ public void beginDefinitionList(Map<String, String> parameters)
     public void beginDefinitionTerm()
     {
         ++this.inlineDepth;
-        ++this.definitionListDepth.peek().definitionListItemIndex;
+        if (!this.definitionListDepth.isEmpty()) {
+            ++this.definitionListDepth.peek().definitionListItemIndex;
+        } else if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Invalid nesting: definition term outside definition list.", new IllegalStateException());
+        }
 
         super.beginDefinitionTerm();
 
@@ -340,7 +353,11 @@ public void beginList(ListType type, Map<String, String> parameters)
     public void beginListItem()
     {
         ++this.inlineDepth;
-        ++this.listDepth.peek().listItemIndex;
+        if (!this.listDepth.isEmpty()) {
+            ++this.listDepth.peek().listItemIndex;
+        } else if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Invalid nesting: list item outside list.", new IllegalStateException());
+        }
 
         super.beginListItem();
 
@@ -351,7 +368,11 @@ public void beginListItem()
     public void beginListItem(Map<String, String> parameters)
     {
         ++this.inlineDepth;
-        ++this.listDepth.peek().listItemIndex;
+        if (!this.listDepth.isEmpty()) {
+            ++this.listDepth.peek().listItemIndex;
+        } else if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Invalid nesting: list item with parameters outside list.", new IllegalStateException());
+        }
 
         super.beginListItem(parameters);
 

xwiki-rendering-api/src/test/java/org/xwiki/rendering/listener/chaining/BlockStateChainingListenerTest.java

@@ -21,17 +21,23 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Map;
 
 import org.apache.commons.text.CaseUtils;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.mockito.stubbing.Stubber;
 import org.xwiki.rendering.listener.ListType;
 import org.xwiki.rendering.listener.Listener;
 import org.xwiki.rendering.listener.MetaData;
+import org.xwiki.test.LogLevel;
+import org.xwiki.test.junit5.LogCaptureExtension;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.Mockito.doAnswer;
@@ -50,6 +56,9 @@ class BlockStateChainingListenerTest
 
     private ChainingListener mockListener;
 
+    @RegisterExtension
+    private LogCaptureExtension logCaptureExtension = new LogCaptureExtension(LogLevel.DEBUG);
+
     @BeforeEach
     void setUpChain()
     {
@@ -188,4 +197,61 @@ void testOnMethod(Method method, Object[] parameters) throws InvocationTargetExc
 
         this.listener.endDocument(MetaData.EMPTY);
     }
+
+    @ParameterizedTest
+    @ValueSource(booleans = { true, false })
+    void testListItemWithoutParent(boolean withParameter) throws Exception
+    {
+        Class<?>[] parameterTypes = withParameter ? new Class<?>[] { Map.class } : new Class<?>[0];
+        Method beginMethod = BlockStateChainingListener.class.getDeclaredMethod("beginListItem", parameterTypes);
+        Method endMethod = BlockStateChainingListener.class.getDeclaredMethod("endListItem", parameterTypes);
+        Object[] parameters = withParameter ? new Object[] { Listener.EMPTY_PARAMETERS } : new Object[0];
+
+        this.listener.beginDocument(MetaData.EMPTY);
+        beginMethod.invoke(this.listener, parameters);
+        assertEquals(-1, this.listener.getListItemIndex());
+        assertFalse(this.listener.isInList());
+        assertEquals(0, this.listener.getListDepth());
+        endMethod.invoke(this.listener, parameters);
+        beginMethod.invoke(this.listener, parameters);
+        assertEquals(-1, this.listener.getListItemIndex());
+        assertFalse(this.listener.isInList());
+        assertEquals(0, this.listener.getListDepth());
+        endMethod.invoke(this.listener, parameters);
+        this.listener.endDocument(MetaData.EMPTY);
+
+        assertEquals(2, this.logCaptureExtension.size());
+        String expectedMessage =
+            "Invalid nesting: list item" + (withParameter ? " with parameters" : "") + " outside list.";
+        for (int i = 0; i < 2; ++i) {
+            assertEquals(expectedMessage, this.logCaptureExtension.getLogEvent(i).getMessage());
+        }
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "Term", "Description" })
+    void testDefinitionListItemWithoutParent(String methodSuffix) throws Exception
+    {
+        Method beginMethod = BlockStateChainingListener.class.getDeclaredMethod("beginDefinition" + methodSuffix);
+        Method endMethod = BlockStateChainingListener.class.getDeclaredMethod("endDefinition" + methodSuffix);
+        this.listener.beginDocument(MetaData.EMPTY);
+        beginMethod.invoke(this.listener);
+        assertFalse(this.listener.isInDefinitionList());
+        assertEquals(-1, this.listener.getDefinitionListItemIndex());
+        assertEquals(0, this.listener.getDefinitionListDepth());
+        endMethod.invoke(this.listener);
+        beginMethod.invoke(this.listener);
+        assertFalse(this.listener.isInDefinitionList());
+        assertEquals(-1, this.listener.getDefinitionListItemIndex());
+        assertEquals(0, this.listener.getDefinitionListDepth());
+        endMethod.invoke(this.listener);
+        this.listener.endDocument(MetaData.EMPTY);
+
+        assertEquals(2, this.logCaptureExtension.size());
+        String expectedMessage =
+            "Invalid nesting: definition " + methodSuffix.toLowerCase() + " outside definition list.";
+        for (int i = 0; i < 2; ++i) {
+            assertEquals(expectedMessage, this.logCaptureExtension.getLogEvent(i).getMessage());
+        }
+    }
 }