From 3877b50b189e64432cc3d42b679cc4cef9f6bd65 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Fri, 7 Nov 2014 15:01:34 -0500 Subject: [PATCH 1/3] storage: encode file names --- lib/storage/file.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/storage/file.js b/lib/storage/file.js index 211af929960..8d727ce9a03 100644 --- 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; From 52cb7274c7ed3f4156bc19ef618508d276924665 Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Fri, 7 Nov 2014 15:17:05 -0500 Subject: [PATCH 2/3] add regression test --- regression/storage.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/regression/storage.js b/regression/storage.js index 0cc0f87f3c3..48ca3708fe4 100644 --- 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'); From f38f2452b808cfe19c4d724ebeed76edf2a9aaef Mon Sep 17 00:00:00 2001 From: Stephen Sawchuk Date: Fri, 7 Nov 2014 15:53:51 -0500 Subject: [PATCH 3/3] add unit tests --- test/storage/file.js | 59 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/storage/file.js b/test/storage/file.js index 00aeb81c356..5f5837f1623 100644 --- 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();