Refactoring resource canonicalization

Signed-off-by: Tj Holowaychuk <tj@vision-media.ca>

lib/knox/auth.js

@@ -10,6 +10,7 @@
  */
 
 var crypto = require('crypto');
+var url = require('url');
 
 /**
  * Return an "Authorization" header value with the given `options`
@@ -139,3 +140,25 @@ exports.canonicalizeHeaders = function(headers){
   }
   return buf.sort().join('\n');
 };
+
+/**
+ * Perform the following:
+ *
+ *  - ignore non sub-resources
+ *  - sort lexicographically
+ *
+ * @param {String} resource
+ * @return {String}
+ * @api private
+ */
+exports.canonicalizeResource = function(resource){
+  var urlObj = url.parse(resource, true);
+  var buf = urlObj.pathname;
+  var qbuf = [];
+  Object.keys(urlObj.query).forEach(function (qs) {
+    if (['acl', 'location', 'logging', 'notification', 'partNumber', 'policy', 'requestPayment', 'torrent', 'uploadId', 'uploads', 'versionId', 'versioning', 'versions', 'website'].indexOf(qs) != -1) {
+      qbuf.push(qs + (urlObj.query[qs] !== '' ? '=' + encodeURIComponent(urlObj.query[qs]) : ''));
+    }
+  });
+  return buf + (qbuf.length !== 0 ? '?' + qbuf.sort().join('&') : '');
+};

lib/knox/client.js

@@ -13,7 +13,6 @@ var utils = require('./utils')
   , auth = require('./auth')
   , http = require('http')
   , url = require('url')
-  , qstring = require('querystring')
   , join = require('path').join
   , mime = require('./mime')
   , fs = require('fs');
@@ -53,41 +52,7 @@ var Client = module.exports = exports = function Client(options) {
 Client.prototype.request = function(method, filename, headers){
   var options = { host: this.endpoint, port: 80 }
     , date = new Date
-    , headers = headers || {}
-    , parsed = url.parse(filename, true)
-    , query = parsed.query
-    , queryToBeSigned = {}
-    , newQuery
-    , keys;
-
-  // http://docs.amazonwebservices.com/AmazonS3/latest/dev/index.html?RESTAuthentication.html
-  // says we only should sign the following query params.
-  // Also, the params need to be sorted lexicographically
-  if (query) {
-    qKeys = Object.keys(query).sort();
-    qKeys.forEach(function(k, idx){
-      if (k == 'acl'
-        || k == 'location'
-        || k == 'logging'
-        || k == 'notification'
-        || k == 'partNumber'
-        || k == 'policy'
-        || k == 'requestPayment'
-        || k == 'torrent'
-        || k == 'uploadId'
-        || k == 'uploads'
-        || k == 'versionId'
-        || k == 'versioning'
-        || k == 'versions'
-        || k == 'website') {
-        queryToBeSigned[k] = query[k];
-      }
-    });
-
-    if (Object.keys(queryToBeSigned).length > 0) {
-      newQuery = '?' + qstring.stringify(queryToBeSigned);
-    }
-  }
+    , headers = headers || {};
 
   // Default headers
   utils.merge(headers, {
@@ -101,11 +66,7 @@ Client.prototype.request = function(method, filename, headers){
     , secret: this.secret
     , verb: method
     , date: date
-    // If filename === '?prefix=test' (Note the missing / at start),
-    // options.path will be equal to '/?prefix=test'
-    // but resource = '' (query param prefix does not need to be signed)
-    // This will cause a 403. Hence adding the 2nd '/'
-    , resource: join('/', this.bucket, parsed.pathname, '/', newQuery)
+    , resource: auth.canonicalizeResource(join('/', this.bucket, filename))
     , contentType: headers['Content-Type']
     , amazonHeaders: auth.canonicalizeHeaders(headers)
   });