Fix an inefficient regex in autoInject (#1767)

* Fix an inefficient regex in autoInject

* 'properly strip comments in argument definitions' test failure

* Update test/autoInject.js

Co-authored-by: Rich Trott <rtrott@gmail.com>

* Update on url-comments lib/autoInject.js

Co-authored-by: Rich Trott <rtrott@gmail.com>

* move new tests test/autoInject.js

* indentation fix test/autoInject.js

Co-authored-by: Rich Trott <rtrott@gmail.com>

2 files changed, 30 insertions, 1 deletions

diff --git a/lib/autoInject.js b/lib/autoInject.js
index cff2eb6..21f95bd 100644
--- a/lib/autoInject.js
+++ b/lib/autoInject.js
@@ -6,7 +6,7 @@ var FN_ARGS = /^(?:async\s+)?(?:function)?\s*\w*\s*\(\s*([^)]+)\s*\)(?:\s*{)/;
 var ARROW_FN_ARGS = /^(?:async\s+)?\(?\s*([^)=]+)\s*\)?(?:\s*=>)/;
 var FN_ARG_SPLIT = /,/;
 var FN_ARG = /(=.+)?(\s*)$/;
-var STRIP_COMMENTS = /((\/\/.*$)|(\/\*[\s\S]*?\*\/))/mg;
+var STRIP_COMMENTS = /(\/\*(?:[^/]|\/(?!\*))*\*\/)|\/\/.*$/mg;
 
 function parseParams(func) {
     const src = func.toString().replace(STRIP_COMMENTS, '');
diff --git a/test/autoInject.js b/test/autoInject.js
index 3088faa..e18767a 100644
--- a/test/autoInject.js
+++ b/test/autoInject.js
@@ -224,4 +224,33 @@ describe('autoInject', () => {
             done()
         })
     })
+
+    it('should not be subject to ReDoS', () => {
+        // This test will timeout if the bug is present.
+        var someComments = 'text/*'.repeat(1000000)
+        expect(() => async.autoInject({
+            someComments,
+            a () {}
+        })).to.throw()
+    });
+
+    it('should properly strip comments in argument definitions', (done) => {
+        async.autoInject({
+            task1: function(task2, /* ) */ callback) {
+                callback(null, true);
+            },
+            task2: function task2(task3 // )
+                ,callback) {
+                callback(null, true);
+            },
+            task3: function task3(callback) {
+                callback(null, true);
+            }
+        },
+        (err, result) => {
+            expect(err).to.eql(null);
+            expect(result).to.deep.eql({task1: true, task2: true, task3: true});
+            done();
+        });
+    });
 });