Node: https: Agent.createConnection mutates options object

[ronag]

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

[ronag]

Good first issue?

[vighnesh153]

Hey @ronag , @Trott . I would like to go fix this issue. I have read the CONTRIBUTING.md file. 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]

@vighnesh153: Create a PR and follow the instructions. Also please add a test that fails before fix and succeeds after fix.

[vighnesh153]

Alright. I will start working on it right away.

[vighnesh153]

In the lib/_http_agent.js, I can see a line:
Agent.prototype.createConnection = net.createConnection;
That leads me to the lib/net.js file. There, I saw

module.exports = {
...,
  createConnection: connect,
...
}

So, from there, I went to connect definition and its comment-doc says that it has 3 forms:

// There are various forms:
//
// connect(options, [cb])
// connect(port, [host], [cb])
// connect(path, [cb]);

None of them matches the one that is mentioned in the issue. Am I looking at the correct function?

Also, I see a test directory and inside it, there are several other directories. One of them being internet. Should I add a new test file in that directory or should I use an existing one?

[ronag]

You should be looking for https not http.

See the test in test/parallel for examples and you should either add a new test there or modify and existing one.

[vighnesh153]

Ok. Got it. I will look into that.

[vighnesh153]

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]

@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. https://github.com/nodejs/node/pull/31151