From f9b739ebbcb77bc5a0fc8e3d20ad2eae3285b039 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Sun, 1 Jul 2018 11:00:24 -0400 Subject: [PATCH] http: allow url and options to be passed to http*.request and http*.get Fixes: https://github.com/nodejs/node/issues/20795 PR-URL: https://github.com/nodejs/node/pull/21616 Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Ron Korving Reviewed-By: James M Snell Reviewed-By: Matteo Collina --- doc/api/http.md | 19 +++++++++++-- doc/api/https.md | 14 ++++++++-- lib/_http_client.js | 29 +++++++++++++------- lib/http.js | 8 +++--- test/parallel/test-http-request-arguments.js | 28 +++++++++++++++++++ 5 files changed, 79 insertions(+), 19 deletions(-) create mode 100644 test/parallel/test-http-request-arguments.js diff --git a/doc/api/http.md b/doc/api/http.md index 1776012f814c7f..78f8bc4b6b133f 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -1796,15 +1796,20 @@ The `requestListener` is a function which is automatically added to the [`'request'`][] event. ## http.get(options[, callback]) +## http.get(url[, options][, callback]) -* `options` {Object | string | URL} Accepts the same `options` as +* `url` {string | URL} +* `options` {Object} Accepts the same `options` as [`http.request()`][], with the `method` always set to `GET`. Properties that are inherited from the prototype are ignored. * `callback` {Function} @@ -1868,15 +1873,20 @@ Global instance of `Agent` which is used as the default for all HTTP client requests. ## http.request(options[, callback]) +## http.request(url[, options][, callback]) -* `options` {Object | string | URL} +* `url` {string | URL} +* `options` {Object} * `protocol` {string} Protocol to use. **Default:** `'http:'`. * `host` {string} A domain name or IP address of the server to issue the request to. **Default:** `'localhost'`. @@ -1918,10 +1928,13 @@ changes: Node.js maintains several connections per server to make HTTP requests. This function allows one to transparently issue requests. -`options` can be an object, a string, or a [`URL`][] object. If `options` is a +`url` can be a string or a [`URL`][] object. If `url` is a string, it is automatically parsed with [`new URL()`][]. If it is a [`URL`][] object, it will be automatically converted to an ordinary `options` object. +If both `url` and `options` are specified, the objects are merged, with the +`options` properties taking precedence. + The optional `callback` parameter will be added as a one-time listener for the [`'response'`][] event. diff --git a/doc/api/https.md b/doc/api/https.md index 4fdd2fe7bc8111..51e6e5c1626738 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -112,14 +112,19 @@ https.createServer(options, (req, res) => { ``` ## https.get(options[, callback]) +## https.get(url[, options][, callback]) -- `options` {Object | string | URL} Accepts the same `options` as +- `url` {string | URL} +- `options` {Object} Accepts the same `options` as [`https.request()`][], with the `method` always set to `GET`. - `callback` {Function} @@ -155,9 +160,13 @@ added: v0.5.9 Global instance of [`https.Agent`][] for all HTTPS client requests. ## https.request(options[, callback]) +## https.request(url[, options][, callback]) -- `options` {Object | string | URL} Accepts all `options` from +- `url` {string | URL} +- `options` {Object} Accepts all `options` from [`http.request()`][], with some differences in default values: - `protocol` **Default:** `'https:'` - `port` **Default:** `443` diff --git a/lib/_http_client.js b/lib/_http_client.js index 4542a81cf0e9a1..c4f753412ca83b 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -60,16 +60,16 @@ function validateHost(host, name) { } let urlWarningEmitted = false; -function ClientRequest(options, cb) { +function ClientRequest(input, options, cb) { OutgoingMessage.call(this); - if (typeof options === 'string') { - const urlStr = options; + if (typeof input === 'string') { + const urlStr = input; try { - options = urlToOptions(new URL(urlStr)); + input = urlToOptions(new URL(urlStr)); } catch (err) { - options = url.parse(urlStr); - if (!options.hostname) { + input = url.parse(urlStr); + if (!input.hostname) { throw err; } if (!urlWarningEmitted && !process.noDeprecation) { @@ -80,14 +80,23 @@ function ClientRequest(options, cb) { 'DeprecationWarning', 'DEP0109'); } } - } else if (options && options[searchParamsSymbol] && - options[searchParamsSymbol][searchParamsSymbol]) { + } else if (input && input[searchParamsSymbol] && + input[searchParamsSymbol][searchParamsSymbol]) { // url.URL instance - options = urlToOptions(options); + input = urlToOptions(input); } else { - options = util._extend({}, options); + cb = options; + options = input; + input = null; } + if (typeof options === 'function') { + cb = options; + options = null; + } + + options = util._extend(input || {}, options || {}); + var agent = options.agent; var defaultAgent = options._defaultAgent || Agent.globalAgent; if (agent === false) { diff --git a/lib/http.js b/lib/http.js index 9ed6b3d1de8721..660de78650f45c 100644 --- a/lib/http.js +++ b/lib/http.js @@ -37,12 +37,12 @@ function createServer(opts, requestListener) { return new Server(opts, requestListener); } -function request(options, cb) { - return new ClientRequest(options, cb); +function request(url, options, cb) { + return new ClientRequest(url, options, cb); } -function get(options, cb) { - var req = request(options, cb); +function get(url, options, cb) { + var req = request(url, options, cb); req.end(); return req; } diff --git a/test/parallel/test-http-request-arguments.js b/test/parallel/test-http-request-arguments.js new file mode 100644 index 00000000000000..5cdd514fd50685 --- /dev/null +++ b/test/parallel/test-http-request-arguments.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +// Test providing both a url and options, with the options partially +// replacing address and port portions of the URL provided. +{ + const server = http.createServer( + common.mustCall((req, res) => { + assert.strictEqual(req.url, '/testpath'); + res.end(); + server.close(); + }) + ); + server.listen( + 0, + common.mustCall(() => { + http.get( + 'http://example.com/testpath', + { hostname: 'localhost', port: server.address().port }, + common.mustCall((res) => { + res.resume(); + }) + ); + }) + ); +}