fix: v4 Signing Errors with exactly 7 day expiry (#2170)

* fix: if expiry date object has >= 500 ms then a rounding error occurs and is thrown. added tests to replicate. * fix: typo * fix: smplifiy the unittest
googleapis · Mar 30, 2023 · f930998 · f930998
1 parent 130818d
commit f930998
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 4 deletions.
diff --git a/src/signer.ts b/src/signer.ts
@@ -416,7 +416,7 @@ export class URLSigner {
       throw new Error(ExceptionMessages.EXPIRATION_DATE_PAST);
     }
 
-    return Math.round(expiresInMSeconds / 1000); // The API expects seconds.
+    return Math.floor(expiresInMSeconds / 1000); // The API expects seconds.
   }
 
   parseAccessibleAt(accessibleAt?: string | number | Date): number {

diff --git a/system-test/storage.ts b/system-test/storage.ts
@@ -3290,6 +3290,31 @@ describe('storage', () => {
       assert.strictEqual(body, localFile.toString());
     });
 
+    it('should not throw with expiration of exactly 7 days', async () => {
+      const ACCESSIBLE_AT = new Date().setMilliseconds(999).valueOf();
+      const SEVEN_DAYS_IN_SECONDS = 7 * 24 * 60 * 60;
+      const SEVEN_DAYS_IN_MS = SEVEN_DAYS_IN_SECONDS * 1000;
+      await assert.doesNotReject(
+        async () => {
+          await file.getSignedUrl({
+            version: 'v4',
+            action: 'read',
+            accessibleAt: ACCESSIBLE_AT,
+            expires: ACCESSIBLE_AT + SEVEN_DAYS_IN_MS,
+            virtualHostedStyle: true,
+          });
+        },
+        err => {
+          assert(err instanceof Error);
+          assert.strictEqual(
+            err.message,
+            `Max allowed expiration is seven days (${SEVEN_DAYS_IN_SECONDS} seconds).`
+          );
+          return true;
+        }
+      );
+    });
+
     it('should create a signed read url with accessibleAt in the past', async () => {
       const [signedReadUrl] = await file.getSignedUrl({
         version: 'v4',

diff --git a/test/signer.ts b/test/signer.ts
@@ -56,7 +56,7 @@ describe('signer', () => {
     let bucket: BucketI;
     let file: FileI;
 
-    const NOW = new Date('2019-03-18T00:00:00Z');
+    const NOW = new Date('2019-03-18T00:00:00.999Z');
     let fakeTimers: sinon.SinonFakeTimers;
 
     beforeEach(() => (fakeTimers = sinon.useFakeTimers(NOW)));
@@ -554,7 +554,7 @@ describe('signer', () => {
         signer = new URLSigner(authClient, bucket, file);
         CONFIG = {
           method: 'GET',
-          expiration: Math.floor((NOW.valueOf() + 2000) / 1000),
+          expiration: (NOW.valueOf() + 2000) / 1000,
           bucket: bucket.name,
         };
       });
@@ -573,6 +573,30 @@ describe('signer', () => {
         );
       });
 
+      it('should not throw with expiration of exactly 7 days', async () => {
+        const ACCESSIBLE_AT = NOW.valueOf();
+        const SEVEN_DAYS_IN_SECONDS = 7 * 24 * 60 * 60;
+        const SEVEN_DAYS_IN_MS = SEVEN_DAYS_IN_SECONDS * 1000;
+        await assert.doesNotReject(
+          async () => {
+            await signer.getSignedUrl({
+              method: 'GET',
+              expires: ACCESSIBLE_AT + SEVEN_DAYS_IN_MS,
+              accessibleAt: ACCESSIBLE_AT,
+              version: 'v4',
+            });
+          },
+          err => {
+            assert(err instanceof Error);
+            assert.strictEqual(
+              err.message,
+              `Max allowed expiration is seven days (${SEVEN_DAYS_IN_SECONDS} seconds).`
+            );
+            return true;
+          }
+        );
+      });
+
       describe('headers', () => {
         it('should add path-styled host header', async () => {
           const getCanonicalHeaders = sandbox
@@ -988,7 +1012,7 @@ describe('signer', () => {
 
       it('returns expiration date in seconds', () => {
         const expires = signer.parseExpires(NOW);
-        assert.strictEqual(expires, Math.round(NOW.valueOf() / 1000));
+        assert.strictEqual(expires, Math.floor(NOW.valueOf() / 1000));
       });
     });
   });