http: don't write empty data on req/res end() #41116
Conversation
nodejs-github-bot
added
http
This comment has been minimized.
This comment has been minimized.
| this._send('', 'latin1', finish); | ||
| } else { | ||
| process.nextTick(finish); |
There was a problem hiding this comment.
The problem I see here is that the 'finish' event might be emitted before buffered data is flushed and similarly the outgoingMessage.end() callback might be called before previous outgoingMessage.write() callbacks.
There was a problem hiding this comment.
I understand, but I think at this point there should not buffered data and a consecutive call to end() should not reach here, or am I missing smthg? Thanks
There was a problem hiding this comment.
What do you think about adding a check for socket.writableLength to the above else if branch?
...
} else if (!this._headerSent || this.outputSize || chunk || this.socket?.writableLength) {
this._send('', 'latin1', finish);
} else {
...
There was a problem hiding this comment.
That sounds good. We might even use OutgoingMessage.writableLength directly. Thanks!
There was a problem hiding this comment.
@lpinca pushed the changes
There was a problem hiding this comment.
I think at this point there should not buffered data and a consecutive call to
end()should not reach here
I think you are right because the transfer encoding would be chunked but I'm not sure if there are cases where that path can be taken anyway. For example in #41062 the user specifies the Content-Length header so the chunked encoding is not used.
When calling OutgoingMessage.end() with empty data argument, avoid writing to the socket unless there's still pending data to be sent. Fixes: nodejs#41062
santigimeno
force-pushed
the
santi/fix_41062
branch
from
9946887
to
5697fc8
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
lpinca
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in ef7a686 |
When calling OutgoingMessage.end() with empty data argument, avoid writing to the socket unless there's still pending data to be sent. Fixes: #41062 PR-URL: #41116 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: James M Snell <jasnell@gmail.com>
When calling OutgoingMessage.end() with empty data argument, avoid writing to the socket unless there's still pending data to be sent. Fixes: #41062 PR-URL: #41116 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Hi, Do you know when it will be pushed to v14 LTS branch? We are affected by this too. Thank you =) |
When calling OutgoingMessage.end() with empty data argument, avoid writing to the socket unless there's still pending data to be sent. Fixes: #41062 PR-URL: #41116 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: James M Snell <jasnell@gmail.com>
When calling OutgoingMessage.end() with empty data argument, avoid writing to the socket unless there's still pending data to be sent. Fixes: #41062 PR-URL: #41116 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: James M Snell <jasnell@gmail.com>
When calling OutgoingMessage.end() with empty data argument, avoid writing to the socket unless there's still pending data to be sent. Fixes: nodejs#41062 PR-URL: nodejs#41116 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: James M Snell <jasnell@gmail.com>
When calling OutgoingMessage.end() with empty data argument, avoid writing to the socket unless there's still pending data to be sent. Fixes: #41062 PR-URL: #41116 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: James M Snell <jasnell@gmail.com>
When calling OutgoingMessage.end() with empty data argument, avoid
writing to the socket unless there's still pending data to be sent.
Fixes: #41062