On Tue, Dec 13, 2016 at 11:30 AM, Christophe Fergeau
<cfergeau at redhat.com> wrote:
> 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
Ack both patches!
Best Regards,
--
Fabiano Fidêncio