Merge pull request #239 from stephenplusplus/spp--signed-url-test

storage: fix signed url tests. fixes #238

lib/storage/index.js

@@ -325,6 +325,8 @@ Bucket.prototype.remove = function(name, callback) {
  *
  * {@link https://developers.google.com/storage/docs/accesscontrol#Signed-URLs}
  *
+ * @throws {Error} if an expiration timestamp from the past is given.
+ *
  * @param {object} options - Configuration object.
  * @param {string} options.action - "read", "write", or "delete"
  * @param {string=} options.contentMd5 - The MD5 digest value in base64. If you
@@ -346,6 +348,10 @@ Bucket.prototype.remove = function(name, callback) {
  * }, function(err, url) {});
  */
 Bucket.prototype.getSignedUrl = function(options, callback) {
+  if (options.expires < Math.floor(Date.now() / 1000)) {
+    throw new Error('An expiration date cannot be in the past.');
+  }
+
   options.action = {
       read: 'GET',
       write: 'PUT',

regression/storage.js

@@ -224,25 +224,5 @@ describe('storage', function() {
           });
         });
     });
-
-    it('should allow control of expiration', function(done) {
-      var offsetSeconds = 5;
-      bucket.getSignedUrl({
-          action: 'read',
-          expires: Math.round(Date.now() / 1000) + offsetSeconds,
-          resource: filename
-        }, function(err, signedReadUrl) {
-          assert.ifError(err);
-          request.get(signedReadUrl, function(err, resp, body) {
-            assert.equal(body, localFile);
-          });
-          setTimeout(function() {
-            request.get(signedReadUrl, function(err, resp) {
-              assert.equal(resp.statusCode, 400);
-              bucket.remove(filename, done);
-            });
-          }, (offsetSeconds + 1) * 1000);
-        });
-    });
   });
 });

test/storage/index.js

@@ -21,6 +21,7 @@
 var assert = require('assert');
 var gcloud = require('../../lib');
 var storage = require('../../lib/storage');
+var url = require('url');
 
 var credentials = require('../testdata/privateKeyFile.json');
 var noop = function() {};
@@ -142,15 +143,46 @@ describe('Bucket', function() {
     bucket.remove('file-name');
   });
 
-  it('should create a signed url', function(done) {
-    bucket.getSignedUrl({
+  describe('getSignedUrl', function() {
+    it('should create a signed url', function(done) {
+      bucket.getSignedUrl({
         action: 'read',
-        resource: 'filename',
-        expires: Date.now() / 1000
-      }, function(err, url) {
+        expires: Math.round(Date.now() / 1000) + 5,
+        resource: 'filename'
+      }, function(err, signedUrl) {
         assert.ifError(err);
-        assert.equal(typeof url, 'string');
+        assert.equal(typeof signedUrl, 'string');
         done();
       });
+    });
+
+    describe('expires', function() {
+      var nowInSeconds = Math.floor(Date.now() / 1000);
+
+      it('should use the provided expiration date', function(done) {
+        var expirationTimestamp = nowInSeconds + 60;
+        bucket.getSignedUrl({
+          action: 'read',
+          resource: 'filename',
+          expires: expirationTimestamp
+        }, function(err, signedUrl) {
+          assert.ifError(err);
+          var expires = url.parse(signedUrl, true).query.Expires;
+          assert.equal(expires, expirationTimestamp);
+          done();
+        });
+      });
+
+      it('should throw if a date from the past is given', function() {
+        var expirationTimestamp = nowInSeconds - 1;
+        assert.throws(function() {
+          bucket.getSignedUrl({
+            action: 'read',
+            resource: 'filename',
+            expires: expirationTimestamp
+          }, function() {});
+        }, /cannot be in the past/);
+      });
+    });
   });
 });