Skip to content

Commit

Permalink
fix: improve performance
Browse files Browse the repository at this point in the history
Replacing the regular expressions with algorithmic equivalents
remediates performance issues.

There are no doubt further performance enhancements that can be made
to the code here, but this addresses the most critical. On my
container, the performance tests added in this commit took less than 1
second to run with the code changes here compared to around 40 seconds
using the current module code.

(I also happen to think the algorithms are easier to understand,
maintain, and debug in this form as well.)
  • Loading branch information
Trott committed Sep 26, 2021
1 parent 1eda787 commit af21f96
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 26 deletions.
140 changes: 115 additions & 25 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,116 @@

var isExtglob = require('is-extglob');
var chars = { '{': '}', '(': ')', '[': ']'};
var strictRegex = /\\(.)|(^!|\*|[\].+)]\?|\[[^\\\]]+\]|\{[^\\}]+\}|\(\?[:!=][^\\)]+\)|\([^|]+\|[^\\)]+\))/;
var relaxedRegex = /\\(.)|(^!|[*?{}()[\]]|\(\?)/;
var strictCheck = function (str) {
if (str[0] === '!') {
return true;
}
var index = 0;
while (index < str.length) {
if (str[index] === '*') {
return true;
}

if (str[index + 1] === '?' && /[\].+)]/.test(str[index])) {
return true;
}

if (str[index] === '[' && str[index + 1] !== ']') {
var closeIndex = str.indexOf(']', index);
if (closeIndex > index) {
var slashIndex = str.indexOf('\\', index);
if (slashIndex === -1 || slashIndex > closeIndex) {
return true;
}
}
}

if (str[index] === '{' && str[index + 1] !== '}') {
closeIndex = str.indexOf('}', index);
if (closeIndex > index) {
slashIndex = str.indexOf('\\', index);
if (slashIndex === -1 || slashIndex > closeIndex) {
return true;
}
}
}

if (str[index] === '(' && str[index + 1] === '?' && /[:!=]/.test(str[index + 2]) && str[index + 3] !== ')') {
closeIndex = str.indexOf(')', index);
if (closeIndex > index) {
slashIndex = str.indexOf('\\', index);
if (slashIndex === -1 || slashIndex > closeIndex) {
return true;
}
}
}

if (str[index] === '(' && str[index + 1] !== '|') {
var pipeIndex = str.indexOf('|', index);
if (pipeIndex > index && str[pipeIndex + 1] !== ')') {
closeIndex = str.indexOf(')', pipeIndex);
if (closeIndex > pipeIndex) {
slashIndex = str.indexOf('\\', pipeIndex);
if (slashIndex === -1 || slashIndex > closeIndex) {
return true;
}
}
}
}

if (str[index] === '\\') {
var open = str[index+1];
index += 2;
var close = chars[open];

if (close) {
var n = str.indexOf(close, index);
if (n !== -1) {
index = n + 1;
}
}

if (str[index] === '!') {
return true;
}
} else {
index++;
}
}
return false;
}

var relaxedCheck = function (str) {
if (str[0] === '!') {
return true;
}
var index = 0;
while (index < str.length) {
if (/[*?{}()[\]]/.test(str[index])) {
return true;
}

if (str[index] === '\\') {
var open = str[index+1];
index += 2;
var close = chars[open];

if (close) {
var n = str.indexOf(close, index);
if (n !== -1) {
index = n + 1;
}
}

if (str[index] === '!') {
return true;
}
} else {
index++;
}
}
return false;
}

module.exports = function isGlob(str, options) {
if (typeof str !== 'string' || str === '') {
Expand All @@ -19,30 +127,12 @@ module.exports = function isGlob(str, options) {
return true;
}

var regex = strictRegex;
var match;
var check = strictCheck;

// optionally relax regex
// optionally relax check
if (options && options.strict === false) {
regex = relaxedRegex;
check = relaxedCheck;
}

while ((match = regex.exec(str))) {
if (match[2]) return true;
var idx = match.index + match[0].length;

// if an open bracket/brace/paren is escaped,
// set the index to the next closing character
var open = match[1];
var close = open ? chars[open] : null;
if (open && close) {
var n = str.indexOf(close, idx);
if (n !== -1) {
idx = n + 1;
}
}

str = str.slice(idx);
}
return false;
};
return check(str);
}
16 changes: 15 additions & 1 deletion test.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,20 @@ describe('isGlob', function() {
assert(isGlob('\\*(abc|xyz)/*(abc|xyz)'));
assert(isGlob('\\+(abc|xyz)/+(abc|xyz)'));
});

it('should be performant and not subject to ReDoS/exponential backtracking', function() {
if (!String.prototype.repeat) {
return;
}
// These will time out if the algorithm is inefficient.
isGlob('('.repeat(1e5));
isGlob('('.repeat(1e5), {strict: false});
isGlob('['.repeat(1e5));
isGlob('['.repeat(1e5), {strict: false});
isGlob('{'.repeat(1e5));
isGlob('{'.repeat(1e5), {strict: false});
isGlob('\\'.repeat(1e5));
isGlob('\\'.repeat(1e5), {strict: false});
});
});
});

0 comments on commit af21f96

Please sign in to comment.