fix bug: if expiry date object has >= 500 ms then a rounding error oc…

…curs and is thrown. added tests to replicate.
googleapis · Mar 27, 2023 · a6ef06c · a6ef06c
1 parent 2c967f6
commit a6ef06c
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 5 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
@@ -29,7 +29,7 @@ import {
   SignerExceptionMessages,
 } from '../src/signer';
 import {encodeURI, formatAsUTCISO, qsStringify} from '../src/util';
-import {ExceptionMessages} from '../src/storage';
+import {Storage, ExceptionMessages} from '../src/storage';
 import {OutgoingHttpHeaders} from 'http';
 
 interface SignedUrlArgs {
@@ -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,52 @@ describe('signer', () => {
         );
       });
 
+      it('should not throw with expiration of exactly 7 days', async () => {
+        const file = new Storage({
+          projectId: 'xxxx',
+          credentials: {
+            type: 'service_account',
+            private_key: crypto.generateKeyPairSyn c('rsa', {
+              modulusLength: 512,
+              publicKeyEncoding: {
+                type: 'spki',
+                format: 'pem',
+              },
+              privateKeyEncoding: {
+                type: 'pkcs8',
+                format: 'pem',
+              },
+            }).privateKey,
+            client_email: 'xxxx',
+            client_id: 'xxx',
+          },
+        })
+          .bucket(BUCKET_NAME)
+          .file(FILE_NAME);
+        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 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;
+          }
+        );
+      });
+
       describe('headers', () => {
         it('should add path-styled host header', async () => {
           const getCanonicalHeaders = sandbox
@@ -988,7 +1034,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));
       });
     });
   });