Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ssl socket leak #33

Closed
wants to merge 1 commit into from
Closed

fix: ssl socket leak #33

wants to merge 1 commit into from

Conversation

alexpenev-s
Copy link
Contributor

-closes:#32

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @fengmk2, @lattmann and @pmalouin to be potential reviewers

@fengmk2
Copy link
Member

fengmk2 commented Mar 14, 2016

@alexpenev-s
Copy link
Contributor Author

It extends its own copy of options.
On a subsequent request, when addRequest()is called. It is called with the options specified by https.request(options) which don't have the ca property.

The issue occurs when ca is in the global options object and not in the options coming from the request.

@fengmk2
Copy link
Member

fengmk2 commented Mar 14, 2016

@saperal Can you add a test case for this change?

And you can also send a pr for nodejs itself too. https://github.com/nodejs/node/blob/master/lib/_http_agent.js#L108

@fengmk2
Copy link
Member

fengmk2 commented Apr 5, 2016

@saperal nice move! I will merge this soon. https://github.com/nodejs/node/pull/5713/files

@fengmk2 fengmk2 self-assigned this Apr 5, 2016
@fengmk2 fengmk2 added the bug label Apr 5, 2016
@fengmk2
Copy link
Member

fengmk2 commented Apr 5, 2016

Landed 88d1e1e

@fengmk2 fengmk2 closed this Apr 5, 2016
@fengmk2
Copy link
Member

fengmk2 commented Apr 5, 2016

2.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agentkeepalive might leak SSL sockets
3 participants