HeinrichApfelmus / threepenny-gui

GUI framework that uses the web browser as a display.
https://heinrichapfelmus.github.io/threepenny-gui/
Other
437 stars 77 forks source link

added check for https:// to decide whether to use ssl-encrypted websockets #252

Closed dailyinvention closed 3 years ago

dailyinvention commented 3 years ago

Hello,

I hope you receive this well. The Cardano RTview web interface is using your library threepenny-gui library for their UI and I discovered that some code in your javascript doesn't account for connecting to websockets over SSL. I added a fix to the javascript to account for if it's being loaded over SSL or not.

Here's the initial issue this fixes: https://github.com/input-output-hk/cardano-rt-view/issues/124

HeinrichApfelmus commented 3 years ago

Thanks for reporting!

I have pushed a commit that fixes this issue in a slightly different way by removing my inelegant application of .slice(5).

That said, the webserver that is included in Threepenny does not speak SSL by default, I think. This may still work if it runs behind another webserver and both https:// and wss:// are forwarded to the respective http:// and ws:// ports. Please keep me posted if problems persist.

dailyinvention commented 3 years ago

Thanks for reporting!

I have pushed a commit that fixes this issue in a slightly different way by removing my inelegant application of .slice(5).

That said, the webserver that is included in Threepenny does not speak SSL by default, I think. This may still work if it runs behind another webserver and both https:// and wss:// are forwarded to the respective http:// and ws:// ports. Please keep me posted if problems persist.

Thank you much!

dailyinvention commented 3 years ago

Owner fixed the issue. Closing.

dailyinvention commented 3 years ago

Thanks for reporting!

I have pushed a commit that fixes this issue in a slightly different way by removing my inelegant application of .slice(5).

That said, the webserver that is included in Threepenny does not speak SSL by default, I think. This may still work if it runs behind another webserver and both https:// and wss:// are forwarded to the respective http:// and ws:// ports. Please keep me posted if problems persist.

The threepenny-gui is being used behind a reverse proxy in my case, so I'm passing the requests via SSL. I'm interested in using Cardano's RTview, which uses your library, and needed this fix for it to work. I understand you weren't originally testing it with SSL.

dailyinvention commented 3 years ago

@HeinrichApfelmus , sorry to bother you again. I tried your changes out on my server it was giving me an error again with the url. The url was returning as ws://xxx.xxxxxxxx.xxx//websocket/. There was an extra slash in the URL that was causing this problem. It needs to be "ws://xxx.xxxxxxxx.xxx/websocket/".

I think line 21 needs to be:

url.href = url.href + 'websocket/';

After implementing that change everything seems to work for me.

HeinrichApfelmus commented 3 years ago

You're right. Sorry about the mistake. Reminder to self: "First test, then push".