[fix] Trap errors from decodeURI. Fixes #127

flatiron · Jul 26, 2012 · 2069a8b · 2069a8b
1 parent c583a86
commit 2069a8b
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 5 deletions.
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')
       }
     }
   }