[virt-tools-list] [virt-viewer] vnc: Error out when failing to parse URI in open_uri()

[Christophe Fergeau]

Hey,

On Sat, Dec 10, 2016 at 01:42:10PM +0100, Fabiano Fidêncio wrote:
> On Fri, Dec 9, 2016 at 4:48 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > Currently, if an invalid URI is passed to
> > virt_viewer_session_vnc_open_uri(), we will pass a NULL host/port to
> > vnc_display_open_host(), which will trigger a crash (see
> > https://bugzilla.gnome.org/show_bug.cgi?id=775890).
> >
> > This commit errors out before calling vnc_display_open_host() when we
> > failed to parse the hostname or the port from the URI.
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1378031
> > ---
> >  src/virt-viewer-session-vnc.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/virt-viewer-session-vnc.c b/src/virt-viewer-session-vnc.c
> > index 231f22f..5cd20af 100644
> > --- a/src/virt-viewer-session-vnc.c
> > +++ b/src/virt-viewer-session-vnc.c
> > @@ -215,7 +215,7 @@ virt_viewer_session_vnc_open_uri(VirtViewerSession* session,
> >      VirtViewerApp *app = virt_viewer_session_get_app(session);
> >      gchar *portstr;
> >      gchar *hoststr = NULL;
> > -    gboolean ret;
> > +    gboolean ret = FALSE;
> >
> >      g_return_val_if_fail(self != NULL, FALSE);
> >      g_return_val_if_fail(self->priv->vnc != NULL, FALSE);
> > @@ -250,9 +250,11 @@ virt_viewer_session_vnc_open_uri(VirtViewerSession* session,
> >          xmlFreeURI(uri);
> >      }
> >
> > -    ret = vnc_display_open_host(self->priv->vnc,
> > -                                hoststr,
> > -                                portstr);
> > +    if ((hoststr != NULL) && (portstr != NULL)) {
> > +        ret = vnc_display_open_host(self->priv->vnc,
> > +                                    hoststr,
> > +                                    portstr);
> > +    }
> >      g_free(portstr);
> >      g_free(hoststr);
> >      return ret;
> > --
> > 2.9.3
> >
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> Christophe,
> 
> Wouldn't worth setting the error to something meaningful as well?

Good point, though there is no GError setting in that function yet.
I'll send a followup patch.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20161213/8a126310/attachment.sig>