From 0bd6221dbfd8baa31b9f52e7051a2faa7cf34496 Mon Sep 17 00:00:00 2001 From: John Bacon Date: Sat, 11 Jan 2014 17:55:38 -0500 Subject: [PATCH 1/3] Improved solution to issue #179 (invalid options throwing errors in joinRegExp) This slightly more robust solution ignores options that are not strings or regular expressions in the passed patterns array. The prior solution still accepted non-string, non-regexp options (e.g. [], {}, true, etc.). Credit goes to @soundslocke for the fix. --- src/raven.js | 15 ++++++++------- test/raven.test.js | 6 ++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/raven.js b/src/raven.js index 7a2f79474ddb..397bf35b7ad8 100644 --- a/src/raven.js +++ b/src/raven.js @@ -644,14 +644,15 @@ function joinRegExp(patterns) { // Be mad. var sources = [], i = patterns.length; while (i--) { - if (!isUndefined(patterns[i]) && patterns[i]) { - sources[i] = isString(patterns[i]) ? - // If it's a string, we need to escape it - // Taken from: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions - patterns[i].replace(/([.*+?^=!:${}()|\[\]\/\\])/g, "\\$1") : - // If it's a regexp already, we want to extract the source - patterns[i].source; + if (isString(patterns[i])) { + // If it's a string, we need to escape it + // Taken from: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions + sources.unshift(patterns[i].replace(/([.*+?^=!:${}()|\[\]\/\\])/g, "\\$1")); + } else if (!isUndefined(patterns[i]) && patterns[i] && patterns[i].source) { + // If it's a regexp already, we want to extract the source + sources.unshift(patterns[i].source); } + // Intentionally skip other cases } return new RegExp(sources.join('|'), 'i'); } diff --git a/test/raven.test.js b/test/raven.test.js index 14736ffad69c..f68610e5d091 100644 --- a/test/raven.test.js +++ b/test/raven.test.js @@ -879,6 +879,12 @@ describe('globals', function() { 'a', 'b', null, undefined ]).source, 'a|b'); }); + + it('should skip entries that are not strings or regular expressions in the passed array of patterns', function() { + assert.equal(joinRegExp([ + 'a', 'b', null, 'a.b', undefined, true, /d/, 123, {}, /[0-9]/, [] + ]).source, 'a|b|a\\.b|d|[0-9]'); + }); }); }); From 1d91556070ce4607a9462045cc6332642b7745ec Mon Sep 17 00:00:00 2001 From: John Bacon Date: Sat, 11 Jan 2014 18:49:30 -0500 Subject: [PATCH 2/3] Refactor towards a for loop and .push while removing redundant check ```unshift``` is significantly slower than ```push```. Shift towards using ```push``` and remove a redundant conditional. --- src/raven.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/raven.js b/src/raven.js index 397bf35b7ad8..51a50434e56a 100644 --- a/src/raven.js +++ b/src/raven.js @@ -642,15 +642,15 @@ function isSetup() { function joinRegExp(patterns) { // Combine an array of regular expressions and strings into one large regexp // Be mad. - var sources = [], i = patterns.length; - while (i--) { + var sources = []; + for (var i = 0; i < patterns.length; i++) { if (isString(patterns[i])) { // If it's a string, we need to escape it // Taken from: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions - sources.unshift(patterns[i].replace(/([.*+?^=!:${}()|\[\]\/\\])/g, "\\$1")); - } else if (!isUndefined(patterns[i]) && patterns[i] && patterns[i].source) { + sources.push(patterns[i].replace(/([.*+?^=!:${}()|\[\]\/\\])/g, "\\$1")); + } else if (patterns[i] && patterns[i].source) { // If it's a regexp already, we want to extract the source - sources.unshift(patterns[i].source); + sources.push(patterns[i].source); } // Intentionally skip other cases } From 74b868d217318f9b35d645c80293ed84b3457d3c Mon Sep 17 00:00:00 2001 From: John Bacon Date: Sat, 11 Jan 2014 18:55:55 -0500 Subject: [PATCH 3/3] Cache the value of patterns.length outside the for loop Likely [benefits](http://stackoverflow.com/questions/6261953/do-modern-javascript-jiters-need-array-length-caching-in-loops) the performance of older browsers that lack smart caching. Thanks again to @soundslocke for the heads up. --- src/raven.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/raven.js b/src/raven.js index 51a50434e56a..174ecb3711d5 100644 --- a/src/raven.js +++ b/src/raven.js @@ -642,8 +642,8 @@ function isSetup() { function joinRegExp(patterns) { // Combine an array of regular expressions and strings into one large regexp // Be mad. - var sources = []; - for (var i = 0; i < patterns.length; i++) { + var sources = [], len = patterns.length; + for (var i = 0; i < len; i++) { if (isString(patterns[i])) { // If it's a string, we need to escape it // Taken from: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions