XWIKI-14613: Dashboard macro can break with a stackoverflow

xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/main/java/org/xwiki/rendering/internal/macro/dashboard/DashboardMacro.java

@@ -33,6 +33,8 @@
 import org.xwiki.component.annotation.Component;
 import org.xwiki.component.manager.ComponentLookupException;
 import org.xwiki.component.manager.ComponentManager;
+import org.xwiki.context.Execution;
+import org.xwiki.context.ExecutionContext;
 import org.xwiki.rendering.block.Block;
 import org.xwiki.rendering.block.GroupBlock;
 import org.xwiki.rendering.internal.macro.script.NestedScriptMacroEnabled;
@@ -120,6 +122,8 @@ public class DashboardMacro extends AbstractMacro<DashboardMacroParameters> impl
      */
     private static final String DESCRIPTION = "A macro to define a dashboard.";
 
+    private static final String DASHBOARD_MACRO_CALLS = "dashboardMacroCalls";
+
     /**
      * CSS file skin extension, to include the dashboard css.
      */
@@ -152,6 +156,9 @@ public class DashboardMacro extends AbstractMacro<DashboardMacroParameters> impl
     @Inject
     private Logger logger;
 
+    @Inject
+    private Execution execution;
+
     /**
      * Instantiates the dashboard macro, setting the name, description and parameters type.
      */
@@ -165,6 +172,9 @@ public DashboardMacro()
     public List<Block> execute(DashboardMacroParameters parameters, String content, MacroTransformationContext context)
         throws MacroExecutionException
     {
+        // We don't allow calling the Dashboard macro inside the Dashboard macro to prevent recursions!
+        preventDashboardRecursion();
+
         // get the gadgets from the objects
         List<Gadget> gadgets;
         try {
@@ -216,9 +226,43 @@ public List<Block> execute(DashboardMacroParameters parameters, String content,
         topLevel.setParameter("class",
             MACRO_NAME + (StringUtils.isEmpty(parameters.getStyle()) ? "" : " " + parameters.getStyle()));
 
+        // Reduce by 1 the recursive count so that we can have several dashboard macros rendered in the same context
+        reduceDashboardRecursionCounter();
+
         return Collections.<Block> singletonList(topLevel);
     }
 
+    private void preventDashboardRecursion() throws MacroExecutionException
+    {
+        ExecutionContext ec = this.execution.getContext();
+        if (ec != null) {
+            Integer dashboardCalls = (Integer) ec.getProperty(DASHBOARD_MACRO_CALLS);
+            if (dashboardCalls != null) {
+                if (dashboardCalls > 0) {
+                    throw new MacroExecutionException(
+                        "Dashboard macro recursion detected. Don't call the Dashboard macro inside of itself...");
+                } else {
+                    ec.setProperty(DASHBOARD_MACRO_CALLS, dashboardCalls + 1);
+                }
+            } else {
+                ec.setProperty(DASHBOARD_MACRO_CALLS, 1);
+            }
+        }
+    }
+
+    private void reduceDashboardRecursionCounter() throws MacroExecutionException
+    {
+        ExecutionContext ec = this.execution.getContext();
+        if (ec != null) {
+            Integer dashboardCalls = (Integer) ec.getProperty(DASHBOARD_MACRO_CALLS);
+            if (dashboardCalls != null) {
+                if (dashboardCalls > 0) {
+                    ec.setProperty(DASHBOARD_MACRO_CALLS, dashboardCalls - 1);
+                }
+            }
+        }
+    }
+
     /**
      * Includes the js and css resources for the dashboard macro.
      * 

xwiki-platform-core/xwiki-platform-dashboard/xwiki-platform-dashboard-macro/src/test/java/org/xwiki/rendering/internal/macro/dashboard/DashboardMacroTest.java

+/*
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.xwiki.rendering.internal.macro.dashboard;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.xwiki.context.Execution;
+import org.xwiki.context.ExecutionContext;
+import org.xwiki.properties.BeanDescriptor;
+import org.xwiki.properties.BeanManager;
+import org.xwiki.rendering.block.Block;
+import org.xwiki.rendering.macro.MacroExecutionException;
+import org.xwiki.rendering.macro.dashboard.DashboardMacroParameters;
+import org.xwiki.rendering.macro.dashboard.DashboardRenderer;
+import org.xwiki.rendering.macro.dashboard.GadgetRenderer;
+import org.xwiki.rendering.transformation.MacroTransformationContext;
+import org.xwiki.test.mockito.MockitoComponentMockingRule;
+
+import static junit.framework.TestCase.fail;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link DashboardMacro}.
+ *
+ * @version $Id$
+ */
+public class DashboardMacroTest
+{
+    @Rule
+    public MockitoComponentMockingRule<DashboardMacro> mocker =
+        new MockitoComponentMockingRule<>(DashboardMacro.class);
+
+    @Test
+    public void executeWhenInsideDashboardMacro() throws Exception
+    {
+        BeanManager beanManager = this.mocker.getInstance(BeanManager.class);
+        BeanDescriptor descriptor = mock(BeanDescriptor.class);
+        when(beanManager.getBeanDescriptor(any())).thenReturn(descriptor);
+        when(descriptor.getProperties()).thenReturn(Collections.emptyList());
+
+        Execution execution = this.mocker.getInstance(Execution.class);
+        ExecutionContext ec = new ExecutionContext();
+        when(execution.getContext()).thenReturn(ec);
+        ec.setProperty("dashboardMacroCalls", 1);
+
+        DashboardMacroParameters parameters = new DashboardMacroParameters();
+        MacroTransformationContext macroContext = new MacroTransformationContext();
+
+        try {
+            this.mocker.getComponentUnderTest().execute(parameters, "", macroContext);
+            fail("Exception should have been raised here");
+        } catch (MacroExecutionException expected) {
+            assertEquals("Dashboard macro recursion detected. Don't call the Dashboard macro inside of itself...",
+                expected.getMessage());
+        }
+    }
+
+    @Test
+    public void executeWhenNotInsideDashboardMacro() throws Exception
+    {
+        BeanManager beanManager = this.mocker.getInstance(BeanManager.class);
+        BeanDescriptor descriptor = mock(BeanDescriptor.class);
+        when(beanManager.getBeanDescriptor(any())).thenReturn(descriptor);
+        when(descriptor.getProperties()).thenReturn(Collections.emptyList());
+
+        DashboardRenderer renderer = this.mocker.registerMockComponent(DashboardRenderer.class, "columns");
+
+        GadgetRenderer gadgetRenderer = this.mocker.registerMockComponent(GadgetRenderer.class);
+
+        Execution execution = this.mocker.getInstance(Execution.class);
+        ExecutionContext ec = new ExecutionContext();
+        when(execution.getContext()).thenReturn(ec);
+
+        DashboardMacroParameters parameters = new DashboardMacroParameters();
+        MacroTransformationContext macroContext = new MacroTransformationContext();
+        this.mocker.getComponentUnderTest().execute(parameters, "", macroContext);
+
+        // We verify that the counter ends up at 0 so that calls to subsequent dashboard macros can succeed.
+        assertEquals(0, ec.getProperty("dashboardMacroCalls"));
+    }
+}