Repository navigation
Undocumented behavior for handling options of http.request #47624
Description
Activity
- addedhttpIssues and PRs related to the http subsystem.Issues and PRs related to the http subsystem.
on Apr 20, 2023 I think this problem can be solved by adding validations in
urlToHttpOptions(url)function, which is called from theelse if (isURL(input))branch as @ZEDCWT mentioned. Since we deliberately added the query string inpath, (likeOpt.path + "?A=B"), but we didn't modify thesearchandqueryfields in this object. Therefore, although this object is an URL object, thepath,searchandqueryfields are inconsistent, so theurlToHttpOptions(url)function cannot recognize the added search parameters.If it is ok, can I take this issue, thanks!
I think we've also encountered this issue.
It seems that in v20, if the URL’s
pathproperty is set after parsing the URL, then used for an HTTP request, the path is absent from the request.I've created this reproducer:
const https = require('https'); const url = require('url'); // Works in v18 fails in v20: const opts = url.parse('https://postman-echo.com'); opts.path = '/get'; // Works in both versions: // const opts = url.parse('https://postman-echo.com/get'); // Curiously, deleting either of these properties causes it to work in both versions: // delete opts.href; // delete opts.protocol; console.log('node version:', process.version); console.log('parsed url:', opts); https.request(opts, response => { console.log('status:', response.statusCode); response.on('data', data => process.stdout.write(data)); }).end();
v20.1.0 output:
node version: v20.1.0 parsed url: Url { protocol: 'https:', slashes: true, auth: null, host: 'postman-echo.com', port: null, hostname: 'postman-echo.com', hash: null, search: null, query: null, pathname: '/', path: '/get', href: 'https://postman-echo.com/' } status: 302 Found. Redirecting to https://docs.postman-echo.comv18.16.0 output (expected)
node version: v18.16.0 parsed url: Url { protocol: 'https:', slashes: true, auth: null, host: 'postman-echo.com', port: null, hostname: 'postman-echo.com', hash: null, search: null, query: null, pathname: '/', path: '/get', href: 'https://postman-echo.com/' } status: 200 { "args": {}, "headers": { "x-forwarded-proto": "https", "x-forwarded-port": "443", "host": "postman-echo.com", "x-amzn-trace-id": "Root=1-64532010-42007c685001b2c957e08cbc" }, "url": "https://postman-echo.com/get" }@nodejs/http probably also @nodejs/url
cc @Trott
FWIW, setting
pathnameinstead ofpathseems to work as expected (as implied above, but just to make that clear for anyone looking for a quick fix).Is e3bb668 the likely commit that introduced this change? @anonrig
I don't think it is likely that the
isURLled to this. Specifically, we selectedhrefandprotocolbecause those two attributes are the easiest & fastest to calculate to use as an identifiable. This was done because of the lazy-loading feature of URL parser.FWIW, setting pathname instead of path seems to work as expected
Unfortunately, the old
urlclass usespathattribute whereas the WHATWG API uses botpathandpathnamefor defining the pathname of the URL.I think the breaking change is related to how
urlToHttpOptionsis changed. Previously we were not passing...urlreferencing at: ad2c3c0#diff-26e6b1e2a90040b1ce20e7f305b2ff3140b483f863ee2573b4c6af9b696ad31eR1111 @aduh95- addedurlIssues and PRs related to the legacy built-in url module.Issues and PRs related to the legacy built-in url module.
on May 4, 2023 Thanks for the bisect @Trott. I'll take a look at it.
Version
v20.0.0
Platform
Microsoft Windows NT 10.0.19045.0 x64
Subsystem
No response
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
Always
What is the expected behavior? Why is that the expected behavior?
Running using v19.9.0, it prints
{ "args": { "A": "B" }, "headers": { "Host": "httpbin.org", .... }, .... "url": "https://httpbin.org/get?A=B" }What do you see instead?
{ "args": {}, "headers": { "Host": "httpbin.org", ... }, ... "url": "https://httpbin.org/get" }Additional information
Before #47339,
url.isURLchecks if property href & origin exists, and it changed to check if property href & protocol exist now.So before that, the
Optabove goes to the else branch ofClientRequest, but now it goes to theelse if (isURL(input))branch, in which it ignores thepathproperty given and just glues pathname & search together.Reading the document, it says
url can be a string or a URL objectalso never mentions anything about search or pathname.since I'm not providing a WHATWG URL object, I'm expecting to call this signature
http.request(options[, callback])retaining mypathproperty as what v19 and before do.