http: don't write empty data on req/res end()#41116
Merged
nodejs-github-bot merged 1 commit intonodejs:masterfrom Dec 10, 2021
Merged
http: don't write empty data on req/res end()#41116nodejs-github-bot merged 1 commit intonodejs:masterfrom
nodejs-github-bot merged 1 commit intonodejs:masterfrom
Conversation
nodejs-github-bot
added
http
This comment has been minimized.
This comment has been minimized.
lpinca
reviewed
Dec 8, 2021
| } else if (!this._headerSent || this.outputSize || chunk) { | ||
| this._send('', 'latin1', finish); | ||
| } else { | ||
| process.nextTick(finish); |
Member
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.
Member
Author
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
Member
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 {
...
Member
Author
There was a problem hiding this comment.
That sounds good. We might even use OutgoingMessage.writableLength directly. Thanks!
Member
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.
lpinca
approved these changes
Dec 8, 2021
Flarna
approved these changes
Dec 8, 2021
This comment has been minimized.
This comment has been minimized.
46 tasks
This comment has been minimized.
This comment has been minimized.
Collaborator
48 tasks
jasnell
approved these changes
Dec 10, 2021
lpinca
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
Collaborator
|
Landed in ef7a686 |
This was referenced Dec 10, 2021
This was referenced Dec 11, 2021
danielleadams
pushed a commit
that referenced
this pull request
Dec 13, 2021
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 <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
46 tasks
danielleadams
pushed a commit
that referenced
this pull request
Dec 14, 2021
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 <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
24 tasks
This was referenced Dec 16, 2021
|
Hi, Do you know when it will be pushed to v14 LTS branch? We are affected by this too. Thank you =) |
danielleadams
pushed a commit
that referenced
this pull request
Jan 31, 2022
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 <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
danielleadams
pushed a commit
that referenced
this pull request
Jan 31, 2022
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 <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
Linkgoron
pushed a commit
to Linkgoron/node
that referenced
this pull request
Jan 31, 2022
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 <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
danielleadams
pushed a commit
that referenced
this pull request
Feb 1, 2022
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 <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When calling OutgoingMessage.end() with empty data argument, avoid
writing to the socket unless there's still pending data to be sent.
Fixes: #41062