You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Assert is raised when emitting 'connection' event #11017
I have recently came across a very strange issue, which I believe is a bug related to a misuse of streams with httpParsers.
It is quite easy to reproduce, using the following code:
varhttp=require("http");varserver=http.Server(function(req,res){res.writeHead(200,{});res.end();});server.on("connect",function(req,clientSocket,head){clientSocket.write("HTTP/1.1 200 Connection Established"+"\r\n"+"Proxy-agent: Node-Proxy"+"\r\n"+"\r\n");connection=clientSocket;// This shouldn't raise the assert.server.emit("connection",clientSocket);});server.listen(0);
Then, when a HTTP-CONNECT request is received, the process crashes because of the following assert:
After spending some time in debugging this, I found out that the following flow causes the assertion:
The function 'onParserExecuteCommon' in _http_server.js is called, and it calls to 'unconsume' with the socket's external stream.
My handler is called (as you can see in the attached code, I've registered to 'connect' event).
When I emit the 'connection' event with the received socket, the function 'connectionListener' in _http_server.js is called. It then executes the following line (currently, line 334 in _http_server.js): parser.consume(external);
This line raises the assertion.
If I understand correctly, what actually happens here is that some code consumes the stream, then unconsumes it, and finally consumes it again. Clearly no assertions should be raised, and the fact that an assertion is raised - probably indicates that some property wasn't cleared successfully from the stream.
I've opened a PR with a fix to this issue: #11015
After applying this fix, my application works as expected, with (apparently) no degradation at all.
Activity
evanlucas commentedon Mar 22, 2017
This appears to have been fixed by #11015 so I'm going to go ahead and close. Thanks!