]> Piment Noir Git Repositories - e-mobility-charging-stations-simulator.git/commitdiff
fix(utils): clamp computeExponentialBackOffDelay output to setTimeout-safe range...
authorJérôme Benoit <jerome.benoit@piment-noir.org>
Sun, 5 Jul 2026 18:30:45 +0000 (20:30 +0200)
committerGitHub <noreply@github.com>
Sun, 5 Jul 2026 18:30:45 +0000 (20:30 +0200)
Fix a latent overflow bug in `computeExponentialBackOffDelay` that turned intended exponential backoff into a tight-loop retry storm at high retry counts.

The function returned `baseDelayMs * 2^retry + jitter` unclamped. With
`baseDelayMs = 100` (default) it overflows `MAX_SETINTERVAL_DELAY_MS`
(2_147_483_647 ms, ~24.8 days) at retry ≥25; with `baseDelayMs = 60_000`
(BootNotification interval default) it overflows at retry ≥16. Node.js
`setTimeout` silently coerces out-of-range delays to 1 ms per its docs,
producing a tight-loop reconnect/retry storm instead of the intended
exponential wait.

Fix: wrap the return value with `clampToSafeTimerValue()` inside
`computeExponentialBackOffDelay`. Single-site DRY defense covering all
5 present and future callers.

Call sites verified (5):
- ChargingStation.ts:1608 getReconnectDelay — vulnerable
  (wsConnectionRetryCount unbounded when autoReconnectMaxRetries = -1)
- ChargingStation.ts:2532 onOpen BootNotification retry — vulnerable
  (registrationRetryCount unbounded when registrationMaxRetries = -1)
- ChargingStation.ts:2788 sendMessageBuffer — vulnerable
  (messageIdx unbounded)
- OCPP20ServiceUtils.ts:253 computeReconnectDelay — safe by
  construction (maxRetries: RetryBackOffRepeatTimes)
- OCPP20CertSigningRetryManager.ts:96 scheduleNextRetry — safe by
  construction (maxRetries: CertSigningRepeatTimes)

Extends `computeExponentialBackOffDelay` JSDoc `@returns` to advertise
the clamp guarantee explicitly:
`delay in milliseconds, guaranteed within [0, Constants.MAX_SETINTERVAL_DELAY_MS]`.

Regression tests added at retry 25 (base 100 ms), retry 30 (extreme),
and a 20-sample jitter loop at retry 25 with `jitterPercent: 0.2`.
At retry ≥25 the pre-jitter delay already exceeds the ceiling, so the
jitter loop asserts strict equality with MAX (deterministic).

Priority-3 anchor comments preserved: mathematical formula anchor for
the overflow boundary and clamp-after-jitter ordering invariant.

Quality gates: typecheck clean, lint clean, tests/utils/Utils.test.ts
54/54 pass.

Diff: +34/-3 across 2 files.

Reviews applied (3 rounds, 4 agents each): 8 substantive findings
across v1+v2 addressed; v3 saturation HIGH declared by 4/4 agents
(0 substantive findings, 6 false positives rejected via caller-trace).

src/utils/Utils.ts
tests/utils/Utils.test.ts

index b8212e7ab4c21b2d1be19204541cf049d65c527f..1f94218ca75548b5c22ccd5e69503fdfab419888 100644 (file)
@@ -418,14 +418,15 @@ export const insertAt = (str: string, subStr: string, pos: number): string =>
   `${str.slice(0, pos)}${subStr}${str.slice(pos)}`
 
 /**
- * Generalized exponential back-off: baseDelayMs × 2^min(retryNumber, maxRetries) + jitter.
+ * Generalized exponential back-off: baseDelayMs × 2^min(retryNumber, maxRetries) + jitter,
+ * clamped to [0, Constants.MAX_SETINTERVAL_DELAY_MS] for setTimeout/setInterval safety.
  * @param options - back-off configuration
  * @param options.baseDelayMs - base delay in milliseconds
  * @param options.retryNumber - current retry attempt (0-based)
  * @param options.maxRetries - stop doubling after this many retries (default: unlimited)
  * @param options.jitterMs - maximum fixed random jitter in milliseconds (default: 0)
  * @param options.jitterPercent - proportional jitter as fraction of computed delay, e.g. 0.2 = 20% (default: 0)
- * @returns delay in milliseconds
+ * @returns delay in milliseconds, guaranteed within [0, Constants.MAX_SETINTERVAL_DELAY_MS]
  */
 export const computeExponentialBackOffDelay = (options: {
   baseDelayMs: number
@@ -443,7 +444,7 @@ export const computeExponentialBackOffDelay = (options: {
   } else if (jitterMs != null && jitterMs > 0) {
     jitter = secureRandom() * jitterMs
   }
-  return delay + jitter
+  return clampToSafeTimerValue(delay + jitter)
 }
 
 /**
index 068320e38d25636462119f2026f7f77126579f83..fae297f4484df3d9e105d26d3c294305d0f8adb2 100644 (file)
@@ -706,6 +706,36 @@ await describe('Utils', async () => {
       const maxDelay = computeExponentialBackOffDelay({ baseDelayMs, retryNumber: 10 })
       assert.strictEqual(maxDelay, 102400) // ~102 seconds
     })
+
+    await it('should clamp overflowing delays to MAX_SETINTERVAL_DELAY_MS', () => {
+      // At retry 25 with base 100ms: 100 * 2^25 = 3_355_443_200 ms > MAX_SETINTERVAL_DELAY_MS
+      // Without clamping, setTimeout would silently coerce to 1 ms (Node.js docs), producing
+      // a tight-loop reconnect storm instead of the intended exponential backoff.
+      const overflowDelay = computeExponentialBackOffDelay({
+        baseDelayMs: 100,
+        retryNumber: 25,
+      })
+      assert.strictEqual(overflowDelay, Constants.MAX_SETINTERVAL_DELAY_MS)
+
+      const extremeDelay = computeExponentialBackOffDelay({
+        baseDelayMs: 100,
+        retryNumber: 30,
+      })
+      assert.strictEqual(extremeDelay, Constants.MAX_SETINTERVAL_DELAY_MS)
+
+      // Clamping applies AFTER jitter is added — jitter cannot push past the safe ceiling.
+      // At retry 25 with base 100ms, `delay = 100 * 2^25 = 3.35e9` already exceeds the ceiling
+      // before jitter, so every iteration pins exactly to MAX_SETINTERVAL_DELAY_MS regardless
+      // of the random draw.
+      for (let i = 0; i < 20; i++) {
+        const jitteredDelay = computeExponentialBackOffDelay({
+          baseDelayMs: 100,
+          jitterPercent: 0.2,
+          retryNumber: 25,
+        })
+        assert.strictEqual(jitteredDelay, Constants.MAX_SETINTERVAL_DELAY_MS)
+      }
+    })
   })
 
   await it('should return timestamped log prefix with optional string', () => {