Escape question marks in filenames

Author: hurrymaplelad

lib/client.js

@@ -64,7 +64,7 @@ function removeLeadingSlash(filename) {
 function encodeSpecialCharacters(filename) {
   // Note: these characters are valid in URIs, but S3 does not like them for
   // some reason.
-  return encodeURI(filename).replace(/[!'()#*+ ]/g, function (char) {
+  return encodeURI(filename).replace(/[!'()#*+? ]/g, function (char) {
     return '%' + char.charCodeAt(0).toString(16);
   });
 }
@@ -255,7 +255,7 @@ Client.prototype.request = function(method, filename, headers){
   var options = { hostname: this.host, agent: this.agent, port: this.port }
     , date = new Date
     , headers = headers || {}
-    , fixedFilename = encodeSpecialCharacters(ensureLeadingSlash(filename));
+    , fixedFilename = ensureLeadingSlash(filename);
 
   // Default headers
   headers.Date = date.toUTCString()
@@ -323,7 +323,7 @@ Client.prototype.request = function(method, filename, headers){
 
 Client.prototype.put = function(filename, headers){
   headers = utils.merge({}, headers || {});
-  return this.request('PUT', filename, headers);
+  return this.request('PUT', encodeSpecialCharacters(filename), headers);
 };
 
 /**
@@ -548,7 +548,7 @@ Client.prototype.copyFileTo = function(sourceFilename, destBucket, destFilename,
  */
 
 Client.prototype.get = function(filename, headers){
-  return this.request('GET', filename, headers);
+  return this.request('GET', encodeSpecialCharacters(filename), headers);
 };
 
 /**
@@ -583,7 +583,7 @@ Client.prototype.getFile = function(filename, headers, fn){
  */
 
 Client.prototype.head = function(filename, headers){
-  return this.request('HEAD', filename, headers);
+  return this.request('HEAD', encodeSpecialCharacters(filename), headers);
 };
 
 /**
@@ -618,7 +618,7 @@ Client.prototype.headFile = function(filename, headers, fn){
  */
 
 Client.prototype.del = function(filename, headers){
-  return this.request('DELETE', filename, headers);
+  return this.request('DELETE', encodeSpecialCharacters(filename), headers);
 };
 
 /**
@@ -781,11 +781,9 @@ Client.prototype.list = function(params, headers, fn){
     params = null;
   }
 
-  // Stringify the list params but don't encode them yet, since ::request
-  // already encodes the value, leading to double-encoding issues
-  var url = params ? '?' + decodeURIComponent(qs.stringify(params)) : '';
-
-  return this.getFile(url, headers, function(err, res){
+  var url = params ? '?' + qs.stringify(params) : '';
+  var req = this.request('GET', url, headers);
+  registerReqListeners(req, function(err, res){
     if (err) return fn(err);
 
     var xmlStr = '';
@@ -809,8 +807,11 @@ Client.prototype.list = function(params, headers, fn){
 
           fn(null, data);
         });
-    }).on('error', fn);
+    });
   });
+  req.on('error', fn);
+  req.end();
+  return req;
 };
 
 /**

test/knox.test.js

@@ -317,6 +317,17 @@ function runTestsForStyle(style, userFriendlyName) {
           done();
         });
       });
+
+      specify('with ? in the file name', function (done) {
+        var buffer = new Buffer('a string of stuff');
+        var headers = { 'Content-Type': 'text/plain' };
+
+        client.putBuffer(buffer, '/buffer?with?questions.txt', headers, function (err, res) {
+          assert.ifError(err);
+          assert.equal(res.statusCode, 200);
+          done();
+        });
+      });
     });
 
     describe('copy()', function () {
@@ -593,7 +604,8 @@ function runTestsForStyle(style, userFriendlyName) {
       it('should remove the files as seen in list()', function (done) {
         // Intentionally mix no leading slashes or leading slashes: see #121.
         var files = ['/test/user3.json', 'test/string.txt', '/test/apos\'trophe.txt', '/buffer.txt', '/buffer2.txt',
-                     'google', 'buffer with spaces.txt', 'buffer+with+pluses.txt', '/ø', 'ø/ø', '/test/versioned.txt#1'];
+                     'google', 'buffer with spaces.txt', 'buffer+with+pluses.txt', 'buffer?with?questions.txt',
+                      '/ø', 'ø/ø', '/test/versioned.txt#1'];
         client.deleteMultiple(files, function (err, res) {
           assert.ifError(err);
           assert.equal(res.statusCode, 200);
@@ -610,6 +622,7 @@ function runTestsForStyle(style, userFriendlyName) {
             assert(keys.indexOf('google') === -1);
             assert(keys.indexOf('buffer with spaces.txt') === -1);
             assert(keys.indexOf('buffer+with+pluses.txt') === -1);
+            assert(keys.indexOf('buffer?with?questions.txt') === -1);
             assert(keys.indexOf('ø') === -1);
             assert(keys.indexOf('ø/ø') === -1);
             assert(keys.indexOf('/test/versioned.txt#1') === -1);