<div dir="ltr"><div><div>Hi Galen,<br><br></div>I agree about relative URLs. I could try that and check it fixes the VNC problem but I can not reproduce the bug with upstream code.<br><br></div>About your patch, please, go ahead and send it here. I'd be glad to merge it upstream.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 11, 2017 at 12:09 PM, Galen Pospisil <span dir="ltr"><<a href="mailto:galen.m.pospisil@gmail.com" target="_blank">galen.m.pospisil@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><br></div><div>See: <a href="https://github.com/kimchi-project/kimchi/commit/b14fb611d3d79904694a14e67f28db8f9c8acf40" target="_blank">https://github.com/kimchi-<wbr>project/kimchi/commit/<wbr>b14fb611d3d79904694a14e67f28db<wbr>8f9c8acf40</a><div><br></div><div><br></div></div><div><div class="h5"><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 11, 2017 at 8:51 AM, Patrick Barrett <span dir="ltr"><<a href="mailto:patrick@psbarrett.com" target="_blank">patrick@psbarrett.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Yes, that would also fix it in this case, but as far as I can tell it<br>
would still be technically broken, there should not be a second '?' in<br>
the first page URL. I don't have access to test it while I'm at work,<br>
but AFAIK if you get rid of the slash and change the '?' to '&' it<br>
should also work and be properly formatted, I'm just not totally sure if<br>
it's semantically correct.<br>
<span class="m_5699627381032001600HOEnZb"><font color="#888888"><br>
--<br>
Patrick Barrett<br>
<a href="mailto:patrick@psbarrett.com" target="_blank">patrick@psbarrett.com</a><br>
</font></span><div class="m_5699627381032001600HOEnZb"><div class="m_5699627381032001600h5"><br>
On Mon, Sep 11, 2017, at 09:42, Aline Manera wrote:<br>
> Ops... I haven't tested serial console before and with this patch it is<br>
> broken.<br>
><br>
> Analyzing the code and your detailed explanation, don't we just need to<br>
> make the path relative instead of absolute?<br>
> So instead referring to '/websockify' it would be 'websockify'<br>
><br>
> On 09/11/2017 11:16 AM, Aline Manera wrote:<br>
> > Hi Patrick,<br>
> ><br>
> > Thanks for this patch!<br>
> ><br>
> > I could not reproduce the problem but I tested this patch and it does<br>
> > not break anything, ie, VNC continue working for me.<br>
> ><br>
> > Could you check it solves the issue?<br>
> ><br>
> > Regards,<br>
> > Aline Manera<br>
> ><br>
> > On 09/03/2017 11:44 AM, <a href="mailto:patrick@psbarrett.com" target="_blank">patrick@psbarrett.com</a> wrote:<br>
> >> From: Patrick Barrett <<a href="mailto:patrick@psbarrett.com" target="_blank">patrick@psbarrett.com</a>><br>
> >><br>
> >> Currently kimchi directs you to<br>
> >> `<a href="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=" rel="noreferrer" target="_blank">https://urldefense.proofpoint<wbr>.com/v2/url?u=https-3A__hera.<wbr>lan-3A8001_plugins_kimchi_<wbr>novnc_vnc-5Fauto.html-3Fport-<wbr>3D8001-26path-3D_websockify-<wbr>3Ftoken-3D-5B...-5D-26encrypt-<wbr>3D1&d=DwIBaQ&c=jf_iaSHvJObTbx-<wbr>siA1ZOg&r=IH2IbOioVtBemYyjWtO2<wbr>FV71Y6c8-CWLHAPDI5dc2gs&m=55pe<wbr>rbMCcacetij4ysJl2fw3_svtUDOuvY<wbr>1ooDsEu94&s=VTJ3PxuzW_QFrrxibr<wbr>pU87drdUNAlQrSbpwXNlXMsZ4&e=</a><br>
> >> `<br>
> >> when you click on 'View Console`, however this is improperly encoded<br>
> >> and Firefox guesses that the second `?` was meant to be an `&`. This<br>
> >> means that `token` and `encrypt` are sent to the current page rather<br>
> >> than being used for the websocket path.<br>
> >><br>
> >> It seems that noVNC should "work" with this, but there's another bug<br>
> >> (or kimchi is using path improperly, not sure, could be fixed in<br>
> >> either AFAIK) in noVNC where it assumes path is relative and prepends<br>
> >> a `/` to it, which then when it uses and anchor tag to reformat the<br>
> >> URL (I think) in `WebUtil.injectParamIfMissing`<wbr>. However when path<br>
> >> already has a leading `/` it interprets "websockify" as the host<br>
> >> (that is it becomes `//websockify/`) and the path becomes just<br>
> >> `/?token=[...]`.<br>
> >><br>
> >> That code path is only executed when token is part of the original<br>
> >> URL, so this commit urlencodes the path to prevent the<br>
> >> interpretation of the token in the original URL and use it only in<br>
> >> path. This could also be fixed by changing the `?` to an `&` and<br>
> >> removing the leading `/`. However, `encodeURIcomponent` should<br>
> >> probably be used regardless since parts of path can come from a<br>
> >> config file.<br>
> >><br>
> >> **Caveats**<br>
> >><br>
> >> This is **UNTESTED** because I don't know how to build it all and<br>
> >> wanted to just get this since it's simple, but I have manually<br>
> >> tested it with the URL I believe this should produce.<br>
> >><br>
> >> I wasn't totally sure about the encrypt param because it was just<br>
> >> another & param in the URL and there's nothing to indicate where if<br>
> >> it's part of path or part of the page url. However, it doesn't seem<br>
> >> to make a visible difference where it's put, or even if it is<br>
> >> removed. (This would also make a difference to the alternate<br>
> >> solution I suggested above.)<br>
> >><br>
> >> Patrick Barrett (1):<br>
> >> urlencode path<br>
> >><br>
> >> ui/js/src/kimchi.api.js | 24 ++++++++++++++----------<br>
> >> 1 file changed, 14 insertions(+), 10 deletions(-)<br>
> >><br>
> ><br>
> > ______________________________<wbr>_________________<br>
> > Kimchi-devel mailing list<br>
> > <a href="mailto:Kimchi-devel@ovirt.org" target="_blank">Kimchi-devel@ovirt.org</a><br>
> > <a href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/mailman<wbr>/listinfo/kimchi-devel</a><br>
> ><br>
><br>
______________________________<wbr>_________________<br>
Kimchi-devel mailing list<br>
<a href="mailto:Kimchi-devel@ovirt.org" target="_blank">Kimchi-devel@ovirt.org</a><br>
<a href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/mailman<wbr>/listinfo/kimchi-devel</a><br>
</div></div></blockquote></div><br></div></div></div></div>
<br>______________________________<wbr>_________________<br>
Kimchi-devel mailing list<br>
<a href="mailto:Kimchi-devel@ovirt.org">Kimchi-devel@ovirt.org</a><br>
<a href="http://lists.ovirt.org/mailman/listinfo/kimchi-devel" rel="noreferrer" target="_blank">http://lists.ovirt.org/<wbr>mailman/listinfo/kimchi-devel</a><br>
<br></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Aline Manera<br></div>
</div>