Append newline to end of extensionHeaders if necessary

aknuds1 · stephenplusplus · commit 17e510b32b0b · 2016-05-16T07:44:30.000-04:00
diff --git a/lib/storage/file.js b/lib/storage/file.js
@@ -1144,25 +1144,25 @@ File.prototype.getSignedPolicy = function(options, callback) {
  *
  * @throws {Error} if an expiration timestamp from the past is given.
  *
- * @param {object} options - Configuration object.
- * @param {string} options.action - "read" (HTTP: GET), "write" (HTTP: PUT), or
+ * @param {object} config - Configuration object.
+ * @param {string} config.action - "read" (HTTP: GET), "write" (HTTP: PUT), or
  *     "delete" (HTTP: DELETE).
- * @param {string=} options.contentMd5 - The MD5 digest value in base64. If you
+ * @param {string=} config.contentMd5 - The MD5 digest value in base64. If you
  *     provide this, the client must provide this HTTP header with this same
  *     value in its request.
- * @param {string=} options.contentType - If you provide this value, the client
+ * @param {string=} config.contentType - If you provide this value, the client
  *     must provide this HTTP header set to the same value.
- * @param {*} options.expires - A timestamp when this link will expire. Any
- *     value given is passed to `new Date()`.
- * @param {string=} options.extensionHeaders - If these headers are used, the
+ * @param {*} config.expires - A timestamp when this link will expire. Any value
+ *     given is passed to `new Date()`.
+ * @param {object=} config.extensionHeaders - If these headers are used, the
  *     server will check to make sure that the client provides matching values.
- * @param {string=} options.promptSaveAs - The filename to prompt the user to
+ * @param {string=} config.promptSaveAs - The filename to prompt the user to
  *     save the file as when the signed url is accessed. This is ignored if
- *     options.responseDisposition is set.
- * @param {string=} options.responseDisposition - The
+ *     `config.responseDisposition` is set.
+ * @param {string=} config.responseDisposition - The
  *     [response-content-disposition parameter](http://goo.gl/yMWxQV) of the
  *     signed url.
- * @param {string=} options.responseType - The response-content-type parameter
+ * @param {string=} config.responseType - The response-content-type parameter
  *     of the signed url.
  * @param {function=} callback - The callback function.
  * @param {?error} callback.err - An error returned while making this request
@@ -1215,26 +1215,26 @@ File.prototype.getSignedPolicy = function(options, callback) {
  *   });
  * });
  */
-File.prototype.getSignedUrl = function(options, callback) {
-  var expires = new Date(options.expires);
+File.prototype.getSignedUrl = function(config, callback) {
+  var expires = new Date(config.expires);
   var expiresInSeconds = Math.round(expires / 1000); // The API expects seconds.
 
   if (expires < Date.now()) {
     throw new Error('An expiration date cannot be in the past.');
   }
 
-  options = extend({}, options);
+  config = extend({}, config);
 
-  options.action = {
+  config.action = {
     read: 'GET',
     write: 'PUT',
     delete: 'DELETE'
-  }[options.action];
+  }[config.action];
 
   var name = encodeURIComponent(this.name);
   var targetGeneration = this.generation;
 
-  options.resource = '/' + this.bucket.name + '/' + name;
+  config.resource = '/' + this.bucket.name + '/' + name;
 
   this.storage.getCredentials(function(err, credentials) {
     if (err) {
@@ -1252,33 +1252,44 @@ File.prototype.getSignedUrl = function(options, callback) {
       return;
     }
 
+    var extensionHeadersString = '';
+
+    if (config.extensionHeaders) {
+      for (var headerName in config.extensionHeaders) {
+        extensionHeadersString += format('{name}:{value}\n', {
+          name: headerName,
+          value: config.extensionHeaders[headerName],
+        });
+      }
+    }
+
     var sign = crypto.createSign('RSA-SHA256');
     sign.update([
-      options.action,
-      (options.contentMd5 || ''),
-      (options.contentType || ''),
+      config.action,
+      (config.contentMd5 || ''),
+      (config.contentType || ''),
       expiresInSeconds,
-      (options.extensionHeaders || '') + options.resource
+      extensionHeadersString + config.resource
     ].join('\n'));
     var signature = sign.sign(credentials.private_key, 'base64');
 
     var responseContentType = '';
-    if (is.string(options.responseType)) {
+    if (is.string(config.responseType)) {
       responseContentType =
         '&response-content-type=' +
-        encodeURIComponent(options.responseType);
+        encodeURIComponent(config.responseType);
     }
 
     var responseContentDisposition = '';
-    if (is.string(options.promptSaveAs)) {
+    if (is.string(config.promptSaveAs)) {
       responseContentDisposition =
         '&response-content-disposition=attachment; filename="' +
-        encodeURIComponent(options.promptSaveAs) + '"';
+        encodeURIComponent(config.promptSaveAs) + '"';
     }
-    if (is.string(options.responseDisposition)) {
+    if (is.string(config.responseDisposition)) {
       responseContentDisposition =
         '&response-content-disposition=' +
-        encodeURIComponent(options.responseDisposition);
+        encodeURIComponent(config.responseDisposition);
     }
 
     var generation = '';
@@ -1287,7 +1298,7 @@ File.prototype.getSignedUrl = function(options, callback) {
     }
 
     callback(null, [
-      'https://storage.googleapis.com' + options.resource,
+      'https://storage.googleapis.com' + config.resource,
       '?GoogleAccessId=' + credentials.client_email,
       '&Expires=' + expiresInSeconds,
       '&Signature=' + encodeURIComponent(signature),
diff --git a/test/storage/file.js b/test/storage/file.js
@@ -28,6 +28,7 @@ var stream = require('stream');
 var through = require('through2');
 var tmp = require('tmp');
 var url = require('url');
+var crypto = require('crypto');
 
 var Bucket = require('../../lib/storage/bucket.js');
 var ServiceObject = require('../../lib/common/service-object.js');
@@ -1913,6 +1914,42 @@ describe('File', function() {
         }, /cannot be in the past/);
       });
     });
+
+    describe('extensionHeaders', function() {
+      it('should add headers to signature', function(done) {
+        var extensionHeaders = {
+          'x-goog-acl': 'public-read',
+          'x-foo': 'bar'
+        };
+
+        var expires = Date.now() + 5;
+        var expiresInSeconds = Math.round(expires / 1000);
+        var name = encodeURIComponent(directoryFile.name);
+        var resource = '/' + directoryFile.bucket.name + '/' + name;
+
+        var sign = crypto.createSign('RSA-SHA256');
+
+        sign.update([
+          'GET',
+          '',
+          '',
+          expiresInSeconds,
+          'x-goog-acl:public-read\nx-foo:bar\n' + resource
+        ].join('\n'));
+
+        var expSignature = sign.sign(credentials.private_key, 'base64');
+
+        directoryFile.getSignedUrl({
+          action: 'read',
+          expires: expires,
+          extensionHeaders: extensionHeaders
+        }, function(err, signedUrl) {
+          assert.ifError(err);
+          assert(signedUrl.indexOf(encodeURIComponent(expSignature)) > -1);
+          done();
+        });
+      });
+    });
   });
 
   describe('makePrivate', function() {
