search

kimchi-project / kimchi

An HTML5 management interface for KVM guests
https://github.com/kimchi-project/kimchi/releases/latest
Other
3.09k stars 365 forks source link

Fix VNC 1006: urlencode path #1172

Closed azdle closed 7 years ago

azdle commented 7 years ago

Currently kimchi directs you to https://hera.lan:8001/plugins/kimchi/novnc/vnc_auto.html?port=8001&path=/websockify?token=[...]&encrypt=1 when you click on 'View Console, however this is improperly encoded and Firefox guesses that the second?was meant to be an&. This means thattokenandencrypt` are sent to the current page rather than being used for the websocket path.

It seems that noVNC should "work" with this, but there's another bug (or kimchi is using path improperly, not sure, could be fixed in either AFAIK) in noVNC where it assumes path is relative and prepends a / to it, which then when it uses and anchor tag to reformat the URL (I think) in WebUtil.injectParamIfMissing. However when path already has a leading / it interprets "websockify" as the host (that is it becomes //websockify/) and the path becomes just /?token=[...].

That code path is only executed when token is part of the original URL, so this commit urlencodes the path to prevent the interpretation of the token in the original URL and use it only in path. This could also be fixed by changing the ? to an & and removing the leading /. However, encodeURIcomponent should probably be used regardless since parts of path can come from a config file.

Caveats

This is UNTESTED because I don't know how to build it all and don't feel like figuring out right now, but I have manually tested it with the URL I believe this should produce.

I wasn't totally sure about the encrypt param because it was just another & param in the URL and there's nothing to indicate where if it's part of path or part of the page url. However, it doesn't seem to make a visible difference where it's put, or even if it is removed. (This would also make a difference to the alternate solution I suggested above.)

time-river commented 7 years ago

Your solution is pretty good. Mine is bad... here

azdle commented 7 years ago

I mean yours works. That's what I'm doing manually right now as a workaround.

azdle commented 7 years ago

Submitted patch to mailing list here: http://lists.ovirt.org/pipermail/kimchi-devel/2017-September/017865.html

jaydrogers commented 6 years ago

I am getting the same error. I tried switching /websockify to websockify but I still get this error:

screen shot 2018-07-21 at 7 56 20 pm

My Environment

Method of how I installed Kimchi

I followed the instructions on the release page (https://github.com/kimchi-project/kimchi/releases/tag/2.5.0) to install through dpkg.

One thing I noticed is that the 2.5.0 release came out a few weeks before this issue was reported.

Any known workarounds on how to fix this on a fresh install?

Woovie commented 6 years ago

Based on the URL provided in your screenshot, it's still doing /websockify.