Fix namespace reconnect state, Engine.IO transport validation, and handshake metadata#28
Open
itsfuad wants to merge 3 commits intosocketio:mainfrom
Open
Fix namespace reconnect state, Engine.IO transport validation, and handshake metadata#28itsfuad wants to merge 3 commits intosocketio:mainfrom
itsfuad wants to merge 3 commits intosocketio:mainfrom
Conversation
Move OPTIONS response after editResponseHeaders so custom headers are included in preflight responses. Allow websocket upgrades from polling when an Upgrade header is present; reject other invalid transport transitions with TRANSPORT_MISMATCH. Refactor Socket.IO client handshake creation (createHandshakeBase), fix socket removal key, and update/add tests accordingly.
Contributor
Author
|
@darrachequesne Could you kindly review please? |
Contributor
Author
|
@darrachequesne Just a friendly ping. Didn't hear any feedback from you. |
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
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 PR fixes four confirmed protocol and metadata bugs across the Socket.IO and
Engine.IO layers:
the same namespace works on the same Engine.IO session
transport=websocketrequests for polling sessions unless the requestis a real WebSocket upgrade
editResponseHeadersto CORS preflight responses as documentedsecureandxdomainfrom the actual request URL andOriginheaderinstead of hardcoded values
How This Solves The Issues
Namespace reconnect
Clientstored namespace sockets by namespace name but removed them by socketid. This PR makes removal use
socket.nsp.name, so41/custom,actually clearsthe namespace entry and a later
40/custom,can reconnect cleanly.Transport mismatch validation
Engine.IO now distinguishes a valid polling-to-websocket upgrade request from an
ordinary HTTP request that merely carries
transport=websocket. Non-upgrademismatches are rejected with the existing
TRANSPORT_MISMATCHerror instead ofhanging on the polling transport.
Preflight response headers
editResponseHeadersnow runs before the earlyOPTIONSreturn path, so customresponse headers are applied consistently to preflight and non-preflight
requests.
Handshake metadata
Handshake metadata is now derived from the request URL and
Originheader:secureistrueforhttps:requestsxdomainis onlytruewhenOrigindiffers from the request originTests
Added regression coverage for:
41/custom,transport=websocketeditResponseHeadersonOPTIONSpreflight requestssecureandxdomainmetadata derivationFull test suite result on branch
fix/repro-report-and-protocol-bugs:Command used:
/home/fuad/.deno/bin/deno test -A