Merge branch 'hotfix/0.0.2.1-fix-redirect-protocol'

Author: BryanDonovan

HISTORY.md

@@ -3,3 +3,7 @@
 
 - 0.0.2
   Added responder.redirect().
+
+- 0.0.2.1-fix-redirect-protocol
+  Adding http/https protocol to redirect urls.
+  Redirect url full paths require a leading slash.

lib/server/responder.js

@@ -8,6 +8,18 @@ function is_relative_path(url) {
     return url.indexOf('.') === 0;
 }
 
+function is_https(req) {
+    return req && req.headers['x-forwarded-proto'] === 'https';
+}
+
+function prepend_http_protocol(url, req) {
+    if (is_https(req)) {
+        return 'https://' + url;
+    } else {
+        return 'http://' + url;
+    }
+}
+
 var responder = {
     error: function (res, err, next) {
         next(err);
@@ -82,9 +94,13 @@ var responder = {
             if (is_relative_path(url)) {
                 url = url.replace(/^./, '');
                 url = host + req.path + url;
+            } else if (url.match(/^\//)) {
+                url = host + url; // a bit hacky
             } else {
-                url = host + '/' + url;
+                return responder.error(res, new restify.InternalError('Full-path redirect url requires leading slash (/)'), next);
             }
+
+            url = prepend_http_protocol(url, req);
         }
 
         res.header('Content-Type', content_type);

test/server/responder.functional.js

@@ -21,6 +21,11 @@ var methods = {
     },
 
     redirect_to_full_path: function (req, res, next) {
+        var args = {url: '/foo/bar'};
+        return responder.redirect(req, res, args, next);
+    },
+
+    redirect_to_full_path_without_leading_slash: function (req, res, next) {
         var args = {url: 'foo/bar'};
         return responder.redirect(req, res, args, next);
     },
@@ -56,6 +61,12 @@ var routes = [
         func: methods.redirect_to_full_path,
         middleware: []
     },
+    {
+        method: "get",
+        url: "/test/redirects/full_path_without_leading_slash",
+        func: methods.redirect_to_full_path_without_leading_slash,
+        middleware: []
+    },
     {
         method: "get",
         url: "/test/redirects/relative_path",
@@ -123,19 +134,29 @@ describe("functional - server/responder.js", function () {
         context("when full path passed in without host and port", function () {
             it("redirects to the correct URL using request's host and port", function (done) {
                 http_client.get('/test/redirects/full_path', function (err, result, raw_res) {
-                    var expected_location = http.host + ':' + http.port + '/foo/bar';
+                    var expected_location = 'http://' + http.host + ':' + http.port + '/foo/bar';
                     assert.equal(raw_res.headers.location, expected_location);
                     assert.equal(raw_res.statusCode, 302);
                     done();
                 });
             });
         });
 
+        context("when full path passed in without leading slash", function () {
+            it("returns Internal error", function (done) {
+                http_client.get('/test/redirects/full_path_without_leading_slash', function (err, result, raw_res) {
+                    assert.equal(err.code, 'Internal');
+                    assert.ok(err.message.match(/leading slash/));
+                    done();
+                });
+            });
+        });
+
         context("when relative path passed in", function () {
             it("redirects to the correct URL using request's host and port", function (done) {
                 var starting_path = '/test/redirects/relative_path';
                 http_client.get(starting_path, function (err, result, raw_res) {
-                    var expected_location = http.host + ':' + http.port + starting_path + '/foo/bar';
+                    var expected_location = 'http://' + http.host + ':' + http.port + starting_path + '/foo/bar';
                     assert.equal(raw_res.headers.location, expected_location);
                     assert.equal(raw_res.statusCode, 302);
                     done();

test/server/responder.unit.js

@@ -1,14 +1,20 @@
 var assert = require('assert');
 var sinon = require('sinon');
+var support = require('../support');
 var responder = main.server.responder;
 
 var fake_res = {
     header: function () {},
     status: function () {},
-    json: function () {}
+    json: function () {},
+    send: function () {}
 };
 
-var fake_req = {};
+var fake_req = {
+    headers: {
+        host: 'local.foo.com'
+    }
+};
 
 var fake_next = function () {};
 
@@ -78,7 +84,7 @@ describe("responder.js", function () {
                     next();
                 });
 
-                responder.redirect(fake_res, fake_req, null, fake_next);
+                responder.redirect(fake_req, fake_res, null, fake_next);
 
                 assert.ok(responder.error.called);
 
@@ -94,13 +100,27 @@ describe("responder.js", function () {
                 });
 
                 delete args.url;
-                responder.redirect(fake_res, fake_req, args, fake_next);
+                responder.redirect(fake_req, fake_res, args, fake_next);
 
                 assert.ok(responder.error.called);
 
                 responder.error.restore();
                 done();
             });
         });
+
+        context("when request's protocol is https", function() {
+            sinon.stub(fake_res, 'header');
+
+            var https_req = support.shallow_clone(fake_req);
+            https_req.headers['x-forwarded-proto'] = 'https';
+            var args = {url: '/foo/bar'};
+
+            responder.redirect(https_req, fake_res, args, fake_next);
+
+            assert.ok(fake_res.header.calledWith('Location', "https://local.foo.com/foo/bar"));
+
+            fake_res.header.restore();
+        });
     });
 });