[Kimchi-devel] [PATCH] [Kimchi 0/1] Fix VNC 1006: urlencode path

[Aline Manera]

Hi Patrick,

Thanks for this patch!

I could not reproduce the problem but I tested this patch and it does 
not break anything, ie, VNC continue working for me.

Could you check it solves the issue?

Regards,
Aline Manera

On 09/03/2017 11:44 AM, patrick at psbarrett.com wrote:
> From: Patrick Barrett <patrick at psbarrett.com>
>
> Currently kimchi directs you to
> `https://urldefense.proofpoint.com/v2/url?u=https-3A__hera.lan-3A8001_plugins_kimchi_novnc_vnc-5Fauto.html-3Fport-3D8001-26path-3D_websockify-3Ftoken-3D-5B...-5D-26encrypt-3D1&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=IH2IbOioVtBemYyjWtO2FV71Y6c8-CWLHAPDI5dc2gs&m=55perbMCcacetij4ysJl2fw3_svtUDOuvY1ooDsEu94&s=VTJ3PxuzW_QFrrxibrpU87drdUNAlQrSbpwXNlXMsZ4&e= `
> when you click on 'View Console`, however this is improperly encoded
> and Firefox guesses that the second `?` was meant to be an `&`. This
> means that `token` and `encrypt` 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
> wanted to just get this since it's simple, 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.)
>
> Patrick Barrett (1):
>    urlencode path
>
>   ui/js/src/kimchi.api.js | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
>