On Wed, Feb 12, 2014 at 11:59 AM, Christophe Fergeau
<cferg...@redhat.com> wrote:
> On Mon, Feb 03, 2014 at 07:02:37PM +0100, Marc-André Lureau wrote:
>> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>>
>> This will require glib 2.28 for GTls support, atm
>> ---
>> gtk/spice-session.c | 3 +
>> gtk/wocky-http-proxy.c | 166
>> +++++++++++++++++++++++++++++++++++++++++--------
>> gtk/wocky-http-proxy.h | 14 +++++
>> 3 files changed, 157 insertions(+), 26 deletions(-)
>>
>> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
>> index ae14a1f..6ac397c 100644
>> --- a/gtk/spice-session.c
>> +++ b/gtk/spice-session.c
>> @@ -636,6 +636,9 @@ static void spice_session_class_init(SpiceSessionClass
>> *klass)
>> #if GLIB_CHECK_VERSION(2, 26, 0)
>> _wocky_http_proxy_get_type();
>> #endif
>> +#if GLIB_CHECK_VERSION(2, 28, 0)
>> + _wocky_https_proxy_get_type();
>> +#endif
>>
>> gobject_class->dispose = spice_session_dispose;
>> gobject_class->finalize = spice_session_finalize;
>> diff --git a/gtk/wocky-http-proxy.c b/gtk/wocky-http-proxy.c
>> index 7210859..25f2af5 100644
>> --- a/gtk/wocky-http-proxy.c
>> +++ b/gtk/wocky-http-proxy.c
>> @@ -1,7 +1,9 @@
>> /* wocky-http-proxy.c: Source for WockyHttpProxy
>> *
>> * Copyright (C) 2010 Collabora, Ltd.
>> + * Copyright (C) 2014 Red Hat, Inc.
>> * @author Nicolas Dufresne <nicolas.dufre...@collabora.co.uk>
>> + * @author Marc-André Lureau <marcandre.lur...@redhat.com>
>> *
>> * This library is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU Lesser General Public
>> @@ -40,13 +42,13 @@ static void wocky_http_proxy_iface_init (GProxyInterface
>> *proxy_iface);
>>
>> #define wocky_http_proxy_get_type _wocky_http_proxy_get_type
>> G_DEFINE_TYPE_WITH_CODE (WockyHttpProxy, wocky_http_proxy, G_TYPE_OBJECT,
>> - G_IMPLEMENT_INTERFACE (G_TYPE_PROXY,
>> - wocky_http_proxy_iface_init)
>> - g_io_extension_point_set_required_type (
>> - g_io_extension_point_register (G_PROXY_EXTENSION_POINT_NAME),
>> - G_TYPE_PROXY);
>> - g_io_extension_point_implement (G_PROXY_EXTENSION_POINT_NAME,
>> - g_define_type_id, "http", 0))
>> + G_IMPLEMENT_INTERFACE (G_TYPE_PROXY,
>> + wocky_http_proxy_iface_init)
>> + g_io_extension_point_set_required_type (
>> + g_io_extension_point_register (G_PROXY_EXTENSION_POINT_NAME),
>> + G_TYPE_PROXY);
>> + g_io_extension_point_implement (G_PROXY_EXTENSION_POINT_NAME,
>> + g_define_type_id, "http", 0))
>>
>> static void
>> wocky_http_proxy_init (WockyHttpProxy *proxy)
>> @@ -180,10 +182,34 @@ wocky_http_proxy_connect (GProxy *proxy,
>> {
>> GInputStream *in;
>> GOutputStream *out;
>> - GDataInputStream *data_in;
>> - gchar *buffer;
>> + GDataInputStream *data_in = NULL;
>> + gchar *buffer = NULL;
>> gboolean has_cred;
>>
>> +#if GLIB_CHECK_VERSION(2, 28, 0)
>> + if (WOCKY_IS_HTTPS_PROXY (proxy))
>> + {
>> + GIOStream *tlsconn;
>> +
>> + tlsconn = g_tls_client_connection_new (io_stream,
>> +
>> G_SOCKET_CONNECTABLE(proxy_address),
>> + error);
>> + if (!tlsconn)
>> + goto error;
>> +
>> + GTlsCertificateFlags tls_validation_flags =
>> G_TLS_CERTIFICATE_VALIDATE_ALL;
>> +#ifdef DEBUG
>> + tls_validation_flags -= G_TLS_CERTIFICATE_UNKNOWN_CA +
>> G_TLS_CERTIFICATE_BAD_IDENTITY;
>> +#endif
>> + g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION
>> (tlsconn),
>> + tls_validation_flags);
>> + if (!g_tls_connection_handshake (G_TLS_CONNECTION (tlsconn),
>> cancellable, error))
>> + goto error;
>> +
>> + io_stream = tlsconn;
>
> Missed that initially, but tlsconn is leaked in error cases, and I think we
> will leak a reference to it in success case too. My understanding is that
if error, function returns NULL
> we don't own the io_stream passed as an argument, so we need to ref it in
> the http case, but in the https case, we already own a ref on the io_stream
> (which is tlsconn), so we ref it one too many.
Where do you see we ref something?
I remember I did check with valgrind and debugging than all references
where correct. But I could be wrong. Please give further details.
>
>> +
>> +static void
>> wocky_http_proxy_connect_async (GProxy *proxy,
>> GIOStream *io_stream,
>> GProxyAddress *proxy_address,
>> @@ -300,34 +361,55 @@ wocky_http_proxy_connect_async (GProxy *proxy,
>> +
>> +#if GLIB_CHECK_VERSION(2, 28, 0)
>> + if (WOCKY_IS_HTTPS_PROXY (proxy))
>> + {
>> + GError *error = NULL;
>> + GIOStream *tlsconn;
>> +
>> + tlsconn = g_tls_client_connection_new (io_stream,
>> +
>> G_SOCKET_CONNECTABLE(proxy_address),
>> + &error);
>> + if (error)
>> + {
>> + complete_async_from_error (data, error);
>
> tlsconn is leaked here (dunno if it's guaranteed to be NULL when error is
> set).
>
Yes it is guaratee, I can add a warning and g_clear_object if you want.
>> + return;
>> + }
>> +
>> + g_return_if_fail (tlsconn != NULL);
>> +
>> + GTlsCertificateFlags tls_validation_flags =
>> G_TLS_CERTIFICATE_VALIDATE_ALL;
>> +#ifdef DEBUG
>> + tls_validation_flags -= G_TLS_CERTIFICATE_UNKNOWN_CA +
>> G_TLS_CERTIFICATE_BAD_IDENTITY;
>> +#endif
>> + g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION
>> (tlsconn),
>> + tls_validation_flags);
>> + g_tls_connection_handshake_async (G_TLS_CONNECTION (tlsconn),
>> + G_PRIORITY_DEFAULT, cancellable,
>> + handshake_completed, data);
>
> A g_object_unref(tlsconn) is probably needed here.
>
no, the async is running and we need to keep the ref.
>
> Christophe
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
