http: do not rely on the 'agentRemove' event

Do not use the `'agentRemove'` event to null `socket._httpMessage` as that event is public and can be used to not keep a request in the agent. Backport-PR-URL: #20786 PR-URL: #20786 Fixes: #20690 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]>
nodejs · Jul 13, 2018 · 52cc2d2 · 52cc2d2 · lpinca · Jul 14, 2018
1 parent 873ee2e
commit 52cc2d2
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 1 deletion.
diff --git a/lib/_http_agent.js b/lib/_http_agent.js
@@ -277,7 +277,6 @@ function installListeners(agent, s, options) {
     s.removeListener('close', onClose);
     s.removeListener('free', onFree);
     s.removeListener('agentRemove', onRemove);
-    s._httpMessage = null;
   }
   s.on('agentRemove', onRemove);
 }

diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -470,6 +470,7 @@ function socketOnData(d) {
       // TODO(isaacs): Need a way to reset a stream to fresh state
       // IE, not flowing, and not explicitly paused.
       socket._readableState.flowing = null;
+      socket._httpMessage = null;
 
       req.emit(eventName, res, socket, bodyHead);
       req.emit('close');

diff --git a/test/parallel/test-http-agent-remove.js b/test/parallel/test-http-agent-remove.js
@@ -0,0 +1,21 @@
+'use strict';
+const { mustCall } = require('../common');
+
+const http = require('http');
+const { strictEqual } = require('assert');
+
+const server = http.createServer(mustCall((req, res) => {
+  res.flushHeaders();
+}));
+
+server.listen(0, mustCall(() => {
+  const req = http.get({
+    port: server.address().port
+  }, mustCall(() => {
+    const { socket } = req;
+    socket.emit('agentRemove');
+    strictEqual(socket._httpMessage, req);
+    socket.destroy();
+    server.close();
+  }));
+}));