diff --git a/lib/request.js b/lib/request.js index b214e668..aa8750b6 100644 --- a/lib/request.js +++ b/lib/request.js @@ -2,7 +2,6 @@ 'use strict'; -var url = require('url'); var sprintf = require('util').format; var assert = require('assert-plus'); @@ -11,6 +10,7 @@ var Negotiator = require('negotiator'); var uuid = require('uuid'); var dtrace = require('./dtrace'); +var utils = require('./utils'); ///-- Helpers /** @@ -74,7 +74,7 @@ function patch(Request) { var protocol = this.isSecure() ? 'https://' : 'http://'; var hostname = this.headers.host; - return url.resolve(protocol + hostname + this.path() + '/', path); + return new URL(path, protocol + hostname + this.path() + '/').href; }; /** @@ -424,7 +424,7 @@ function patch(Request) { */ Request.prototype.getUrl = function getUrl() { if (this._cacheURL !== this.url) { - this._url = url.parse(this.url); + this._url = utils.parseRequestUrl(this.url); this._cacheURL = this.url; } return this._url; diff --git a/lib/response.js b/lib/response.js index 87aced7a..7f857d18 100644 --- a/lib/response.js +++ b/lib/response.js @@ -4,7 +4,6 @@ var http = require('http'); var sprintf = require('util').format; -var url = require('url'); var assert = require('assert-plus'); var mime = require('mime'); @@ -740,6 +739,7 @@ function patch(Response) { var self = this; var statusCode = 302; var finalUri; + var finalHostname; var redirectLocation; var next; @@ -768,32 +768,32 @@ function patch(Response) { ? opt.secure : req.isSecure(); - // if hostname is passed in, use that as the base, - // otherwise fall back on current url. - var parsedUri = url.parse(opt.hostname || currentFullPath, true); - - // create the object we'll use to format for the final uri. - // this object will eventually get passed to url.format(). - // can't use parsedUri to seed it, as it confuses the url module - // with some existing parsed state. instead, we'll pick the things - // we want and use that as a starting point. - finalUri = { - port: parsedUri.port, - hostname: parsedUri.hostname, - query: parsedUri.query, - pathname: parsedUri.pathname - }; + // if hostname is passed in, use that as the base, otherwise + // fall back on current url. mutate a URL instance directly + // instead of bouncing through a plain-object descriptor. + var raw = opt.hostname || currentFullPath; + var isAbsolute = raw.indexOf('://') !== -1; + finalUri = isAbsolute + ? new URL(raw) + : new URL(raw, 'http://localhost'); - // start building url based on options. - // start with the host if (opt.hostname) { - finalUri.hostname = opt.hostname; + // an explicit hostname replaces the current path entirely, + // so start from a clean slate. + finalUri.pathname = '/'; + finalUri.search = ''; } + // hostname is only known if it was passed in explicitly, or if + // the current url was itself absolute (e.g. proxied requests). + finalHostname = + opt.hostname || (isAbsolute ? finalUri.hostname : null); + // then set protocol IFF hostname is set - otherwise we end up with // malformed URL. - if (finalUri.hostname) { - finalUri.protocol = secure === true ? 'https' : 'http'; + if (finalHostname) { + finalUri.protocol = secure === true ? 'https:' : 'http:'; + finalUri.hostname = finalHostname; } // then set current path after the host @@ -832,7 +832,16 @@ function patch(Response) { return next(new InternalServerError('could not construct url')); } - redirectLocation = url.format(finalUri); + if (typeof finalUri === 'string') { + redirectLocation = finalUri; + } else { + // a URL instance can't represent a host-less location, so build + // that case by hand; otherwise the instance already has + // everything it needs to serialize itself. + redirectLocation = finalHostname + ? finalUri.toString() + : finalUri.pathname + (finalUri.search || ''); + } self.emit('redirect', redirectLocation); diff --git a/lib/utils.js b/lib/utils.js index 77da5389..bcdc00ac 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -54,9 +54,60 @@ function mergeQs(obj1, obj2) { return merged; } +/** + * Parses a raw HTTP request URL string into a legacy url.parse-shaped object. + * Handles the special OPTIONS '*' request-target. + * + * @public + * @function parseRequestUrl + * @param {String} rawUrl - the raw request URL string + * @returns {Object} url descriptor with pathname, search, query, href etc. + */ +function parseRequestUrl(rawUrl) { + var pathname, search, hash, port, hostname; + + if (rawUrl === '*') { + pathname = '*'; + search = null; + hash = null; + port = null; + hostname = null; + } else if (rawUrl.indexOf('://') !== -1) { + var absParsed = new URL(rawUrl); + pathname = absParsed.pathname; + search = absParsed.search || null; + hash = absParsed.hash || null; + port = absParsed.port || null; + hostname = absParsed.hostname || null; + } else { + var relParsed = new URL(rawUrl, 'http://localhost'); + pathname = relParsed.pathname; + search = relParsed.search || null; + hash = relParsed.hash || null; + port = null; + hostname = null; + } + + return { + protocol: null, + slashes: null, + auth: null, + host: null, + port: port, + hostname: hostname, + hash: hash, + search: search, + query: search ? search.slice(1) : null, + pathname: pathname, + path: pathname + (search || ''), + href: rawUrl + }; +} + ///--- Exports module.exports = { shallowCopy: shallowCopy, - mergeQs: mergeQs + mergeQs: mergeQs, + parseRequestUrl: parseRequestUrl }; diff --git a/test/request.test.js b/test/request.test.js index 1bebd715..971125a6 100644 --- a/test/request.test.js +++ b/test/request.test.js @@ -287,3 +287,86 @@ test( }); } ); + +test( + module, + 'getUrl should return correct shape for path with no query', + function(t) { + SERVER.get('/geturl-plain', function(req, res, next) { + var u = req.getUrl(); + t.equal(u.href, '/geturl-plain'); + t.equal(u.pathname, '/geturl-plain'); + t.equal(u.search, null); + t.equal(u.query, null); + t.equal(u.hash, null); + t.equal(u.host, null); + t.equal(u.hostname, null); + t.equal(u.port, null); + t.equal(u.protocol, null); + res.send(); + return next(); + }); + + CLIENT.get('/geturl-plain', function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); + } +); + +test(module, 'getUrl should return correct query string', function(t) { + SERVER.get('/geturl-qs', function(req, res, next) { + var u = req.getUrl(); + t.equal(u.href, '/geturl-qs?a=1&b=2'); + t.equal(u.pathname, '/geturl-qs'); + t.equal(u.search, '?a=1&b=2'); + t.equal(u.query, 'a=1&b=2'); + t.equal(u.hash, null); + t.equal(u.host, null); + t.equal(u.hostname, null); + t.equal(u.port, null); + t.equal(u.protocol, null); + res.send(); + return next(); + }); + + CLIENT.get('/geturl-qs?a=1&b=2', function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test(module, 'getUrl should handle OPTIONS * request-target', function(t) { + SERVER.opts('*', function(req, res, next) { + var u = req.getUrl(); + t.equal(u.pathname, '*'); + t.equal(u.search, null); + t.equal(u.query, null); + res.send(200); + return next(); + }); + + CLIENT.opts('*', function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test(module, 'getUrl result is cached across calls', function(t) { + SERVER.get('/geturl-cache', function(req, res, next) { + var u1 = req.getUrl(); + var u2 = req.getUrl(); + t.strictEqual(u1, u2); + res.send(); + return next(); + }); + + CLIENT.get('/geturl-cache', function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); +}); diff --git a/test/response.test.js b/test/response.test.js index 15c7ce19..be7bdb09 100644 --- a/test/response.test.js +++ b/test/response.test.js @@ -1,7 +1,6 @@ 'use strict'; /* eslint-disable func-names */ -var url = require('url'); var restifyClients = require('restify-clients'); var errs = require('restify-errors'); @@ -246,12 +245,9 @@ test(module, 'redirect should extend existing query params', function(t) { CLIENT.get(join(LOCALHOST, '/6?a=1'), function(err, _, res) { t.ifError(err); t.equal(res.statusCode, 302); - var parsedUrl = url.parse(res.headers.location, true); - t.deepEqual(parsedUrl.query, { - a: 1, - b: 2 - }); - t.equal(parsedUrl.query.b, 2); + var parsedUrl = new URL(res.headers.location); + t.equal(parsedUrl.searchParams.get('a'), '1'); + t.equal(parsedUrl.searchParams.get('b'), '2'); t.equal(parsedUrl.pathname, '/6'); // t.equal(res.headers.location, join(LOCALHOST, '/6?a=1&b=2')); @@ -346,8 +342,8 @@ test(module, 'redirect using opts.port', function(t) { CLIENT.get(join(LOCALHOST, '/9'), function(err, _, res) { t.ifError(err); t.equal(res.statusCode, 302); - var parsedUrl = url.parse(res.headers.location, true); - t.equal(parsedUrl.port, 3000); + var parsedUrl = new URL(res.headers.location); + t.equal(Number(parsedUrl.port), 3000); t.end(); }); }); @@ -367,8 +363,8 @@ test(module, 'redirect using external url and custom port', function(t) { CLIENT.get(join(LOCALHOST, '/9'), function(err, _, res) { t.ifError(err); t.equal(res.statusCode, 302); - var parsedUrl = url.parse(res.headers.location, true); - t.equal(parsedUrl.port, 3000); + var parsedUrl = new URL(res.headers.location); + t.equal(Number(parsedUrl.port), 3000); t.equal(parsedUrl.hostname, 'www.foo.com'); t.equal(parsedUrl.pathname, '/99'); t.end(); @@ -389,14 +385,123 @@ test(module, 'redirect using default hostname with custom port', function(t) { CLIENT.get(join(LOCALHOST, '/9'), function(err, _, res) { t.ifError(err); t.equal(res.statusCode, 302); - var parsedUrl = url.parse(res.headers.location, true); - t.equal(parsedUrl.port, 3000); + var parsedUrl = new URL(res.headers.location); + t.equal(Number(parsedUrl.port), 3000); t.equal(parsedUrl.pathname, '/99'); t.equal(res.headers.location, 'http://127.0.0.1:3000/99'); t.end(); }); }); +test(module, 'redirect opts.port exact location header', function(t) { + SERVER.get('/port-pin', function(req, res, next) { + res.redirect({ port: 3000 }, next); + }); + + CLIENT.get(join(LOCALHOST, '/port-pin'), function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 302); + t.equal(res.headers.location, 'http://127.0.0.1:3000/port-pin'); + t.end(); + }); +}); + +test( + module, + 'redirect external hostname and port exact location header', + function(t) { + SERVER.get('/extport-pin', function(req, res, next) { + res.redirect( + { hostname: 'www.foo.com', pathname: '/bar', port: 3000 }, + next + ); + }); + + CLIENT.get(join(LOCALHOST, '/extport-pin'), function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 302); + t.equal(res.headers.location, 'http://www.foo.com:3000/bar'); + t.end(); + }); + } +); + +test(module, 'redirect extend query params exact location header', function(t) { + SERVER.get('/extqs-pin', function(req, res, next) { + res.redirect({ query: { b: '2' } }, next); + }); + + CLIENT.get(join(LOCALHOST, '/extqs-pin?a=1'), function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 302); + var loc = res.headers.location; + var u = new URL(loc); + t.equal(u.searchParams.get('a'), '1'); + t.equal(u.searchParams.get('b'), '2'); + t.equal(u.pathname, '/extqs-pin'); + t.end(); + }); +}); + +// eslint-disable-next-line max-len +test(module, 'redirect should repeat query param when same key', function(t) { + SERVER.get('/dupqs-pin', function(req, res, next) { + res.redirect({ query: { a: '2' } }, next); + }); + + CLIENT.get(join(LOCALHOST, '/dupqs-pin?a=1'), function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 302); + var u = new URL(res.headers.location); + t.deepEqual(u.searchParams.getAll('a'), ['2', '1']); + t.end(); + }); +}); + +// eslint-disable-next-line max-len +test( + module, + 'redirect with only pathname (no hostname) uses current host', + function(t) { + SERVER.get('/rel-path-pin', function(req, res, next) { + res.redirect({ pathname: '/other' }, next); + }); + + CLIENT.get(join(LOCALHOST, '/rel-path-pin'), function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 302); + var u = new URL(res.headers.location); + t.equal(u.pathname, '/other'); + t.equal(u.hostname, '127.0.0.1'); + t.end(); + }); + } +); + +// eslint-disable-next-line max-len +test( + module, + 'redirect with pathname and query (no hostname) preserves query string', + function(t) { + SERVER.get('/rel-qs-pin', function(req, res, next) { + res.redirect( + { pathname: '/other', query: { x: '1', y: '2' } }, + next + ); + }); + + CLIENT.get(join(LOCALHOST, '/rel-qs-pin'), function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 302); + var u = new URL(res.headers.location); + t.equal(u.pathname, '/other'); + t.equal(u.searchParams.get('x'), '1'); + t.equal(u.searchParams.get('y'), '2'); + t.end(); + }); + } +); + // eslint-disable-next-line test( module, diff --git a/test/server.test.js b/test/server.test.js index 44e47863..69daceb7 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -1182,6 +1182,27 @@ test( } ); +// eslint-disable-next-line max-len +test( + module, + 'req.absoluteUri() resolves plain subpath relative to current path', + function(t) { + SERVER.get('/base-path', function(req, res, next) { + var prefix = 'http://127.0.0.1:' + PORT; + t.equal(req.absoluteUri('child'), prefix + '/base-path/child'); + t.equal(req.absoluteUri('/absolute'), prefix + '/absolute'); + res.send(); + next(); + }); + + CLIENT.get('/base-path', function(err, _, res) { + t.ifError(err); + t.equal(res.statusCode, 200); + t.end(); + }); + } +); + test(module, 'GH-693 sending multiple response header values', function(t) { SERVER.get('/', function(req, res, next) { res.link('/', 'self'); diff --git a/test/utils.test.js b/test/utils.test.js index f4b56d32..813ef438 100644 --- a/test/utils.test.js +++ b/test/utils.test.js @@ -2,6 +2,7 @@ /* eslint-disable func-names */ var mergeQs = require('../lib/utils').mergeQs; +var parseRequestUrl = require('../lib/utils').parseRequestUrl; if (require.cache[__dirname + '/lib/helper.js']) { delete require.cache[__dirname + '/lib/helper.js']; @@ -12,6 +13,77 @@ var helper = require('./lib/helper.js'); var test = helper.test; +// parseRequestUrl + +test(module, 'parseRequestUrl: plain path', function(t) { + var r = parseRequestUrl('/foo/bar'); + + t.equal(r.pathname, '/foo/bar'); + t.equal(r.search, null); + t.equal(r.query, null); + t.equal(r.hash, null); + t.equal(r.path, '/foo/bar'); + t.equal(r.href, '/foo/bar'); + t.equal(r.hostname, null); + t.equal(r.port, null); + t.done(); +}); + +test(module, 'parseRequestUrl: path with query string', function(t) { + var r = parseRequestUrl('/foo?a=1&b=2'); + + t.equal(r.pathname, '/foo'); + t.equal(r.search, '?a=1&b=2'); + t.equal(r.query, 'a=1&b=2'); + t.equal(r.path, '/foo?a=1&b=2'); + t.equal(r.href, '/foo?a=1&b=2'); + t.done(); +}); + +test(module, 'parseRequestUrl: path with hash', function(t) { + var r = parseRequestUrl('/foo/bar#section'); + + t.equal(r.pathname, '/foo/bar'); + t.equal(r.hash, '#section'); + t.equal(r.search, null); + t.equal(r.href, '/foo/bar#section'); + t.done(); +}); + +test(module, 'parseRequestUrl: absolute URL', function(t) { + var r = parseRequestUrl('http://example.com/foo?x=1'); + + t.equal(r.pathname, '/foo'); + t.equal(r.search, '?x=1'); + t.equal(r.query, 'x=1'); + t.equal(r.hostname, 'example.com'); + t.equal(r.port, null); + t.equal(r.path, '/foo?x=1'); + t.done(); +}); + +test(module, 'parseRequestUrl: absolute URL with port', function(t) { + var r = parseRequestUrl('http://example.com:8080/foo'); + + t.equal(r.pathname, '/foo'); + t.equal(r.hostname, 'example.com'); + t.equal(r.port, '8080'); + t.equal(r.search, null); + t.done(); +}); + +test(module, 'parseRequestUrl: OPTIONS * request-target', function(t) { + var r = parseRequestUrl('*'); + + t.equal(r.pathname, '*'); + t.equal(r.search, null); + t.equal(r.query, null); + t.equal(r.hash, null); + t.equal(r.path, '*'); + t.equal(r.href, '*'); + t.done(); +}); + // mergeQs test(module, 'merge qs', function(t) {