This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 8159dfe66 ThresholdCircuitBreaker accepts negative increments and
overflows its accumulator, silently keeping the breaker closed (f027).
8159dfe66 is described below
commit 8159dfe661a064707e9b4de92aba1b952f3370d7
Author: Gary Gregory <[email protected]>
AuthorDate: Sat Sep 5 15:39:48 2026 -0400
ThresholdCircuitBreaker accepts negative increments and overflows its
accumulator, silently keeping the breaker closed (f027).
---
src/changes/changes.xml | 1 +
.../lang3/concurrent/EventCountCircuitBreaker.java | 10 +++++++++
.../lang3/concurrent/ThresholdCircuitBreaker.java | 26 +++++++++++++++++-----
.../concurrent/EventCountCircuitBreakerTest.java | 11 +++++++++
.../concurrent/ThresholdCircuitBreakerTest.java | 23 +++++++++++++++++++
5 files changed, 66 insertions(+), 5 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 70dc9d021..be119519d 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -276,6 +276,7 @@ java.lang.NullPointerException: Cannot invoke
<action type="fix" dev="ggregory" due-to="Gary
Gregory">FormattableUtils.append pads right-justified output with insert(0) per
char; O(width^2); '%500000s' costs ~1.25e11 char moves (f024).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">Attacker-controlled exception message forges frames in
getRootCauseStackTrace output AND suppresses all real frames, beyond cosmetic
log spoofing (f025).</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">StrBuilder.indexOf materializes the whole builder as a String per
call; deleteAll/replaceAll multiply it into ~750 GB churn on a 1 MB builder
(f026).</action>
+ <action type="fix" dev="ggregory" due-to="Gary
Gregory">ThresholdCircuitBreaker accepts negative increments and overflows its
accumulator, silently keeping the breaker closed (f027).</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add JavaVersion.JAVA_27.</action>
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add SystemUtils.IS_JAVA_27.</action>
diff --git
a/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
b/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
index 5c4386713..3741f56fa 100644
---
a/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
+++
b/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
@@ -478,9 +478,19 @@ public boolean incrementAndCheckState() {
/**
* {@inheritDoc}
+ *
+ * <p>
+ * The event count is a protective counter and only moves toward the
opening threshold:
+ * negative increments are rejected.
+ * </p>
+ *
+ * @throws IllegalArgumentException if the increment is negative.
*/
@Override
public boolean incrementAndCheckState(final Integer increment) {
+ if (increment.intValue() < 0) {
+ throw new IllegalArgumentException("Increment must not be
negative: " + increment);
+ }
return performStateCheck(increment);
}
diff --git
a/src/main/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreaker.java
b/src/main/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreaker.java
index 235b4d144..a7a213e6f 100644
---
a/src/main/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreaker.java
+++
b/src/main/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreaker.java
@@ -108,19 +108,35 @@ public long getThreshold() {
/**
* {@inheritDoc}
*
- * <p>If the threshold is zero, the circuit breaker will be in a permanent
<em>open</em> state.</p>
+ * <p>
+ * If the threshold is zero, the circuit breaker will be in a permanent
<em>open</em> state.
+ * </p>
+ * <p>
+ * The internal counter is a protective counter and only moves toward the
threshold: negative
+ * increments are rejected, and an increment that would overflow {@link
Long#MAX_VALUE} saturates
+ * the counter at {@link Long#MAX_VALUE} and opens the circuit breaker
instead of silently wrapping
+ * negative (which would disable the trip condition).
+ * </p>
+ *
+ * @throws IllegalArgumentException if the increment is negative.
*/
@Override
public boolean incrementAndCheckState(final Long increment) {
if (threshold == 0) {
open();
}
-
- final long used = this.used.addAndGet(increment);
- if (used > threshold) {
+ final long delta = increment.longValue();
+ if (delta < 0) {
+ throw new IllegalArgumentException("Increment must not be
negative: " + delta);
+ }
+ final long used = this.used.accumulateAndGet(delta, (current, add) -> {
+ final long next = current + add;
+ // Both operands are non-negative, so overflow shows up as a
decrease: saturate.
+ return next < current ? Long.MAX_VALUE : next;
+ });
+ if (used > threshold || used == Long.MAX_VALUE) {
open();
}
-
return checkState();
}
diff --git
a/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
b/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
index aab088648..8bbe62e72 100644
---
a/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
+++
b/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
@@ -20,6 +20,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.beans.PropertyChangeEvent;
@@ -256,6 +257,16 @@ void testIntervalCalculation() {
assertEquals(2 * NANO_FACTOR / 1000, breaker.getClosingInterval(),
"Wrong closing interval");
}
+ /**
+ * Tests that a negative increment is rejected: the event count must only
move toward
+ * the opening threshold.
+ */
+ @Test
+ void testNegativeIncrementRejected() {
+ final EventCountCircuitBreaker breaker = new
EventCountCircuitBreaker(OPENING_THRESHOLD, 1, TimeUnit.SECONDS);
+ assertThrows(IllegalArgumentException.class, () ->
breaker.incrementAndCheckState(-1), "Negative increments must be rejected");
+ }
+
/**
* Tests that an open circuit breaker does not close itself when the
number of events
* received is over the threshold.
diff --git
a/src/test/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreakerTest.java
b/src/test/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreakerTest.java
index 7159f2816..8e90b4ae7 100644
---
a/src/test/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreakerTest.java
+++
b/src/test/java/org/apache/commons/lang3/concurrent/ThresholdCircuitBreakerTest.java
@@ -18,6 +18,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import org.apache.commons.lang3.AbstractLangTest;
@@ -56,6 +57,28 @@ void testGettingThreshold() {
assertEquals(Long.valueOf(threshold),
Long.valueOf(circuit.getThreshold()), "Wrong value of threshold");
}
+ /**
+ * Tests that a negative increment is rejected: the counter must only move
toward the threshold.
+ */
+ @Test
+ void testNegativeIncrementRejected() {
+ final ThresholdCircuitBreaker circuit = new
ThresholdCircuitBreaker(threshold);
+ assertThrows(IllegalArgumentException.class, () ->
circuit.incrementAndCheckState(-1L), "Negative increments must be rejected");
+ }
+
+ /**
+ * Tests that overflowing the internal counter saturates and opens the
circuit breaker instead of wrapping negative and silently disabling the trip
+ * condition.
+ */
+ @Test
+ void testOverflowSaturatesAndOpens() {
+ final ThresholdCircuitBreaker circuit = new
ThresholdCircuitBreaker(Long.MAX_VALUE);
+ circuit.incrementAndCheckState(Long.MAX_VALUE);
+ // Without saturation this would wrap negative and report the circuit
closed forever.
+ assertFalse(circuit.incrementAndCheckState(1L), "Overflow must open
the circuit, not wrap the counter");
+ assertFalse(circuit.checkState(), "Circuit must stay open after
overflow");
+ }
+
/**
* Tests that the threshold is working as expected when incremented and no
exception is thrown.
*/
