working draft [ci skip]

lib/auth/v2/getCanonicalizedAmzHeaders.js

@@ -6,9 +6,15 @@ function getCanonicalizedAmzHeaders(headers) {
     Need to include 'x-amz-date' here even though AWS docs
     ambiguous on this.
     */
-    const amzHeaders = Object.keys(headers)
+    const amzHeaders = [];
-        .filter(val => val.substr(0, 6) === 'x-amz-')
+    const keys = Object.keys(headers);
-        .map(val => [val.trim(), headers[val].trim()]);
+    const len = keys.length;
+    for (let i = 0; i < len; ++i) {
+        const key = keys[i];
+        if (key.startsWith('x-amz-')) {
+            amzHeaders.push([key.trim(), headers[key].trim()]);
+        }
+    }
     /*
     AWS docs state that duplicate headers should be combined
     in the same header with values concatenated with
@@ -27,18 +33,16 @@ function getCanonicalizedAmzHeaders(headers) {
         return '';
     }
 
-
     // Sort the amz headers by key (first item in tuple)
-    amzHeaders.sort((a, b) => {
+    amzHeaders.sort((a, b) => a[0] > b[0] ? 1 : -1);
-        if (a[0] > b[0]) {
-            return 1;
-        }
-        return -1;
-    });
     // Build headerString
-    return amzHeaders.reduce((headerStr, current) =>
+    let headerStr = '';
-        `${headerStr}${current[0]}:${current[1]}\n`,
+    const finLen = amzHeaders.length;
-        '');
+    for (let i = 0; i < finLen; ++i) {
+        const current = amzHeaders[i];
+        headerStr += current[0] + ':' + current[1] + '\n';
+    }
+    return headerStr;
 }
 
 module.exports = getCanonicalizedAmzHeaders;

lib/auth/v2/getCanonicalizedResource.js

@@ -2,19 +2,6 @@
 
 const url = require('url');
 
-function getCanonicalizedResource(request) {
-    /*
-    This variable is used to determine whether to insert
-    a '?' or '&'.  Once a query parameter is added to the resourceString,
-    it changes to '&' before any new query parameter is added.
-    */
-    let queryChar = '?';
-    // If bucket specified in hostname, add to resourceString
-    let resourceString = request.gotBucketNameFromHost ?
-        `/${request.bucketName}` : '';
-    // Add the path to the resourceString
-    resourceString += url.parse(request.url).pathname;
-
 /*
 If request includes a specified subresource,
 add to the resourceString: (a) a '?', (b) the subresource,
@@ -42,6 +29,25 @@ function getCanonicalizedResource(request) {
     'website',
 ];
 
+// Specified subresources:
+const subresourcesMap = {
+    'acl': true,
+    'lifecycle': true,
+    'location': true,
+    'logging': true,
+    'notification': true,
+    'partNumber': true,
+    'policy': true,
+    'requestPayment': true,
+    'torrent': true,
+    'uploadId': true,
+    'uploads': true,
+    'versionId': true,
+    'versioning': true,
+    'versions': true,
+    'website': true,
+};
+
 /*
 If the request includes parameters in the query string,
 that override the headers, include
@@ -58,29 +64,88 @@ function getCanonicalizedResource(request) {
     'response-expires',
 ];
 
+function makeValue(query, key) {
+    const val = query[key];
+    if (val !== '') {
+        return key + '=' + val;
+    }
+    return key;
+}
+
+function getCanonicalizedResource(request) {
+    /*
+    This variable is used to determine whether to insert
+    a '?' or '&'.  Once a query parameter is added to the resourceString,
+    it changes to '&' before any new query parameter is added.
+    */
+    let queryChar = '?';
+    // If bucket specified in hostname, add to resourceString
+    let resourceString = request.gotBucketNameFromHost ?
+        `/${request.bucketName}` : '';
+    // Add the path to the resourceString
+    const url = request.url;
+    let index = url.indexOf('?');
+    let pathname = url;
+    if (index === -1) {
+        index = url.indexOf('#');
+    }
+    if (index !== -1) {
+        pathname = url.substring(0, index);
+    }
+
+    resourceString += pathname; 
+
     // Check which specified subresources are present in query string,
     // build array with them
     const query = request.query;
-    const presentSubresources = Object.keys(query).filter(val =>
+    const presentSubresources = [];
-        subresources.indexOf(val) !== -1);
+    const queryKeys = Object.keys(query);
+    const queryKeysLen = queryKeys.length;
+    for (let i = 0; i < queryKeysLen; ++i) {
+        const key = queryKeys[i];
+        if (subresourcesMap[key]) {
             // Sort the array and add the subresources and their value (if any)
             // to the resourceString
-    presentSubresources.sort();
+            const subResourcesLen = presentSubresources.length;
-    resourceString = presentSubresources.reduce((prev, current) => {
+            if (subResourcesLen === 0) {
-        const ch = (query[current] !== '' ? '=' : '');
+                presentSubresources.push(makeValue(query, key));
-        const ret = `${prev}${queryChar}${current}${ch}${query[current]}`;
+                continue;
-        queryChar = '&';
+            }
-        return ret;
+            for (let j = 0; j < subResourcesLen; ++j) {
-    }, resourceString);
+                if (key < presentSubresources[j]) {
-    // Add the overriding parameters to our resourceString
+                    presentSubresources.splice(j, 0, makeValue(query, key));
-    resourceString = overridingParams.reduce((prev, current) => {
+                    break;
-        if (query[current]) {
+                }
-            const ret = `${prev}${queryChar}${current}=${query[current]}`;
+                if (j === subResourcesLen - 1) {
-            queryChar = '&';
+                    presentSubresources.push(makeValue(query, key));
-            return ret;
+                }
+            }
+        }
+    }
+    // const presentSubresources = Object.keys(query).filter(val =>
+    //     subresources.indexOf(val) !== -1);
+    // presentSubresources.sort();
+    const subResourcesLen = presentSubresources.length;
+    if (subResourcesLen > 0) {
+        queryChar = '&';
+        resourceString += '?' + presentSubresources.join('&');
+    }
+    // resourceString = presentSubresources.reduce((prev, current) => {
+    //     const ch = (query[current] !== '' ? '=' : '');
+    //     const ret = `${prev}${queryChar}${current}${ch}${query[current]}`;
+    //     queryChar = '&';
+    //     return ret;
+    // }, resourceString);
+    // Add the overriding parameters to our resourceString
+    const overridingParamsLen = overridingParams.length;
+    for (let i = 0; i < overridingParamsLen; ++i) {
+        const current = overridingParams[i];
+        const value = query[current];
+        if (value) {
+            resourceString += `${queryChar}${current}=${value}`;
+            queryChar = '&';
+        }
     }
-        return prev;
-    }, resourceString);
 
     /*
     Per AWS, the delete query string parameter must be included when

lib/policyEvaluator/evaluator.js

@@ -20,20 +20,19 @@ const evaluators = {};
  */
 function isResourceApplicable(requestContext, statementResource, log) {
     const resource = requestContext.getResource();
-    if (!Array.isArray(statementResource)) {
+    const arrStatementRessource = Array.isArray(statementResource) ?
-        // eslint-disable-next-line no-param-reassign
+        statementResource : [statementResource];
-        statementResource = [statementResource];
+    const arrLen = arrStatementRessource.length;
-    }
     // ARN format:
     // arn:partition:service:region:namespace:relative-id
     const requestResourceArr = resource.split(':');
     // Pull just the relative id because there is no restriction that it
     // does not contain ":"
     const requestRelativeId = requestResourceArr.slice(5).join(':');
-    for (let i = 0; i < statementResource.length; i ++) {
+    for (let i = 0; i < arrLen; i++) {
         // Handle variables (must handle BEFORE wildcards)
         const policyResource =
-            substituteVariables(statementResource[i], requestContext);
+            substituteVariables(arrStatementRessource[i], requestContext);
         // Handle wildcards
         const arnSegmentsMatch =
             checkArnMatch(policyResource, requestRelativeId,
@@ -43,7 +42,6 @@ function isResourceApplicable(requestContext, statementResource, log) {
             { requestResource: resource, policyResource });
             return true;
         }
-        continue;
     }
     log.trace('no policy resource is applicable to request',
         { requestResource: resource });
@@ -60,19 +58,17 @@ function isResourceApplicable(requestContext, statementResource, log) {
  * @return {boolean} true if applicable, false if not
  */
 function isActionApplicable(requestAction, statementAction, log) {
-    if (!Array.isArray(statementAction)) {
+    const arrStatementAction = Array.isArray(statementAction) ?
-        // eslint-disable-next-line no-param-reassign
+        statementAction : [statementAction];
-        statementAction = [statementAction];
+    const length = arrStatementAction.length;
-    }
-    const length = statementAction.length;
     for (let i = 0; i < length; i ++) {
         // No variables in actions so no need to handle
         const regExStrOfStatementAction =
-            handleWildcards(statementAction[i]);
+            handleWildcards(arrStatementAction[i]);
         const actualRegEx = new RegExp(regExStrOfStatementAction);
         if (actualRegEx.test(requestAction)) {
             log.trace('policy action is applicable to request action', {
-                requestAction, policyAction: statementAction[i],
+                requestAction, policyAction: arrStatementAction[i],
             });
             return true;
         }

lib/policyEvaluator/utils/checkArnMatch.js

@@ -32,6 +32,9 @@ function checkArnMatch(policyArn, requestRelativeId, requestArnArr,
     // Check the other parts of the ARN to make sure they match.  If not,
     // return false.
     for (let j = 0; j < 5; j ++) {
+        // if (regExofArn === '^.*?$') {
+        //     continue;
+        // }
         const segmentRegEx = new RegExp(regExofArn[j]);
         const requestSegment = caseSensitive ? requestArnArr[j] :
             requestArnArr[j].toLowerCase();

lib/policyEvaluator/utils/wildcards.js

@@ -9,38 +9,86 @@ const wildcards = {};
 // TODO: Note that there are special rules for * in Principal.
 // Handle when working with bucket policies.
 
+const mustConvertForRegex = {
+    '\\': '\\\\',
+    '*': '.*?',
+    '?': '.',
+    '$': '\\$',
+    '^': '\\^',
+    '+': '\\+',
+    '.': '\\.',
+    '(': '\\(',
+    ')': '\\)',
+    '|': '\\|',
+    '[': '\\[',
+    ']': '\\]',
+    '{': '\\{',
+    '}': '\\}',
+};
 
-/**
+wildcards.handleWildcards = str => {
- * Converts string into a string that has all regEx characters escaped except
+    const end = str.length;
- * for those needed to check for AWS wildcards.  Converted string can then
+    let res = '';
- * be used for a regEx comparison.
+    let lastIndex = 0;
- * @param {string} string - any input string
+    let i = 0;
- * @return {string} converted string
+    for (; i < end; ++i) {
- */
+        const c = str[i];
-wildcards.handleWildcards = string => {
+        const c2 = str[i + 2];
-    // Replace all '*' with '.*' (allow any combo of letters)
+        if (c === '$' && str[i + 1] === '{' && str[i + 3] === '}'
-    // and all '?' with '.{1}' (allow for any one character)
+            && (c2 === '*' || c2 === '?' || c2 === '$')) {
-    // If *, ? or $ are enclosed in ${}, keep literal *, ?, or $
+            res += (i > lastIndex) ?
-    function characterMap(char) {
+                str.substring(lastIndex, i) + '\\' + c2 : // eslint-disable-line
-        const map = {
+                '\\' + c2; // eslint-disable-line
-            '\\*': '.*?',
+            i += 3;
-            '\\?': '.{1}',
+            lastIndex = i + 1;
-            '\\$\\{\\*\\}': '\\*',
+            continue;
-            '\\$\\{\\?\\}': '\\?',
-            '\\$\\{\\$\\}': '\\$',
-        };
-        return map[char];
         }
-    // Escape all regExp special characters
+        const toSecure = mustConvertForRegex[c];
-    let regExStr = string.replace(/[\\^$*+?.()|[\]{}]/g, '\\$&');
+        if (toSecure) {
-    // Replace the AWS special characters with regExp equivalents
+            res += (i > lastIndex) ? str.substring(lastIndex, i) + toSecure :
-    regExStr = regExStr.replace(
+                toSecure;
-        // eslint-disable-next-line max-len
+            lastIndex = i + 1;
-        /(\\\*)|(\\\?)|(\\\$\\\{\\\*\\\})|(\\\$\\\{\\\?\\\})|(\\\$\\\{\\\$\\\})/g,
+            continue;
-        characterMap);
+        }
-    return `^${regExStr}$`;
+    }
+    if (i > lastIndex) {
+        res += str.substring(lastIndex, i);
+    }
+    // eslint-disable-next-line prefer-template
+    return '^' + res + '$';
 };
 
+// /**
+//  * Converts string into a string that has all regEx characters escaped except
+//  * for those needed to check for AWS wildcards.  Converted string can then
+//  * be used for a regEx comparison.
+//  * @param {string} string - any input string
+//  * @return {string} converted string
+//  */
+// wildcards.handleWildcards = string => {
+//     // Replace all '*' with '.*' (allow any combo of letters)
+//     // and all '?' with '.{1}' (allow for any one character)
+//     // If *, ? or $ are enclosed in ${}, keep literal *, ?, or $
+//     function characterMap(char) {
+//         const map = {
+//             '\\*': '.*?',
+//             '\\?': '.{1}',
+//             '\\$\\{\\*\\}': '\\*',
+//             '\\$\\{\\?\\}': '\\?',
+//             '\\$\\{\\$\\}': '\\$',
+//         };
+//         return map[char];
+//     }
+//     // Escape all regExp special characters
+//     let regExStr = string.replace(/[\\^$*+?.()|[\]{}]/g, '\\$&');
+//     // Replace the AWS special characters with regExp equivalents
+//     regExStr = regExStr.replace(
+//         // eslint-disable-next-line max-len
+//    /(\\\*)|(\\\?)|(\\\$\\\{\\\*\\\})|(\\\$\\\{\\\?\\\})|(\\\$\\\{\\\$\\\})/g,
+//         characterMap);
+//     return `^${regExStr}$`;
+// };
+
 /**
  * Converts each portion of an ARN into a converted regEx string
  * to compare against each portion of the ARN from the request

package.json

@@ -1,6 +1,6 @@
 {
   "name": "arsenal",
-  "version": "1.1.0",
+  "version": "1.1.0-perf",
   "description": "Common utilities for the S3 project components",
   "main": "index.js",
   "repository": {
@@ -26,7 +26,7 @@
     "lolex": "^1.4.0",
     "mocha": "^2.3.3",
     "temp": "^0.8.3",
-    "werelogs": "scality/werelogs"
+    "werelogs": "scality/werelogs#ft/performances"
   },
   "scripts": {
     "lint": "eslint $(git ls-files '*.js')",

tests/unit/policyEvaluator.js

@@ -977,7 +977,7 @@ describe('handleWildcards', () => {
         'unlimited of any character and  will match ? as any single ' +
         'character', () => {
         const result = handleWildcards('lsdkfj?lk*');
-        assert.deepStrictEqual(result, '^lsdkfj.{1}lk.*?$');
+        assert.deepStrictEqual(result, '^lsdkfj.lk.*?$');
     });
 
     it('should convert a string to a regEx string that matches ${*} as ' +
@@ -990,7 +990,7 @@ describe('handleWildcards', () => {
     it('should escape other regular expression special characters', () => {
         const result = handleWildcards('*^.+?()|[\]{}');
         assert.deepStrictEqual(result,
-            '^.*?\\^\\.\\+.{1}\\(\\)\\|\\[\\\]\\{\\}$');
+            '^.*?\\^\\.\\+.\\(\\)\\|\\[\\\]\\{\\}$');
     });
 });