search

node-modules / agentkeepalive

Support keepalive http agent.
MIT License
579 stars 57 forks source link

fix: ssl socket leak #33

Closed alexpenev-s closed 8 years ago

alexpenev-s commented 8 years ago

-closes:#32

mention-bot commented 8 years ago

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

fengmk2 commented 8 years ago

https://github.com/node-modules/agentkeepalive/pull/33/files#diff-5f7fb0850412c6be189faeddea6c5359R226 createSocket() will do the _extend logic.

alexpenev-s commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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

fengmk2 commented 8 years ago

Landed 88d1e1ee167f28819edce7349d0155b91c1a0d75

fengmk2 commented 8 years ago

2.1.1