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

[Galen Pospisil]

AFAIK, relative would be a better way to do things all around.  I tried to
address this earlier, but I think the way I went about it was less than
ideal.  I don't know much about encoding. In any case, relative urls and
port numbers would work best, as even with a reverse proxy in front of
kimchi, the browser will get the correct port.

See:
https://github.com/kimchi-project/kimchi/commit/b14fb611d3d79904694a14e67f28db8f9c8acf40




On Mon, Sep 11, 2017 at 8:51 AM, Patrick Barrett <patrick at psbarrett.com>
wrote:

> 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
> > >
> >
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20170911/89cb61db/attachment.html>