https: Agent.createConnection mutates options object

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

Good first issue?

@ronag

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.

@vighnesh153

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

Alright. I will start working on it right away.

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?

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.

Ok. Got it. I will look into that.

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.

@ronag

@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

Navigation Menu

Repository navigation

https: Agent.createConnection mutates options object #31119

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

https: Agent.createConnection mutates options object #31119

Description

Activity

ronag commented on Dec 28, 2019

vighnesh153 commented on Jan 1, 2020

ronag commented on Jan 1, 2020

vighnesh153 commented on Jan 1, 2020

vighnesh153 commented on Jan 1, 2020

ronag commented on Jan 1, 2020

vighnesh153 commented on Jan 1, 2020

vighnesh153 commented on Jan 1, 2020

vighnesh153 commented on Jan 2, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions