Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,57 @@ public void shouldDecreaseDepth() {
assertEquals(1, counter.getCounter());
}

/**
* Regression for SRQ-31346.
* <p>
* 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.
* <p>
* 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
Expand Down