Merge pull request #293 from stephenplusplus/spp--storage-slashes

storage: encode file names
googleapis · Nov 7, 2014 · b9767ad · b9767ad
2 parents 7c851ba + f38f245
commit b9767ad
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 8 deletions.
diff --git a/lib/storage/file.js b/lib/storage/file.js
@@ -159,9 +159,9 @@ File.prototype.copy = function(destination, callback) {
     throw noDestinationError;
   }
   var path = util.format('/o/{srcName}/copyTo/b/{destBucket}/o/{destName}', {
-    srcName: this.name,
+    srcName: encodeURIComponent(this.name),
     destBucket: destBucket.name,
-    destName: destName
+    destName: encodeURIComponent(destName)
   });
   this.makeReq_('POST', path, null, {}, function(err) {
     if (err) {
@@ -272,7 +272,7 @@ File.prototype.createWriteStream = function(metadata) {
       metadata: metadata,
       request: {
         qs: {
-          name: that.name,
+          name: that.name
         },
         uri: util.format('{base}/{bucket}/o', {
           base: STORAGE_UPLOAD_BASE_URL,
@@ -300,7 +300,8 @@ File.prototype.createWriteStream = function(metadata) {
  */
 File.prototype.delete = function(callback) {
   callback = callback || util.noop;
-  this.makeReq_('DELETE', '/o/' + this.name, null, true, function(err) {
+  var path = '/o/' + encodeURIComponent(this.name);
+  this.makeReq_('DELETE', path, null, true, function(err) {
     if (err) {
       callback(err);
       return;
@@ -319,7 +320,8 @@ File.prototype.delete = function(callback) {
  */
 File.prototype.getMetadata = function(callback) {
   callback = callback || util.noop;
-  this.makeReq_('GET', '/o/' + this.name, null, true, function(err, resp) {
+  var path = '/o/' + encodeURIComponent(this.name);
+  this.makeReq_('GET', path, null, true, function(err, resp) {
     if (err) {
       callback(err);
       return;
@@ -365,7 +367,9 @@ File.prototype.getSignedUrl = function(options, callback) {
     delete: 'DELETE'
   }[options.action];
 
-  options.resource = '/' + this.bucket.name + '/' + this.name;
+  var name = encodeURIComponent(this.name);
+
+  options.resource = '/' + this.bucket.name + '/' + name;
 
   var makeAuthorizedRequest_ = this.bucket.storage.makeAuthorizedRequest_;
 
@@ -411,8 +415,8 @@ File.prototype.getSignedUrl = function(options, callback) {
  */
 File.prototype.setMetadata = function(metadata, callback) {
   callback = callback || util.noop;
-  this.makeReq_(
-      'PATCH', '/o/' + this.name, null, metadata, function(err, resp) {
+  var path = '/o/' + encodeURIComponent(this.name);
+  this.makeReq_('PATCH', path, null, metadata, function(err, resp) {
     if (err) {
       callback(err);
       return;

diff --git a/regression/storage.js b/regression/storage.js
@@ -142,6 +142,25 @@ describe('storage', function() {
       });
     });
 
+    it('should read/write from/to a file in a directory', function(done) {
+      var file = bucket.file('directory/file');
+      var contents = 'test';
+
+      var writeStream = file.createWriteStream();
+      writeStream.write(contents);
+      writeStream.end();
+
+      writeStream.on('error', done);
+      writeStream.on('complete', function() {
+        file.createReadStream()
+          .on('data', function(chunk) {
+            assert.equal(String(chunk), contents);
+          })
+          .on('error', done)
+          .on('end', done);
+      });
+    });
+
     describe('stream write', function() {
       it('should stream write, then remove large file (5mb)', function(done) {
         var file = bucket.file('LargeFile');

diff --git a/test/storage/file.js b/test/storage/file.js
@@ -80,10 +80,14 @@ describe('File', function() {
   };
   var bucket = new Bucket(options, 'bucket-name');
   var file;
+  var directoryFile;
 
   beforeEach(function() {
     file = new File(bucket, FILE_NAME);
     file.makeReq_ = util.noop;
+
+    directoryFile = new File(bucket, 'directory/file.jpg');
+    directoryFile.makeReq_ = util.noop;
   });
 
   describe('initialization', function() {
@@ -111,6 +115,24 @@ describe('File', function() {
       }, /should have a name/);
     });
 
+    it('should URI encode file names', function(done) {
+      var newFile = new File(bucket, 'nested/file.jpg');
+
+      var expectedPath =
+        util.format('/o/{srcName}/copyTo/b/{destBucket}/o/{destName}', {
+          srcName: encodeURIComponent(directoryFile.name),
+          destBucket: file.bucket.name,
+          destName: encodeURIComponent(newFile.name)
+        });
+
+      directoryFile.makeReq_ = function(method, path) {
+        assert.equal(path, expectedPath);
+        done();
+      };
+
+      directoryFile.copy(newFile);
+    });
+
     describe('destination types', function() {
       function assertPathEquals(file, expectedPath, callback) {
         file.makeReq_ = function(method, path) {
@@ -335,6 +357,15 @@ describe('File', function() {
       file.delete();
     });
 
+    it('should URI encode file names', function(done) {
+      directoryFile.makeReq_ = function(method, path) {
+        assert.equal(path, '/o/' + encodeURIComponent(directoryFile.name));
+        done();
+      };
+
+      directoryFile.delete();
+    });
+
     it('should execute callback', function(done) {
       file.makeReq_ = function(method, path, query, body, callback) {
         callback();
@@ -357,6 +388,15 @@ describe('File', function() {
       file.getMetadata();
     });
 
+    it('should URI encode file names', function(done) {
+      directoryFile.makeReq_ = function(method, path) {
+        assert.equal(path, '/o/' + encodeURIComponent(directoryFile.name));
+        done();
+      };
+
+      directoryFile.getMetadata();
+    });
+
     it('should execute callback', function(done) {
       file.makeReq_ = function(method, path, query, body, callback) {
         callback();
@@ -407,6 +447,16 @@ describe('File', function() {
       });
     });
 
+    it('should URI encode file names', function(done) {
+      directoryFile.getSignedUrl({
+        action: 'read',
+        expires: Math.round(Date.now() / 1000) + 5
+      }, function(err, signedUrl) {
+        assert(signedUrl.indexOf(encodeURIComponent(directoryFile.name)) > -1);
+        done();
+      });
+    });
+
     describe('expires', function() {
       var nowInSeconds = Math.floor(Date.now() / 1000);
 
@@ -448,6 +498,15 @@ describe('File', function() {
       file.setMetadata(metadata);
     });
 
+    it('should URI encode file names', function(done) {
+      directoryFile.makeReq_ = function(method, path) {
+        assert.equal(path, '/o/' + encodeURIComponent(directoryFile.name));
+        done();
+      };
+
+      directoryFile.setMetadata();
+    });
+
     it('should execute callback', function(done) {
       file.makeReq_ = function(method, path, query, body, callback) {
         callback();