ARSN-552: don't throw in case of bad Date inputs by leif-scality · Pull Request #2591 · scality/Arsenal

leif-scality · 2026-02-11T11:54:03Z

Don't throw and crash cloudserver in case of an invalid Date header

Repro code:

curl -v -H "Date: BAD_DATE" -H "Authorization: AWS4-HMAC-SHA256 Credential=accessKey1/20260211/us-east-1/s3/aws4_request, SignedHeaders=host, Signature=d459d5b2a2395b4c65d8f8aa2729b22c5abb04614fafbd93ab4fe203e76d21a3" -H "X-Amz-Content-Sha256: fa8d015f89da2a769d1cea7e3bd77a5670d098d7844cda148a40c1304e5b778b" localhost:8000/leif-us-east-1/test-file.txt

2026-02-11T09:33:06.467Z: uncaughtException:
RangeError: Invalid time value
    at Date.toISOString (<anonymous>)
    at convertUTCtoISO8601 (/app/node_modules/arsenal/build/lib/auth/v4/timeUtils.js:26:43)
    at Object.check [as headers] (/app/node_modules/arsenal/build/lib/auth/v4/headerAuthCheck.js:98:57)
    at extractParams (/app/node_modules/arsenal/build/lib/auth/auth.js:122:47)
    at Object.doAuth (/app/node_modules/arsenal/build/lib/auth/auth.js:140:17)
    at /app/lib/api/api.js:238:33
    at nextTask (/app/node_modules/async/dist/async.js:5327:14)
    at Object.waterfall (/app/node_modules/async/dist/async.js:5337:5)
    at processRequest (/app/lib/api/api.js:237:22)
    at Object.callApiMethod (/app/lib/api/api.js:418:16)
{"name":"S3","time":1770802386467,"error":"Invalid time value","stack":"RangeError: Invalid time value\n    at Date.toISOString (<anonymous>)\n    at convertUTCtoISO8601 (/app/node_modules/arsenal/build/lib/auth/v4/timeUtils.js:26:43)\n    at Object.check [as headers] (/app/node_modules/arsenal/build/lib/auth/v4/headerAuthCheck.js:98:57)\n    at extractParams (/app/node_modules/arsenal/build/lib/auth/auth.js:122:47)\n    at Object.doAuth (/app/node_modules/arsenal/build/lib/auth/auth.js:140:17)\n    at /app/lib/api/api.js:238:33\n    at nextTask (/app/node_modules/async/dist/async.js:5327:14)\n    at Object.waterfall (/app/node_modules/async/dist/async.js:5337:5)\n    at processRequest (/app/lib/api/api.js:237:22)\n    at Object.callApiMethod (/app/lib/api/api.js:418:16)","level":"fatal","message":"caught error","hostname":"xps","pid":886}

bert-e · 2026-02-11T11:54:06Z

Hello leif-scality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-02-11T11:54:11Z

Incorrect fix version

The Fix Version/s in issue ARSN-552 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.2.46
8.3.2

Please check the Fix Version/s of ARSN-552, or the target
branch of this pull request.

codecov · 2026-02-11T12:54:04Z

Codecov Report

❌ Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.43%. Comparing base (af610f2) to head (a81fb2d).

Files with missing lines	Patch %	Lines
lib/auth/v4/timeUtils.ts	90.90%	2 Missing ⚠️
lib/auth/v4/headerAuthCheck.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##           development/8.2    #2591      +/-   ##
===================================================
+ Coverage            71.41%   71.43%   +0.01%     
===================================================
  Files                  222      222              
  Lines                17918    17937      +19     
  Branches              3728     3733       +5     
===================================================
+ Hits                 12797    12813      +16     
- Misses                5117     5120       +3     
  Partials                 4        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bert-e · 2026-02-11T15:33:00Z

Incorrect fix version

The Fix Version/s in issue ARSN-552 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

8.2.46
8.3.3

Please check the Fix Version/s of ARSN-552, or the target
branch of this pull request.

lib/auth/v4/headerAuthCheck.ts

BourgoisMickael · 2026-02-11T16:41:28Z

lib/auth/v4/timeUtils.ts

+    // Check length
+    if (str.length !== 16) {
+        return false;
+    }
+
+    // Check 'T' at position 8 and 'Z' at position 15
+    if (str[8] !== 'T' || str[15] !== 'Z') {
+        return false;
+    }
+
+    // Check all other positions are digits
+    for (let i = 0; i < 16; i++) {
+        if (i === 8 || i === 15) {
+            continue; // Skip T and Z
+        }
+        if (str[i] < '0' || str[i] > '9') {
+            return false;
+        }
+    }


This could be simplified and more maintainable with a regex.

Ideally we should use a date library for date manipulation or validation, but I see we don't have any in arsenal

Do we want to compile and run a regex for each request? I agree the regex is simpler, but the code is not that complicated

I tend to agree with @BourgoisMickael , I'd prefer a regexp, it's cleaner and less error-prone (and maybe more efficient too, but that would have to be tested). Normally if you use a static regexp (/.../) it's pre-compiled automatically.

Also you can use the result of the regexp parsing to extract components, instead of using substring.

changed to regex

BourgoisMickael · 2026-02-11T16:44:17Z

lib/auth/v4/timeUtils.ts

+* convertUTCtoISO8601(1328910895000); // '20120210T213455Z'
+* convertUTCtoISO8601('invalid'); // undefined
+*/
+export function convertUTCtoISO8601(timestamp: unknown): string | undefined {


instead of unknown the type should be string | number | null (maybe even | undefined but there is no check for undefined.

replaced with string | number

BourgoisMickael · 2026-02-11T17:00:37Z

lib/auth/auth.ts

    Object.assign(request, { headers: {} });
-    const amzDate = convertUTCtoISO8601(Date.now());
+    // Date.now() should always return a valid date so we assert non null.
+    const amzDate: string = convertUTCtoISO8601(Date.now())!;


if there is a non-null assertion, then you don't need to add the type string, it's the only type left as output of the function

removed the type

BourgoisMickael · 2026-02-11T17:42:10Z

tests/unit/auth/v4/timeUtils.spec.js

+    it('should convert Date.now() to ISO8601 timestamp', () => {
+        const now = Date.now();


prefer a fixed date like the test above should be enough, this test doesn't provide more coverage

removed the test we already have a test with new Date

BourgoisMickael · 2026-02-11T17:43:15Z

tests/unit/auth/v4/timeUtils.spec.js

+    it('should return undefined for object', () => {
+        const actualOutput = convertUTCtoISO8601({});


this should be a typescript error if the function is typed correctly

the function accepts any even in typescript so we need to test the case. Even with the type string | number

BourgoisMickael · 2026-02-11T17:45:46Z

tests/unit/auth/v4/timeUtils.spec.js

+            const input = '20160208T201405Z';
+            assert.strictEqual(isValidISO8601Compact(input), true);


Suggested change

const input = '20160208T201405Z';

assert.strictEqual(isValidISO8601Compact(input), true);

assert.strictEqual(isValidISO8601Compact('20160208T201405Z'), true);

you can go one line like the describe a little below here

refactored to array

BourgoisMickael · 2026-02-11T17:49:30Z

tests/unit/auth/v4/timeUtils.spec.js

+        });
+    });
+
+    describe('null/undefined/non-string inputs', () => {


anything other than string should raise a typescript error here, you should not be able to use another type

Oh the file is javascript, that's why you have no typescript error in there

the function accepts any even in typescript so we need to test the case. Even with the type string | number

BourgoisMickael · 2026-02-11T17:54:06Z

tests/unit/auth/v4/timeUtils.spec.js

+    });
+
+    describe('invalid format', () => {
+        it('should return false for string with wrong length', () => {


you could use an array of (test name, input) and loop over it

[ { name: 'should return false for string with wrong length', input: '2016020T201405Z' }, // ... ].forEach(t => it(t.name, () => assert.strictEqual(isValidISO8601Compact(t.input), false))

jonathan-gramain · 2026-02-13T01:20:24Z

lib/auth/v4/timeUtils.ts

+    // Check length
+    if (str.length !== 16) {
+        return false;
+    }
+
+    // Check 'T' at position 8 and 'Z' at position 15
+    if (str[8] !== 'T' || str[15] !== 'Z') {
+        return false;
+    }
+
+    // Check all other positions are digits
+    for (let i = 0; i < 16; i++) {
+        if (i === 8 || i === 15) {
+            continue; // Skip T and Z
+        }
+        if (str[i] < '0' || str[i] > '9') {
+            return false;
+        }
+    }


I tend to agree with @BourgoisMickael , I'd prefer a regexp, it's cleaner and less error-prone (and maybe more efficient too, but that would have to be tested). Normally if you use a static regexp (/.../) it's pre-compiled automatically.

Also you can use the result of the regexp parsing to extract components, instead of using substring.

jonathan-gramain · 2026-02-13T01:24:38Z

lib/auth/v4/timeUtils.ts

+    try {
+        // date.toISOString() can throw.
+        // date.toISOString() === isoString check prevents silent date corrections (30 February to 1 March)
+        return !isNaN(date.getTime()) && date.toISOString() === isoString;


nitpick: you can use Number.isNaN to be strict about checking exclusively a NaN value (rather than anything that is not parsable to a valid number)

changed to Number.isNaN

jonathan-gramain · 2026-02-13T01:27:36Z

tests/unit/auth/v4/timeUtils.spec.js

 const convertUTCtoISO8601 =
    require('../../../../lib/auth/v4/timeUtils').convertUTCtoISO8601;
+const isValidISO8601Compact =
+    require('../../../../lib/auth/v4/timeUtils').isValidISO8601Compact;


Suggested change

require('../../../../lib/auth/v4/timeUtils').isValidISO8601Compact;

const {

checkTimeSkew,

convertAmzTimeToMs,

convertUTCtoISO8601,

isValidISO8601Compact,

} = require('../../../../lib/auth/v4/timeUtils');

jonathan-gramain · 2026-02-13T21:02:29Z

lib/auth/v4/timeUtils.ts

+* ```
+*/
+export function isValidISO8601Compact(str: string): boolean {
+    if (str == null || typeof str !== 'string') {


nitpick: typeof str !== 'string' would suffice

leif-scality force-pushed the bugfix/ARSN-552-auth-v4-bad-date-throw branch 3 times, most recently from c4ded38 to 4bfcb47 Compare February 11, 2026 12:45

leif-scality changed the title ~~ARSN-522: don't throw in case of bad Date inputs~~ ARSN-552: don't throw in case of bad Date inputs Feb 11, 2026

leif-scality force-pushed the bugfix/ARSN-552-auth-v4-bad-date-throw branch from 4bfcb47 to 4be2cc6 Compare February 11, 2026 14:21

BourgoisMickael reviewed Feb 11, 2026

View reviewed changes

lib/auth/v4/headerAuthCheck.ts Show resolved Hide resolved

leif-scality force-pushed the bugfix/ARSN-552-auth-v4-bad-date-throw branch from 1c8bc34 to c208871 Compare February 11, 2026 16:34

BourgoisMickael reviewed Feb 11, 2026

View reviewed changes

leif-scality force-pushed the bugfix/ARSN-552-auth-v4-bad-date-throw branch from c208871 to a3b28b8 Compare February 12, 2026 13:01

tmacro approved these changes Feb 12, 2026

View reviewed changes

jonathan-gramain reviewed Feb 13, 2026

View reviewed changes

leif-scality added 2 commits February 13, 2026 10:59

ARSN-552: don't throw in case of bad Date inputs

34b503c

ARSN-552: bump package.json version

a81fb2d

leif-scality force-pushed the bugfix/ARSN-552-auth-v4-bad-date-throw branch from b0bb4dd to a81fb2d Compare February 13, 2026 09:59

jonathan-gramain approved these changes Feb 13, 2026

View reviewed changes

		it('should convert Date.now() to ISO8601 timestamp', () => {
		const now = Date.now();

		it('should return undefined for object', () => {
		const actualOutput = convertUTCtoISO8601({});

		const input = '20160208T201405Z';
		assert.strictEqual(isValidISO8601Compact(input), true);

-    require('../../../../lib/auth/v4/timeUtils').isValidISO8601Compact;
+const {
+    checkTimeSkew,
+    convertAmzTimeToMs,
+    convertUTCtoISO8601,
+    isValidISO8601Compact,
+} = require('../../../../lib/auth/v4/timeUtils');

Conversation

leif-scality commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bert-e commented Feb 11, 2026

Hello leif-scality,

Uh oh!

bert-e commented Feb 11, 2026

Incorrect fix version

Uh oh!

codecov bot commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bert-e commented Feb 11, 2026

Incorrect fix version

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leif-scality Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leif-scality Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

leif-scality commented Feb 11, 2026 •

edited

Loading

codecov bot commented Feb 11, 2026 •

edited

Loading

leif-scality Feb 11, 2026 •

edited

Loading

leif-scality Feb 11, 2026 •

edited

Loading