From b7d4f8caca90655036e78d5793101fa80d19e1d3 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Thu, 25 Jun 2015 20:43:23 +0300 Subject: [PATCH] Simplify get function and opts/arg juggling Parse url at the top of got function and work with request object all the way down. This reduces complexity and unnecessary delete calls in redirects. --- index.js | 61 +++++++++++++++++++++++++----------------- test/test-arguments.js | 3 +-- test/test-error.js | 4 +-- test/test-redirects.js | 4 +-- 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/index.js b/index.js index 73f0968..134cb88 100644 --- a/index.js +++ b/index.js @@ -35,18 +35,32 @@ function got(url, opts, cb) { opts = {}; } - opts = objectAssign({}, opts); + opts = objectAssign( + { + protocol: 'http:' + }, + typeof url === 'string' ? urlLib.parse(prependHttp(url)) : url, + opts + ); opts.headers = objectAssign({ 'user-agent': 'https://github.com/sindresorhus/got', 'accept-encoding': 'gzip,deflate' }, lowercaseKeys(opts.headers)); + if (opts.query) { + if (typeof opts.query !== 'string') { + opts.query = querystring.stringify(opts.query); + } + + opts.path = opts.pathname + '?' + opts.query; + delete opts.query; + } + var encoding = opts.encoding; var body = opts.body; var json = opts.json; var timeout = opts.timeout; - var query = opts.query; var proxy; var redirectCount = 0; @@ -54,7 +68,6 @@ function got(url, opts, cb) { delete opts.body; delete opts.json; delete opts.timeout; - delete opts.query; if (json) { opts.headers.accept = opts.headers.accept || 'application/json'; @@ -90,15 +103,12 @@ function got(url, opts, cb) { throw new GotError('got can not be used as stream when options.json is used'); } - function get(url, opts, cb) { - var parsedUrl = typeof url === 'string' ? urlLib.parse(prependHttp(url)) : url; - var fn = parsedUrl.protocol === 'https:' ? https : http; - var arg = objectAssign({}, parsedUrl, opts); + function get(opts, cb) { + var fn = opts.protocol === 'https:' ? https : http; + var url = urlLib.format(opts); - url = typeof url === 'string' ? prependHttp(url) : urlLib.format(url); - - if (arg.agent === undefined) { - arg.agent = infinityAgent[fn === https ? 'https' : 'http'].globalAgent; + if (opts.agent === undefined) { + opts.agent = infinityAgent[fn === https ? 'https' : 'http'].globalAgent; if (process.version.indexOf('v0.10') === 0 && fn === https && ( typeof opts.ca !== 'undefined' || @@ -108,16 +118,19 @@ function got(url, opts, cb) { typeof opts.passphrase !== 'undefined' || typeof opts.pfx !== 'undefined' || typeof opts.rejectUnauthorized !== 'undefined')) { - arg.agent = new infinityAgent.https.Agent(opts); + opts.agent = new infinityAgent.https.Agent({ + ca: opts.ca, + cert: opts.cert, + ciphers: opts.ciphers, + key: opts.key, + passphrase: opts.passphrase, + pfx: opts.pfx, + rejectUnauthorized: opts.rejectUnauthorized + }); } } - if (query) { - arg.path = (arg.path ? arg.path.split('?')[0] : '') + '?' + (typeof query === 'string' ? query : querystring.stringify(query)); - query = undefined; - } - - var req = fn.request(arg, function (response) { + var req = fn.request(opts, function (response) { var statusCode = response.statusCode; var res = response; @@ -135,16 +148,14 @@ function got(url, opts, cb) { return; } - delete opts.host; - delete opts.hostname; - delete opts.port; - delete opts.path; + var redirectUrl = urlLib.resolve(url, res.headers.location); + var redirectOpts = objectAssign(opts, urlLib.parse(redirectUrl)); if (proxy) { - proxy.emit('redirect', res, opts); + proxy.emit('redirect', res, redirectOpts); } - get(urlLib.resolve(url, res.headers.location), opts, cb); + get(redirectOpts, cb); return; } @@ -230,7 +241,7 @@ function got(url, opts, cb) { req.end(); } - get(url, opts, cb); + get(opts, cb); return proxy; } diff --git a/test/test-arguments.js b/test/test-arguments.js index 209a669..36b4b09 100644 --- a/test/test-arguments.js +++ b/test/test-arguments.js @@ -46,10 +46,9 @@ test('overrides querystring from opts', function (t) { }); }); -// Error says http://localhost:6767/test, but request was to http://localhost:6767/ test('pathname confusion', function (t) { got({protocol: 'http:', hostname: s.host, port: s.port, pathname: '/test'}, function (err) { - t.ok(/http:\/\/localhost:6767\/ response code/.test(err)); + t.ok(/http:\/\/localhost:6767\/test response code/.test(err)); t.end(); }); }); diff --git a/test/test-error.js b/test/test-error.js index fee3bb9..7773966 100644 --- a/test/test-error.js +++ b/test/test-error.js @@ -18,7 +18,7 @@ test('setup', function (t) { test('error message', function (t) { got(s.url, function (err) { t.ok(err); - t.equal(err.message, 'GET http://localhost:6767 response code is 404 (Not Found)'); + t.equal(err.message, 'GET http://localhost:6767/ response code is 404 (Not Found)'); t.end(); }); }); @@ -26,7 +26,7 @@ test('error message', function (t) { test('dns error message', function (t) { got('.com', function (err) { t.ok(err); - t.equal(err.message, 'Request to http://.com failed'); + t.equal(err.message, 'Request to http://.com/ failed'); t.ok(err.nested); t.ok(/getaddrinfo ENOTFOUND/.test(err.nested.message)); t.end(); diff --git a/test/test-redirects.js b/test/test-redirects.js index 5c8e27e..df994c4 100644 --- a/test/test-redirects.js +++ b/test/test-redirects.js @@ -74,8 +74,8 @@ test('query in options are not breaking redirects', function (t) { }); }); -test('host+path in options are not breaking redirects', function (t) { - got(s.url + '/relative', {host: s.url, path: '/relative'}, function (err, data) { +test('hostname+path in options are not breaking redirects', function (t) { + got(s.url + '/relative', {hostname: s.host, path: '/relative'}, function (err, data) { t.error(err); t.equal(data, 'reached'); t.end();