Only end upgrade socket connections if unhandled #144
Merged
Conversation
This file contains 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
This fixes an issue where multiple engine.io instances (on different paths) would result in the closing of websocket connections for all of the instances. This happened because each engine.io instance would register an `upgrade` handler on the server. This handler would check for a matching path and otherwise call `socket.end()` Since multiple upgrade events would be triggered with different paths, the peer handlers would close each other. This patch resolves this behavior in the following way: - When an instance upgrade handler encounters a path which it does not recognize it creates a timeout for `destroyUpgradeTimeout`. - At the end of the timeout, the socket is checked for writable state and bytes written. If there has been not activity and the socket is writable, then it will be ended. This allows for peer socket handlers to keep the socket alive by sending some data over it. This also mimics the core node behavior of closing sockets on upgrade when no handler is specified. We consider not handling an upgrade request similar to no handler. However, we cannot immediately end the socket for the reasons noted above. fixes socketio#143
rauchg
added a commit
that referenced
this pull request
Feb 1, 2013
Only end upgrade socket connections if unhandled
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes an issue where multiple engine.io instances (on different
paths) would result in the closing of websocket connections for all of
the instances. This happened because each engine.io instance would
register an
upgradehandler on the server. This handler would checkfor a matching path and otherwise call
socket.end()Since multipleupgrade events would be triggered with different paths, the peer
handlers would close each other.
This patch resolves this behavior in the following way:
recognize it creates a timeout for
destroyUpgradeTimeout.and bytes written. If there has been not activity and the socket is
writable, then it will be ended.
This allows for peer socket handlers to keep the socket alive by sending
some data over it. This also mimics the core node behavior of closing
sockets on upgrade when no handler is specified. We consider not
handling an upgrade request similar to no handler. However, we cannot
immediately end the socket for the reasons noted above.
fixes #143