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

[Patrick Barrett]

Yes, that would also fix it in this case, but as far as I can tell it
would still be technically broken, there should not be a second '?' in
the first page URL. I don't have access to test it while I'm at work,
but AFAIK if you get rid of the slash and change the '?' to '&' it
should also work and be properly formatted, I'm just not totally sure if
it's semantically correct.

-- 
  Patrick Barrett
  patrick at psbarrett.com

On Mon, Sep 11, 2017, at 09:42, Aline Manera wrote:
> 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 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(-)
> >>
> >
> > _______________________________________________
> > Kimchi-devel mailing list
> > Kimchi-devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/kimchi-devel
> >
>