From e696a1f80cd84e7d3db9fb85a18212f970f9a0d3 Mon Sep 17 00:00:00 2001
From: Alex Liu <DonutEspresso@users.noreply.github.com>
Date: Thu, 5 Oct 2017 12:02:50 -0700
Subject: [PATCH] Fix: redirect should work even when hostname or protocol is
 not specified in req.url (#1497)

---
 docs/api/response.md  |  8 ++++++++
 lib/response.js       | 11 +++++++----
 test/response.test.js | 28 +++++++++++++++++++++++++---
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/docs/api/response.md b/docs/api/response.md
index ccb15c1c3..b841dbd10 100644
--- a/docs/api/response.md
+++ b/docs/api/response.md
@@ -93,6 +93,14 @@ Sets the link header.
 * `url` {String} a url to redirect to
 * `next` {Function} a callback function
 * `options` {Object} an options object to configure a redirect
+* `[options.secure]` {Boolean} whether to redirect to http or https
+* `[options.hostname]` {String} redirect location's hostname
+* `[options.pathname]` {String} redirect location's pathname
+* `[options.port]` {String} redirect location's port number
+* `[options.query]` {String} redirect location's query string parameters
+* `[options.overrideQuery]` {Boolean} if true, `options.query` stomps over any
+existing query parameters on current URL. by default, will merge the two.
+* `[options.permanent]` {Boolean} if true, sets 301. defaults to 302.
 
 A convenience method for 301/302 redirects. Using this method will
 tell restify to stop execution of your handler chain. You can also
diff --git a/lib/response.js b/lib/response.js
index 30031719a..1e14da36d 100644
--- a/lib/response.js
+++ b/lib/response.js
@@ -599,14 +599,17 @@ Response.prototype.redirect = function redirect(arg1, arg2, arg3) {
         };
 
         // start building url based on options.
-        // first, set protocol.
-        finalUri.protocol = (secure === true) ? 'https' : 'http';
-
-        // then set host
+        // start with the host
         if (opt.hostname) {
             finalUri.hostname = opt.hostname;
         }
 
+        // then set protocol IFF hostname is set - otherwise we end up with
+        // malformed URL.
+        if (finalUri.hostname) {
+            finalUri.protocol = (secure === true) ? 'https' : 'http';
+        }
+
         // then set current path after the host
         if (opt.pathname) {
             finalUri.pathname = opt.pathname;
diff --git a/test/response.test.js b/test/response.test.js
index 1bbf236fb..333544613 100644
--- a/test/response.test.js
+++ b/test/response.test.js
@@ -101,7 +101,7 @@ test('redirect to new relative string url as-is', function (t) {
         res.redirect('/1', next);
     });
 
-    CLIENT.get(join(LOCALHOST, '/20'), function (err, _, res) {
+    CLIENT.get(join(LOCALHOST, '/20?a=1'), function (err, _, res) {
         t.ifError(err);
         t.equal(res.statusCode, 302);
         t.equal(res.headers.location, '/1');
@@ -109,7 +109,6 @@ test('redirect to new relative string url as-is', function (t) {
     });
 });
 
-
 test('redirect to current url (reload)', function (t) {
     SERVER.get('/2', function (req, res, next) {
         res.redirect({
@@ -158,7 +157,6 @@ test('redirect to current url from https -> http', function (t) {
     });
 });
 
-
 test('redirect by changing path', function (t) {
     SERVER.get('/4', function (req, res, next) {
         res.redirect({
@@ -174,6 +172,30 @@ test('redirect by changing path', function (t) {
     });
 });
 
+test('GH-1494: redirect should succeed even if req.url does not specify host' +
+' or protocol', function (t) {
+    SERVER.get('/5', function (req, res, next) {
+        res.redirect({
+            pathname: '/'
+        }, next);
+    });
+
+    // use a relative URL here instead of request with full protocol and host.
+    // this causes node to receive different values for req.url, which affects
+    // how reconstruction of the redirect URL is done. for example including
+    // full host will result in a req.url value of:
+    //         http://127.0.0.1:57824/5
+    // using relative URL results in a req.url value of:
+    //         /5
+    // this causes a bug as documented in GH-1494
+    CLIENT.get('/5', function (err, _, res) {
+        t.ifError(err);
+        t.equal(res.statusCode, 302);
+        t.equal(res.headers.location, '/');
+        t.end();
+    });
+});
+
 test('redirect should add query params', function (t) {
     SERVER.get('/5', function (req, res, next) {
         res.redirect({