Fix: redirect should work even when hostname or protocol is not speci…

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

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;

test/response.test.js

@@ -101,15 +101,14 @@ 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');
         t.end();
     });
 });
 
-
 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({