Repository navigation
https: Agent.createConnection mutates options object #31119
Description
createConnection should create a copy instead of mutating the passed options object.
const options = { port: 3000 };
htptpAgent.createConnection('localhost', 2000, options);
assert(options.port, 3000); // fails
assert(options.host, undefined); // fails
Activity
ronag commentedon Dec 28, 2019
Good first issue?
[-]https: createConnection mutates options object[/-][+]https: Agent.createConnection mutates options object[/+]vighnesh153 commentedon Jan 1, 2020
Hey @ronag , @Trott . I would like to go fix this issue. I have read the
CONTRIBUTING.mdfile. Is there anything else I need to know before proceeding like should I just create a pull request after solving the issue or do I have to do something else as well? This is my first time contributing to an open-source project. Any help would be appreciated.ronag commentedon Jan 1, 2020
@vighnesh153: Create a PR and follow the instructions. Also please add a test that fails before fix and succeeds after fix.
vighnesh153 commentedon Jan 1, 2020
Alright. I will start working on it right away.
vighnesh153 commentedon Jan 1, 2020
In the
lib/_http_agent.js, I can see a line:Agent.prototype.createConnection = net.createConnection;That leads me to the
lib/net.jsfile. There, I sawSo, from there, I went to
connectdefinition and its comment-doc says that it has 3 forms:None of them matches the one that is mentioned in the issue. Am I looking at the correct function?
Also, I see a
testdirectory and inside it, there are several other directories. One of them beinginternet. Should I add a new test file in that directory or should I use an existing one?ronag commentedon Jan 1, 2020
You should be looking for
httpsnothttp.See the test in
test/parallelfor examples and you should either add a new test there or modify and existing one.vighnesh153 commentedon Jan 1, 2020
Ok. Got it. I will look into that.
vighnesh153 commentedon Jan 1, 2020
While running the tests, some of the tests pass, but many others throw unhandled errors. I haven't touched the code yet. Am I missing out on something?
Edit:
Those were some experimental features. I guess I should ignore those errors.
vighnesh153 commentedon Jan 2, 2020
@ronag This is the PR link. It is a bit messy but I have squashed all commits into one at the end. Please give your feedback on it. #31151
https: prevent options object from being mutated
https: prevent options object from being mutated
https: prevent options object from being mutated