diff --git a/dxa-framework/dxa-common/src/main/java/com/sdl/dxa/common/dto/DepthCounter.java b/dxa-framework/dxa-common/src/main/java/com/sdl/dxa/common/dto/DepthCounter.java index 70a73443f..34d384576 100644 --- a/dxa-framework/dxa-common/src/main/java/com/sdl/dxa/common/dto/DepthCounter.java +++ b/dxa-framework/dxa-common/src/main/java/com/sdl/dxa/common/dto/DepthCounter.java @@ -29,7 +29,17 @@ public synchronized boolean isNotTooDeep() { } public synchronized boolean depthIncreaseAndCheckIfSafe() { - return isNotTooDeep() && counter-- > 0; + // Counter must be decremented unconditionally so it stays balanced with + // the unconditional increment in depthDecrease() that callers run in + // their finally blocks. The previous implementation short-circuited the + // decrement via && when counter reached 0, causing every failed check + // to leak +1 of budget back into the counter — which allowed the model + // expander to recurse past its intended depth limit. See SRQ-31346. + if (unlimited) { + return true; + } + counter--; + return counter >= 0; } public synchronized boolean depthIncreaseAndCheckIfSafe(int levels) { diff --git a/dxa-framework/dxa-common/src/test/java/com/sdl/dxa/common/dto/DepthCounterTest.java b/dxa-framework/dxa-common/src/test/java/com/sdl/dxa/common/dto/DepthCounterTest.java index 9dfbf47f8..7fe748d1c 100644 --- a/dxa-framework/dxa-common/src/test/java/com/sdl/dxa/common/dto/DepthCounterTest.java +++ b/dxa-framework/dxa-common/src/test/java/com/sdl/dxa/common/dto/DepthCounterTest.java @@ -45,6 +45,57 @@ public void shouldDecreaseDepth() { assertEquals(1, counter.getCounter()); } + /** + * Regression for SRQ-31346. + *
+ * Callers of {@link DepthCounter#depthIncreaseAndCheckIfSafe()} pair each + * call with an unconditional {@link DepthCounter#depthDecrease()} in a + * finally block. For the counter to stay balanced, the increment-and-check + * method must decrement on every call, including when the check fails. + *
+ * The previous implementation short-circuited the decrement via {@code &&} + * when the counter reached zero. The {@code finally}-block decrement then + * leaked +1 of budget back into the counter on every "too deep" event, + * letting the page model expander recurse past its intended depth limit + * and ultimately overflow the JVM stack on heavily nested page models. + */ + @Test + public void depthIncreaseAndCheckIfSafe_doesNotLeakWhenLimitExceeded() { + DepthCounter counter = new DepthCounter(2); + + // Budget gets consumed. + assertTrue(counter.depthIncreaseAndCheckIfSafe()); + assertTrue(counter.depthIncreaseAndCheckIfSafe()); + assertEquals(0, counter.getCounter()); + + // Past the limit: check must fail AND counter must still be decremented + // so the matching depthDecrease() restores it cleanly. + assertFalse(counter.depthIncreaseAndCheckIfSafe()); + counter.depthDecrease(); + assertEquals(0, counter.getCounter()); + + // Repeated failures stay balanced — no accumulating budget leak. + for (int i = 0; i < 100; i++) { + assertFalse(counter.depthIncreaseAndCheckIfSafe()); + counter.depthDecrease(); + } + assertEquals(0, counter.getCounter()); + } + + /** + * Unlimited counters must not be decremented on each call — otherwise + * Integer.MAX_VALUE - 1 calls would silently flip them into limited mode. + */ + @Test + public void depthIncreaseAndCheckIfSafe_unlimitedAlwaysSafe() { + DepthCounter counter = DepthCounter.UNLIMITED_DEPTH; + + for (int i = 0; i < 1000; i++) { + assertTrue(counter.depthIncreaseAndCheckIfSafe()); + } + assertEquals(MAX_VALUE, counter.getCounter()); + } + @Test public void shouldBeUnlimited_MaxValue() { //given