stuicey / SSHy

HTML5 SSH Web Client
MIT License
542 stars 83 forks source link

Support for binary websockets #38

Open candlerb opened 4 years ago

candlerb commented 4 years ago

According to README.md, SSHy is compatible with websockify when you use wrapper.html and insert the correct websocket endpoint.

However I found it doesn't work: SSHy requests Sec-WebSocket-Protocol: base64, but websockify has dropped base64 support and now only implements binary. It gives a 400 response. See here for a tcpdump of the exchange.

I think it would be useful for SSHy to support "binary" - or if not, at least to remove the reference to websockify from the documentation.

It looks like binary support might be straightforward. I found this patch: https://github.com/jonsito/labo_sphere/commit/1c3a2354eb591e74891b3b26d6386c72889697d7 However it applies to index.html, not to wrapper.html (which only uses the minified javascript).

candlerb commented 4 years ago

Also, if this were done, it could also mean that SSHy would be able to work with an out-of-the-box wsproxy instead of requiring the patched one supplied with SSHy. That would be a big bonus.

I notice that upstream wsproxy has now pinned the version of ws to 2.x, which means it doesn't need the patch in https://github.com/stuicey/wsProxy/commit/8ef26acda8da95b5be449e7e6071a7aad16774e4 (although it still might be a good idea to submit this upstream so it can move to ws 3.x)

stuicey commented 4 years ago

Yeah, its trivial to update this for use with websockify (see here), I'd be hesitant to make it part of the normal release though unless websockify or another major websocket proxy supports multiplexing? If theres a better suited alternative I don't see the harm in switching, all wsproxy does is bridge ws <-> tcp sockets.

wsproxy was developed for use with Ragnarök Online, so I wouldn't be surprised if they want to stay at ws 2.x to maintain compatability.

candlerb commented 4 years ago

Is ws (library) >= 3.x required for multiplexing then?

I don't know much about websockets, but as far as I can tell, the benefit from multiplexing is if you have lots of tabs open with ssh sessions, all going to the same backend server. Is that correct?

ISTM that binary websockets would be more widely compatible. You can still release an optimised version of wsproxy, using a newer version of ws library, but it would be optional.

Given that wsproxy is barely maintained it's probably worth forking wsproxy with a different name, so it's available via a simple npm i.