From 0ff03a18dfc925bfb271f69b1a92e9c1afe27310 Mon Sep 17 00:00:00 2001
From: Stephen Sawchuk <sawchuk@gmail.com>
Date: Tue, 23 Sep 2014 13:34:08 -0400
Subject: [PATCH] storage: fix signed url tests. fixes #238

---
 lib/storage/index.js  |  6 ++++++
 regression/storage.js | 20 --------------------
 test/storage/index.js | 44 +++++++++++++++++++++++++++++++++++++------
 3 files changed, 44 insertions(+), 26 deletions(-)

diff --git a/lib/storage/index.js b/lib/storage/index.js
index 6a517042275..83c4e6a2147 100644
--- a/lib/storage/index.js
+++ b/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',
diff --git a/regression/storage.js b/regression/storage.js
index ab39409b21e..72f771a067d 100644
--- a/regression/storage.js
+++ b/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);
-        });
-    });
   });
 });
diff --git a/test/storage/index.js b/test/storage/index.js
index eda3489ca40..a3448314f20 100644
--- a/test/storage/index.js
+++ b/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/);
+      });
+    });
   });
 });