From 9c1e48dca53720e1788be7394fadac66904dd068 Mon Sep 17 00:00:00 2001
From: Bryan English <bryan@bryanenglish.com>
Date: Thu, 25 May 2017 00:32:04 -0700
Subject: [PATCH] http: client keep-alive for UNIX domain sockets

Makes `Connection: keep-alive` behave correctly when making client
connections to UNIX domain sockets.

Prior to this, connections would never be re-used, but the keep-alive
would cause the connections to stick around until they time out. This
would lead to an eventual EMFILE error due to all the connections
staying open. This was due to http.Agent not properly supporting UNIX
domain sockets.

PR-URL: https://github.com/nodejs/node/pull/13214
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/_http_agent.js                            |  7 ++++
 lib/_http_client.js                           | 20 +---------
 test/parallel/test-http-agent-getname.js      | 12 +++++-
 .../test-http-unix-socket-keep-alive.js       | 37 +++++++++++++++++++
 4 files changed, 57 insertions(+), 19 deletions(-)
 create mode 100644 test/parallel/test-http-unix-socket-keep-alive.js

diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index ddd36c158ec3ca..e71f0cb441f86e 100644
--- a/lib/_http_agent.js
+++ b/lib/_http_agent.js
@@ -131,6 +131,9 @@ Agent.prototype.getName = function getName(options) {
   if (options.family === 4 || options.family === 6)
     name += ':' + options.family;
 
+  if (options.socketPath)
+    name += ':' + options.socketPath;
+
   return name;
 };
 
@@ -147,6 +150,8 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/,
 
   options = util._extend({}, options);
   util._extend(options, this.options);
+  if (options.socketPath)
+    options.path = options.socketPath;
 
   if (!options.servername) {
     options.servername = options.host;
@@ -199,6 +204,8 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) {
   var self = this;
   options = util._extend({}, options);
   util._extend(options, self.options);
+  if (options.socketPath)
+    options.path = options.socketPath;
 
   if (!options.servername) {
     options.servername = options.host;
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 29ea688ecb1bf0..4b7c755d634a55 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -241,23 +241,7 @@ function ClientRequest(options, cb) {
     this._deferToConnect(null, null, () => this._flush());
   };
 
-  var newSocket;
-  if (this.socketPath) {
-    this._last = true;
-    this.shouldKeepAlive = false;
-    var optionsPath = {
-      path: this.socketPath,
-      timeout: this.timeout,
-      rejectUnauthorized: !!options.rejectUnauthorized
-    };
-    newSocket = this.agent.createConnection(optionsPath, oncreate);
-    if (newSocket && !called) {
-      called = true;
-      this.onSocket(newSocket);
-    } else {
-      return;
-    }
-  } else if (this.agent) {
+  if (this.agent) {
     // If there is an agent we should default to Connection:keep-alive,
     // but only if the Agent will actually reuse the connection!
     // If it's not a keepAlive agent, and the maxSockets==Infinity, then
@@ -275,7 +259,7 @@ function ClientRequest(options, cb) {
     this._last = true;
     this.shouldKeepAlive = false;
     if (typeof options.createConnection === 'function') {
-      newSocket = options.createConnection(options, oncreate);
+      const newSocket = options.createConnection(options, oncreate);
       if (newSocket && !called) {
         called = true;
         this.onSocket(newSocket);
diff --git a/test/parallel/test-http-agent-getname.js b/test/parallel/test-http-agent-getname.js
index f84996ec013eec..4b4e9ac26b44a5 100644
--- a/test/parallel/test-http-agent-getname.js
+++ b/test/parallel/test-http-agent-getname.js
@@ -1,8 +1,9 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 const http = require('http');
+const path = require('path');
 
 const agent = new http.Agent();
 
@@ -31,6 +32,15 @@ assert.strictEqual(
   '0.0.0.0:80:192.168.1.1'
 );
 
+// unix socket
+const socketPath = path.join(common.tmpDir, 'foo', 'bar');
+assert.strictEqual(
+  agent.getName({
+    socketPath
+  }),
+  `localhost:::${socketPath}`
+);
+
 for (const family of [0, null, undefined, 'bogus'])
   assert.strictEqual(agent.getName({ family }), 'localhost::');
 
diff --git a/test/parallel/test-http-unix-socket-keep-alive.js b/test/parallel/test-http-unix-socket-keep-alive.js
new file mode 100644
index 00000000000000..668c440325e0ec
--- /dev/null
+++ b/test/parallel/test-http-unix-socket-keep-alive.js
@@ -0,0 +1,37 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+const server = http.createServer((req, res) => res.end());
+
+common.refreshTmpDir();
+
+server.listen(common.PIPE, common.mustCall(() =>
+  asyncLoop(makeKeepAliveRequest, 10, common.mustCall(() =>
+    server.getConnections(common.mustCall((err, conns) => {
+      assert.ifError(err);
+      assert.strictEqual(conns, 1);
+      server.close();
+    }))
+  ))
+));
+
+function asyncLoop(fn, times, cb) {
+  fn(function handler() {
+    if (--times) {
+      fn(handler);
+    } else {
+      cb();
+    }
+  });
+}
+function makeKeepAliveRequest(cb) {
+  http.get({
+    socketPath: common.PIPE,
+    headers: { connection: 'keep-alive' }
+  }, (res) => res.on('data', common.mustNotCall())
+    .on('error', assert.fail)
+    .on('end', cb)
+  );
+}