[fix] Trap errors from `decodeURI`. Fixes #127

flatiron · Jul 26, 2012 · 2069a8b7de3db6b0a6f91218863c833803e331a4 · 2069a8b
1 parent c583a86
commit 2069a8b7de3db6b0a6f91218863c833803e331a4
Unified Split

Showing with 24 additions and 5 deletions.

+10 −4 lib/director/http/index.js

+12 −0 test/server/helpers/macros.js

+2 −1 test/server/http/http-test.js
diff --git a/lib/director/http/index.js b/lib/director/http/index.js
@@ -108,24 +108,30 @@ Router.prototype.dispatch = function (req, res, callback) {
  // Dispatch `HEAD` requests to `GET`
  //
  var method = req.method === 'HEAD' ? 'get' : req.method.toLowerCase(),
-  
-      url = decodeURI(req.url.split('?', 1)[0]),
      thisArg = { req: req, res: res },
      self = this,
      contentType,
      runlist,
      stream,
      error,
-  
-      fns;
+  
+      fns,
+  
+      url;
-  
-  if (this._hasAccepts) {
+  
+  //
+  
+  // Trap bad URLs from `decodeUri`
+  
+  //
+  
+  try { url = decodeURI(req.url.split('?', 1)[0]) }
+  
+  catch (ex) { url = null }
+  
+
+  
+  if (url && this._hasAccepts) {
    contentType = req.headers['content-type'];
    fns = this.traverse(method, url, this.routes, '', function (route) {
      return !route.accept || route.accept.some(function (a) {
        return a.test(contentType);
      });
    });
  }
-  
-  else {
+  
+  else if (url) {
    fns = this.traverse(method, url, this.routes, '');
  }

diff --git a/test/server/helpers/macros.js b/test/server/helpers/macros.js
@@ -25,6 +25,18 @@ exports.assertGet = function(port, uri, expected) {
  return context;
 };
+  
+exports.assert404 = function (port, uri) {
+  
+  return {
+  
+    topic: function () {
+  
+      request({ uri: 'http://localhost:' + port + '/' + uri }, this.callback);
+  
+    },
+  
+    "should respond with 404": function (err, res, body) {
+  
+      assert.isNull(err);
+  
+      assert.equal(res.statusCode, 404);
+  
+    }
+  
+  };
+  
+};
+  
+
 exports.assertPost = function(port, uri, expected) {
  return {
    topic: function () {

diff --git a/test/server/http/http-test.js b/test/server/http/http-test.js
@@ -49,7 +49,8 @@ vows.describe('director/http').addBatch({
        "a request to foo/bar/bark": assertBark('foo/bar/bark'),
        "a request to foo/update/bark": assertBark('foo/update/bark'),
        "a request to bar/bazz/bark": assertBark('bar/bazz/bark'),
-  
-        "a request to foo/bar/bark?test=test": assertBark('foo/bar/bark?test=test')
+  
+        "a request to foo/bar/bark?test=test": assertBark('foo/bar/bark?test=test'),
+  
+        "a request to foo/%RT": macros.assert404(9090, 'foo/%RT')
      }
    }
  }