https.request(): Agent name inconsistency when servername not supplied

Version: v4.4.4
Platform: Linux XXXXXXXXXXX 3.13.0-62-generic url: improve parsing speed #102~precise1-Ubuntu SMP Wed Aug 12 14:09:54 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

The PR #4389 introduced servername in the computation of the agent name.

If the caller of https.request() does not supply servername, _http_agent may introduce it in its method createSocket() if the header host is supplied. See here.

That introduced an inconsistency issue in the method addRequest(), because getName() is called before createSocket() is called, which means on first call, it will not contain servername, while it will on subsequent calls, in particular when handling destruction.

Call to getName():

node/lib/_http_agent.js

Line 121 in 572e28e

var name = this.getName(options);
Call to createSocket() in the same method:

node/lib/_http_agent.js

Line 144 in 572e28e

this.createSocket(req, options, function(err, newSocket) {

That created an issue for us. We are a high traffic site, and node was leaving a lot of sockets in TIME_WAIT state behind.

Expected

agent name computation and management in sockets and freeSockets should be consistent.

App-level workaround:

Always supply servername in the options of https.request()

(Apologies, I haven't come up with a minimal test to highlight the problem, I figure it was more important to report the problem first, and I'll try to come up with a test after that.)

Are you using a custom agent? The default HTTPS agent includes servername iff options.servername && options.servername !== options.host.

@timotheeg

@timotheeg ping

@bnoordhuis

Hey. Sorry I missed the message from @bnoordhuis. I'm using an agent indeed, but it's the one supplied by https, which I instantiate as:

var secure_agent = new https.Agent({
    keepAlive: true,
    maxSockets: CONFIG.max_http_sockets || 1000,
    maxFreeSockets: CONFIG.max_http_freeSockets || 256,
    keepAliveMsecs: 15 * 1000
});

Sorry, with the workaround of supplying servername, things work for me, and I still haven't taken the time to supply a test case for the issue 😓 .

I think the issue can be seen by code inspection however. And I realized that I missed mentioning a piece of info about my requests, which helps shows the issue clearer. So further info are below.

I am requesting for options.host as an ip but options.header.host as a domain. That caused servername and host to be out of sync based on this. So the second getName() call would have had ``options.servername && options.servername !== options.host` true, and compute the name inconsistently.

So to sum up: https.request() ends up calling agent.addRequest(), which calls getName(), if servername was not supplied in options, the test options.servername && options.servername !== options.host is false and servername is not taken into consideration. addRequest() calls createSocket() later, which then introduces servername. That snippet can cause options.servername and options.host to not be equal in a ip/host header setup, and causes the next call to getName() (that occurs immediately after) to use servername, and compute an agent name that is inconsistent with the previous one.

I hope that makes sense. I don't know when I'll have the time to make that test case, I'll try over the weekend :/

https: fix memory leak with https.request()

@timotheeg

@timotheeg Thank you for reporting this.

The issue has been fixed in Node.js master branch and is likely to be included in the future releases.

https: fix memory leak with https.request()

Navigation Menu

Repository navigation

https.request(): Agent name inconsistency when servername not supplied #6687

Expected

App-level workaround:

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

https.request(): Agent name inconsistency when servername not supplied #6687

Description

Expected

App-level workaround:

Activity

bnoordhuis commented on Jun 27, 2016

cjihrig commented on Jul 5, 2016

timotheeg commented on Jul 7, 2016

imyller commented on Sep 21, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions