-
Notifications
You must be signed in to change notification settings - Fork 387
Connecting with query parameters #24
Comments
Hi @peenuty ! Thanks for pointing this out. socket.io doesn't handle query strings very well. The problem about your approach is, that an IOConnection can be multiplexed. That means, different SocketIO instances can use the same IOConnection. If you send different query strings over a multiplexed connection, only the first one gets sent to the server. Actually, this buggy approach is the way it is done in @LearnBoost socket.io implementation. So, I consider following this. But I suspect that this leads to nasty bugs. Enno PS: There's a bugreport for this issue with an apt title: socketio/socket.io-client#331 |
The issue is still open, but it's a protocol issue, not a socket.io-java-client specific one. I'm hoping for protocol revision 1.0 to fix this issue. For now, query strings are not supported by socket.io-java-client. |
socket.io support connect with querystring since 0.9.6, socketio/socket.io-client#467 |
This would be really nice. Other than this single feature everything is working great with this client. |
any news? |
I have pushed the query string integration in my fork but I cannot send pull-request. here is my commit: fatshotty@bf08624#commitcomment-3446681 |
version 0.2.1 has been deployed on maven repo. |
Hello fatshotty, |
Hi,
I had problems using your api when the url I needed to connect to had ?key=val
I am currently just trying to get a project working in my spare time so I have take your source into my project and fixed it. But my fix is pretty crude and might not working all cases.
The crux of the fix is in the handshake() method in IOConnection, to do something like this:
When you set the url field in IOConnection you only set the origin (there might be a good reason for this...!). Instead I changed it to set the full URL and then wrote a getOrigin method which I called in the right places.
This seems to work, apologies if I have misunderstood your api, thanks for writing it though :)
Rich
The text was updated successfully, but these errors were encountered: