Skip to content

Commit

Permalink
Simplify get function and opts/arg juggling
Browse files Browse the repository at this point in the history
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.

Also this fixes #72.
  • Loading branch information
floatdrop committed Jun 25, 2015
1 parent b345276 commit ebf8f64
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 32 deletions.
52 changes: 27 additions & 25 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,13 @@ 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',
Expand All @@ -46,15 +52,21 @@ function got(url, opts, cb) {
var body = opts.body;
var json = opts.json;
var timeout = opts.timeout;
var query = opts.query;
var proxy;
var redirectCount = 0;

delete opts.encoding;
delete opts.body;
delete opts.json;
delete opts.timeout;
delete opts.query;

if (opts.query) {
if (typeof opts.query !== 'string') {
opts.query = querystring.stringify(opts.query);
}
opts.path = opts.pathname + '?' + opts.query;
delete opts.query;
}

if (json) {
opts.headers.accept = opts.headers.accept || 'application/json';
Expand Down Expand Up @@ -90,15 +102,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' ||
Expand All @@ -108,16 +117,11 @@ 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(opts);
}
}

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;

Expand All @@ -135,16 +139,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;
}

Expand Down Expand Up @@ -230,7 +232,7 @@ function got(url, opts, cb) {
req.end();
}

get(url, opts, cb);
get(opts, cb);

return proxy;
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"node": ">=0.10.0"
},
"scripts": {
"test": "tap test/test-*.js",
"test": "tap test/test-*.js --timeout=2",
"coverage": "istanbul cover tape --report html -- test/test-*.js"
},
"files": [
Expand Down
3 changes: 1 addition & 2 deletions test/test-arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/test-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ 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();
});
});

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();
Expand Down
4 changes: 2 additions & 2 deletions test/test-redirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit ebf8f64

Please sign in to comment.