Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send a socket id different from the engine socket id for each namespace #117

Open
Totodore opened this issue Oct 19, 2023 · 1 comment · Fixed by #123
Open

Send a socket id different from the engine socket id for each namespace #117

Totodore opened this issue Oct 19, 2023 · 1 comment · Fixed by #123
Labels
refactoring This reference a need for a refactoring socketio-v5 It is only about the version 5 of the socket.io protocol vulnerability This reference a vulnerability found on socketioxide or engineioxide

Comments

@Totodore
Copy link
Owner

Totodore commented Oct 19, 2023

Currently the socket id that reference a socket.io socket (one for each namespace) is the same than the engine socket id.

The implementation should match the socket.io server in node which create a new socket id for each new socket connected to a namespace (only for the socket.io v5 protocol) :

https://github.com/socketio/socket.io/blob/9a2a83fdd42faa840d4f11fd223349e5d8e4d52c/lib/socket.ts#L277

@Totodore Totodore added this to Roadmap Oct 17, 2023
@Totodore Totodore converted this from a draft issue Oct 19, 2023
@Totodore Totodore added bug Something isn't working socketio-v5 It is only about the version 5 of the socket.io protocol labels Oct 19, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Roadmap Oct 21, 2023
@Totodore Totodore reopened this Oct 25, 2023
@Totodore
Copy link
Owner Author

Totodore commented Oct 25, 2023

It appears that solving this issue implies a big refactoring. Indeed referencing socketio socket with custom sid instead is impossible because of the following architecture :
client recv data from engine socket -> route to corresponding namespace -> route to corresponding socket with <Engine id -> socket> hashmap

Because the socket id is not in the request it is impossible to map socket by their socket id (and not their engine id).
The solution would be to refactor the code to match the official architecture :
client wraps an engine socket and holds a map of the ns -> socket.

Or alternatively to keep the current architecture we could store a hashmap containing all the namespace for a given socket in the SocketData struct

@Totodore Totodore added the refactoring This reference a need for a refactoring label Oct 25, 2023
@Totodore Totodore moved this from Done to Todo in Roadmap Oct 30, 2023
@Totodore Totodore added vulnerability This reference a vulnerability found on socketioxide or engineioxide and removed bug Something isn't working labels Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring This reference a need for a refactoring socketio-v5 It is only about the version 5 of the socket.io protocol vulnerability This reference a vulnerability found on socketioxide or engineioxide
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

1 participant