-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Support taking over an existing connection #335
Comments
This is something I looked at supporting with a newer NSURLSession API: https://developer.apple.com/reference/foundation/nsurlsessionstreamtask I unfortunately couldn't get it working though. It was something about not being able to customize the HTTP headers with the NSURLSession (it always send a connection: keep-alive instead of the required upgrade header). I haven't looked at it in a good while though, so might be worth be revisiting. That that being said, the other option would be to pass in streams as you have mentioned in a new connect method would also be a feasible way to get this working. I would love a pull request. Ideally we could get the nsurlsessionstreamtask working, but if not, a connect function taking the read and write streams would probably be our second best option. |
From my experience with NSURLSession (as used in Paw) we were unable to control the connection header yes. I was thinking about separating the connection/stream logic out to another protocol then this could be switched in and out, default to the current system of course, but also support the injection of other stream systems. This system would be responsible for setting up the connection and possibly also send the Upgrade request. I have noticed some services return important info the Upgrade response headers.. even through this breaks the spec so letting users swap in a custom class that handles this and then calls a method on the root WebSocket class to hand over control to the parent class. In my case a lot of the networking is based on top of https://github.com/IBM-Swift/BlueSocket so would be cool to have a protocol to comply with that would let me link this socket lib into Starscream.. would also result in a little bit of separation of the WebSocket logic from the initial HTTP upgrade request. |
@daltoniam Working on this im splitting out the newtorking from the Websocket parsing quite a bit it will be a rather large code change will that be ok. So currently I have split it out into 4 bits:
The other benefit of this is it lets us start to write unit tests for different bits, eg the WebScoket frame passing can be tested without mocking a webserver or anything much more simpler and we can just have a mocking socket connection for example for testing how the socket protocols talks with the rest of it. For my work, it's really important that I have an expandable socket lib so I'm lucky in that I can spend a fair bit of time on this maintaining it and pushing new features. |
I think this is a great idea. I believe the least flexible part of Starscream is the initial handshake/upgrade sequence. I think having a system to work with a "hijacked" connection makes a lot of sense. Thanks a lot for taking this on! |
I think with the new design of |
@daltoniam I was using your library for a significant period (thank you very much), and you mentioned that you tried to implement WebSocket connection using NSURLSessionStreamTask. Could you please share an example if you have it? It would be very helpful for me to start with |
I have a situation where a given connection (due to strange corporate auth that is connection based) needs a few HTTP requests first before the upgrade to WS.
Would be great is if I could pass a connection to Starscream once all the setup requests on the connection have been made.
maybe a dedicated connect function that takes the existing read and writes streams. Or even better takes an NSURLsession and extracts the read and write streams from it if that is possible?
I'm happy to put together a pull request just wanted to know if this is the type of feature you would want to add?
The text was updated successfully, but these errors were encountered: