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

Ops... I haven't tested serial console before and with this patch it is broken. Analyzing the code and your detailed explanation, don't we just need to make the path relative instead of absolute? So instead referring to '/websockify' it would be 'websockify' On 09/11/2017 11:16 AM, Aline Manera wrote: > 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(a)psbarrett.com wrote: >> From: Patrick Barrett <patrick(a)psbarrett.com&gt; >> >> 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(-) >> > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >