http: wait for both prefinish/end to keepalive

When `free`ing the socket to be reused in keep-alive Agent wait for both `prefinish` and `end` events. Otherwise the next request may be written before the previous one has finished sending the body, leading to a parser errors. PR-URL: #7149 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>

Author: indutny

lib/_http_client.js

@@ -156,6 +156,8 @@ function ClientRequest(options, cb){
 self._flush();
 self=null;
 });
+
+this._ended=false;
 }
 
 util.inherits(ClientRequest,OutgoingMessage);
@@ -427,6 +429,7 @@ function parserOnIncomingClient(res, shouldKeepAlive){
 
 // add our listener first, so that we guarantee socket cleanup
 res.on('end',responseOnEnd);
+req.on('prefinish',requestOnPrefinish);
 varhandled=req.emit('response',res);
 
 // If the user did not listen for the 'response' event, then they
@@ -439,9 +442,7 @@ function parserOnIncomingClient(res, shouldKeepAlive){
 }
 
 // client
-functionresponseOnEnd(){
-varres=this;
-varreq=res.req;
+functionresponseKeepAlive(res,req){
 varsocket=req.socket;
 
 if(!req.shouldKeepAlive){
@@ -465,6 +466,26 @@ function responseOnEnd(){
 }
 }
 
+functionresponseOnEnd(){
+constres=this;
+constreq=this.req;
+
+req._ended=true;
+if(!req.shouldKeepAlive||req.finished)
+responseKeepAlive(res,req);
+}
+
+functionrequestOnPrefinish(){
+constreq=this;
+constres=this.res;
+
+if(!req.shouldKeepAlive)
+return;
+
+if(req._ended)
+responseKeepAlive(res,req);
+}
+
 functionemitFreeNT(socket){
 socket.emit('free');
 }

test/parallel/test-http-client-keep-alive-release-before-finish.js

@@ -0,0 +1,38 @@
+'use strict';
+constcommon=require('../common');
+consthttp=require('http');
+
+constserver=http.createServer((req,res)=>{
+res.end();
+}).listen(common.PORT,common.mustCall(()=>{
+constagent=newhttp.Agent({
+maxSockets: 1,
+keepAlive: true
+});
+
+constpost=http.request({
+agent: agent,
+method: 'POST',
+port: common.PORT,
+},common.mustCall((res)=>{
+res.resume();
+}));
+
+/* What happens here is that the server `end`s the response before we send
+ * `something`, and the client thought that this is a green light for sending
+ * next GET request
+ */
+post.write(Buffer.alloc(16*1024,'X'));
+setTimeout(()=>{
+post.end('something');
+},100);
+
+http.request({
+agent: agent,
+method: 'GET',
+port: common.PORT,
+},common.mustCall((res)=>{
+server.close();
+res.connection.end();
+})).end();
+}));