Fix infinite loop condition using mergeParams: true

blakeembrey · Aug 3, 2015 · 0ddba82 · 0ddba82
1 parent 0a261dc
commit 0ddba82
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 23 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -1,6 +1,7 @@
 unreleased
 ==========
 
+  * Fix infinite loop condition using `mergeParams: true`
   * Fix inner numeric indices incorrectly altering parent `req.params`
   * deps: array-flatten@1.1.1
     - perf: enable strict mode

diff --git a/index.js b/index.js
@@ -628,10 +628,14 @@ function mergeParams(params, parent) {
   var i = 0
   var o = 0
 
-  // determine numeric gaps
-  while (i === o || o in parent) {
-    if (i in params) i++
-    if (o in parent) o++
+  // determine numeric gap in params
+  while (i in params) {
+    i++
+  }
+
+  // determine numeric gap in parent
+  while (o in parent) {
+    o++
   }
 
   // offset numeric indices in params before merge

diff --git a/test/req.params.js b/test/req.params.js
@@ -89,25 +89,6 @@ describe('req.params', function () {
       .expect(200, '{"foo":"bar"}', done)
     })
 
-    it('should merge numeric properies by offsetting', function (done) {
-      var router = Router({ mergeParams: true })
-      var server = createServer(function (req, res, next) {
-        req.params = {'0': 'foo', '1': 'bar'}
-
-        router(req, res, function (err) {
-          if (err) return next(err)
-          sawParams(req, res)
-        })
-      })
-
-      router.get('/*', hitParams(1))
-
-      request(server)
-      .get('/buzz')
-      .expect('x-params-1', '{"0":"foo","1":"bar","2":"buzz"}')
-      .expect(200, '{"0":"foo","1":"bar"}', done)
-    })
-
     it('should ignore non-object outsite object', function (done) {
       var router = Router({ mergeParams: true })
       var server = createServer(function (req, res, next) {
@@ -145,6 +126,46 @@ describe('req.params', function () {
       .expect('x-params-1', '{"foo":"buzz"}')
       .expect(200, '{"foo":"bar"}', done)
     })
+
+    describe('with numeric properties in req.params', function () {
+      it('should merge numeric properies by offsetting', function (done) {
+        var router = Router({ mergeParams: true })
+        var server = createServer(function (req, res, next) {
+          req.params = {'0': 'foo', '1': 'bar'}
+
+          router(req, res, function (err) {
+            if (err) return next(err)
+            sawParams(req, res)
+          })
+        })
+
+        router.get('/*', hitParams(1))
+
+        request(server)
+        .get('/buzz')
+        .expect('x-params-1', '{"0":"foo","1":"bar","2":"buzz"}')
+        .expect(200, '{"0":"foo","1":"bar"}', done)
+      })
+
+      it('should merge with same numeric properties', function (done) {
+        var router = Router({ mergeParams: true })
+        var server = createServer(function (req, res, next) {
+          req.params = {'0': 'foo'}
+
+          router(req, res, function (err) {
+            if (err) return next(err)
+            sawParams(req, res)
+          })
+        })
+
+        router.get('/*', hitParams(1))
+
+        request(server)
+        .get('/bar')
+        .expect('x-params-1', '{"0":"foo","1":"bar"}')
+        .expect(200, '{"0":"foo"}', done)
+      })
+    })
   })
 })