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

Issue with onshape.com websockets #27

Closed
kfreezen opened this issue Jan 31, 2018 · 6 comments
Closed

Issue with onshape.com websockets #27

kfreezen opened this issue Jan 31, 2018 · 6 comments
Labels

Comments

@kfreezen
Copy link

This seems to be a different issue than #26 as a 502 is not returned. The server closes the connection without warning when connecting via onshape.com.

If I take the websocket URL and paste it into the websocket echo test, it connects fine.

@TechnikEmpire you can contact @montesound if you want to test the behavior.

@kfreezen
Copy link
Author

kfreezen commented Feb 2, 2018

I've been delving into this today, and the conclusion I've come to is that the context.RequestAborted cancellation token is getting canceled, which means the request is getting aborted, right?

The request is getting aborted before the socket is even created, it looks like to me, but I have no idea why. I'll keep examining this to see if I can get anywhere.

@TechnikEmpire
Copy link
Owner

@kfreezen If that cancellation token is being flagged/set then it's being done by the internal processing of kestrel itself. I'd be curious to know why kestrel would be aborting the connection. Maybe timeout? I have not had a chance to look at this stuff yet because I've had a nagging problem in some server side code of mine draining all of my time this week. I'm going to crack into this stuff over the weekend though.

@kfreezen
Copy link
Author

kfreezen commented Feb 2, 2018

I've finally figured out where the problem lies, and it's not pretty.

In FilterWebsocketHandler.cs at line 64ish, we have

await context.WebSockets.AcceptWebSocketAsync();

cad.onshape.com sets a Sec-WebSocket-Protocol and expects a matching protocol header back from us. When no such header is returned, the browser aborts the connection, therefore causing the problem I was observing.

As far as I can tell, there is no way to force ASP.NET/Kestrel to return a matching protocol to the client browser on acceptance of the websocket connection.

The only fix I can come up with is adding a method to do what we want to ASP.NET Core's code, issuing a PR, and hoping it gets accepted.

@TechnikEmpire
Copy link
Owner

@kfreezen Yikes. Well, there's probably exposed mechanisms buried in kestrel where we could handle it. First thing I'd check is to see if it's possible to build a connection handler like I did for handling HTTPS. You can get hold of the connection before kestrel does anything to it through an exposed interface. Might be a solution there. Worst case scenario you could do some reflection based hacks I guess to patch it.

@TechnikEmpire
Copy link
Owner

@kfreezen Nope the solution is to reply with the server-selected subprocotol inline in the AcceptWebSocketAsync(... subproto here...) call.

What should be done though first is to establish the upstream client websocket connection, forward the Sec-WebSocket-Protocol header in that client connection, get the upstream server's selected protocol from the handshake result, and then pass that subprotocol to AcceptWebSocketAsync(...) to make sure we don't screw anything up acting as an invisible proxy.

@montesound
Copy link

montesound commented Feb 5, 2018

wss://websocket.us.fleetmatics.com is not loading on https://reveal.fleetmatics.com/report/

Also a report of avery.com not working.

kfreezen referenced this issue in cloudveiltech/FilterCore Feb 5, 2018
TechnikEmpire added a commit that referenced this issue Apr 14, 2018
Reorders websocket negotiation to get the negotiated subprotocol from the upstream websocket server before accepting the incoming client connection, in order to properly pass this on to the connected client and thus synchronize the subprotocol between upstream and downstream.

Possible fix but probably not quite there yet, untested.
Related to #27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants