From 79d0b484010f75eea87ecd1e01a1ccd7ed43bbdb Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Thu, 23 Apr 2026 09:00:11 +0000 Subject: [PATCH] fix: RetryPolicy constructor rejects NaN and Infinity numeric parameters RetryPolicy's constructor validated numeric parameters using comparison operators (<=, <) which silently accept NaN because all NaN comparisons return false in JavaScript. For example, NaN <= 0 evaluates to false, so NaN maxNumberOfAttempts passed validation and caused infinite retries since attemptCount >= NaN is always false. Add Number.isFinite() guards before all comparison checks to reject NaN, Infinity, and -Infinity for all numeric parameters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/task/retry/retry-policy.ts | 22 ++-- .../durabletask-js/test/retry-policy.spec.ts | 119 +++++++++++++++++- 2 files changed, 125 insertions(+), 16 deletions(-) diff --git a/packages/durabletask-js/src/task/retry/retry-policy.ts b/packages/durabletask-js/src/task/retry/retry-policy.ts index 8dfb45c..f257b2d 100644 --- a/packages/durabletask-js/src/task/retry/retry-policy.ts +++ b/packages/durabletask-js/src/task/retry/retry-policy.ts @@ -65,32 +65,34 @@ export class RetryPolicy { } = options; // Validation aligned with .NET SDK - if (maxNumberOfAttempts <= 0) { - throw new Error("maxNumberOfAttempts must be greater than zero"); + // Use !Number.isFinite() guards to reject NaN and Infinity, which bypass + // comparison operators silently (e.g. NaN <= 0 is false). + if (!Number.isFinite(maxNumberOfAttempts) || maxNumberOfAttempts <= 0) { + throw new Error("maxNumberOfAttempts must be a finite number greater than zero"); } - if (firstRetryIntervalInMilliseconds <= 0) { - throw new Error("firstRetryIntervalInMilliseconds must be greater than zero"); + if (!Number.isFinite(firstRetryIntervalInMilliseconds) || firstRetryIntervalInMilliseconds <= 0) { + throw new Error("firstRetryIntervalInMilliseconds must be a finite number greater than zero"); } - if (backoffCoefficient < 1.0) { - throw new Error("backoffCoefficient must be greater than or equal to 1.0"); + if (!Number.isFinite(backoffCoefficient) || backoffCoefficient < 1.0) { + throw new Error("backoffCoefficient must be a finite number greater than or equal to 1.0"); } if ( maxRetryIntervalInMilliseconds !== undefined && maxRetryIntervalInMilliseconds !== -1 && - maxRetryIntervalInMilliseconds < firstRetryIntervalInMilliseconds + (!Number.isFinite(maxRetryIntervalInMilliseconds) || maxRetryIntervalInMilliseconds < firstRetryIntervalInMilliseconds) ) { - throw new Error("maxRetryIntervalInMilliseconds must be greater than or equal to firstRetryIntervalInMilliseconds"); + throw new Error("maxRetryIntervalInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); } if ( retryTimeoutInMilliseconds !== undefined && retryTimeoutInMilliseconds !== -1 && - retryTimeoutInMilliseconds < firstRetryIntervalInMilliseconds + (!Number.isFinite(retryTimeoutInMilliseconds) || retryTimeoutInMilliseconds < firstRetryIntervalInMilliseconds) ) { - throw new Error("retryTimeoutInMilliseconds must be greater than or equal to firstRetryIntervalInMilliseconds"); + throw new Error("retryTimeoutInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); } this._maxNumberOfAttempts = maxNumberOfAttempts; diff --git a/packages/durabletask-js/test/retry-policy.spec.ts b/packages/durabletask-js/test/retry-policy.spec.ts index d7a87e0..20fae90 100644 --- a/packages/durabletask-js/test/retry-policy.spec.ts +++ b/packages/durabletask-js/test/retry-policy.spec.ts @@ -45,7 +45,7 @@ describe("RetryPolicy", () => { maxNumberOfAttempts: 0, firstRetryIntervalInMilliseconds: 1000, }); - }).toThrow("maxNumberOfAttempts must be greater than zero"); + }).toThrow("maxNumberOfAttempts must be a finite number greater than zero"); }); it("should throw error when firstRetryIntervalInMilliseconds is zero", () => { @@ -55,7 +55,7 @@ describe("RetryPolicy", () => { maxNumberOfAttempts: 3, firstRetryIntervalInMilliseconds: 0, }); - }).toThrow("firstRetryIntervalInMilliseconds must be greater than zero"); + }).toThrow("firstRetryIntervalInMilliseconds must be a finite number greater than zero"); }); it("should throw error when firstRetryIntervalInMilliseconds is negative", () => { @@ -65,7 +65,7 @@ describe("RetryPolicy", () => { maxNumberOfAttempts: 3, firstRetryIntervalInMilliseconds: -100, }); - }).toThrow("firstRetryIntervalInMilliseconds must be greater than zero"); + }).toThrow("firstRetryIntervalInMilliseconds must be a finite number greater than zero"); }); it("should throw error when backoffCoefficient is less than 1.0", () => { @@ -76,7 +76,7 @@ describe("RetryPolicy", () => { firstRetryIntervalInMilliseconds: 1000, backoffCoefficient: 0.5, }); - }).toThrow("backoffCoefficient must be greater than or equal to 1.0"); + }).toThrow("backoffCoefficient must be a finite number greater than or equal to 1.0"); }); it("should throw error when maxRetryIntervalInMilliseconds is less than firstRetryInterval", () => { @@ -87,7 +87,7 @@ describe("RetryPolicy", () => { firstRetryIntervalInMilliseconds: 1000, maxRetryIntervalInMilliseconds: 500, }); - }).toThrow("maxRetryIntervalInMilliseconds must be greater than or equal to firstRetryIntervalInMilliseconds"); + }).toThrow("maxRetryIntervalInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); }); it("should throw error when retryTimeoutInMilliseconds is less than firstRetryInterval", () => { @@ -98,7 +98,114 @@ describe("RetryPolicy", () => { firstRetryIntervalInMilliseconds: 1000, retryTimeoutInMilliseconds: 500, }); - }).toThrow("retryTimeoutInMilliseconds must be greater than or equal to firstRetryIntervalInMilliseconds"); + }).toThrow("retryTimeoutInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); + }); + + describe("NaN and Infinity rejection", () => { + it("should throw error when maxNumberOfAttempts is NaN", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: NaN, + firstRetryIntervalInMilliseconds: 1000, + }); + }).toThrow("maxNumberOfAttempts must be a finite number greater than zero"); + }); + + it("should throw error when maxNumberOfAttempts is Infinity", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: Infinity, + firstRetryIntervalInMilliseconds: 1000, + }); + }).toThrow("maxNumberOfAttempts must be a finite number greater than zero"); + }); + + it("should throw error when firstRetryIntervalInMilliseconds is NaN", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: NaN, + }); + }).toThrow("firstRetryIntervalInMilliseconds must be a finite number greater than zero"); + }); + + it("should throw error when firstRetryIntervalInMilliseconds is Infinity", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: Infinity, + }); + }).toThrow("firstRetryIntervalInMilliseconds must be a finite number greater than zero"); + }); + + it("should throw error when backoffCoefficient is NaN", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: 1000, + backoffCoefficient: NaN, + }); + }).toThrow("backoffCoefficient must be a finite number greater than or equal to 1.0"); + }); + + it("should throw error when backoffCoefficient is Infinity", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: 1000, + backoffCoefficient: Infinity, + }); + }).toThrow("backoffCoefficient must be a finite number greater than or equal to 1.0"); + }); + + it("should throw error when maxRetryIntervalInMilliseconds is NaN", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: 1000, + maxRetryIntervalInMilliseconds: NaN, + }); + }).toThrow("maxRetryIntervalInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); + }); + + it("should throw error when maxRetryIntervalInMilliseconds is Infinity", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: 1000, + maxRetryIntervalInMilliseconds: Infinity, + }); + }).toThrow("maxRetryIntervalInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); + }); + + it("should throw error when retryTimeoutInMilliseconds is NaN", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: 1000, + retryTimeoutInMilliseconds: NaN, + }); + }).toThrow("retryTimeoutInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); + }); + + it("should throw error when retryTimeoutInMilliseconds is Infinity", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: 3, + firstRetryIntervalInMilliseconds: 1000, + retryTimeoutInMilliseconds: Infinity, + }); + }).toThrow("retryTimeoutInMilliseconds must be a finite number greater than or equal to firstRetryIntervalInMilliseconds"); + }); + + it("should throw error when maxNumberOfAttempts is negative Infinity", () => { + expect(() => { + new RetryPolicy({ + maxNumberOfAttempts: -Infinity, + firstRetryIntervalInMilliseconds: 1000, + }); + }).toThrow("maxNumberOfAttempts must be a finite number greater than zero"); + }); }); it("should accept exactly 1.0 as backoffCoefficient", () => {