Re: [spice-gtk 06/13] http-proxy: add https proxy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 12, 2014 at 11:48 AM, Christophe Fergeau
<cfergeau@xxxxxxxxxx> wrote:
> On Tue, Feb 11, 2014 at 12:37:53PM -0500, Marc-André Lureau wrote:
>> > >  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))
>> >
>> > Having runtime type checks in a interface virtual method is not very nice,
>> > having a wocky_https_proxy_iface_init would allow to call directly the
>> > right implementation without having to resort to runtime type checking.
>>
>> I decided to use this consciously, as doing it differently added much
>> more boilerplate for no clear benefit.
>
> Gave it a quick try (only compile-tested), and the resulting code is not
> particularly bad. Having both inheritance and runtime type checks is imo
> quite ugly. I can send a follow-up patch after your series land if it's likely
> to be accepted.

Sure, I'll review then (the patch below looks incomplete from a brief
view). I still prefer a runtime type check for this final case.

>
>
> diff --git a/gtk/wocky-http-proxy.c b/gtk/wocky-http-proxy.c
> index 25f2af5..3eb6931 100644
> --- a/gtk/wocky-http-proxy.c
> +++ b/gtk/wocky-http-proxy.c
> @@ -186,30 +186,6 @@ wocky_http_proxy_connect (GProxy *proxy,
>        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;
> -    }
> -#endif
> -
>        in = g_io_stream_get_input_stream (io_stream);
>        out = g_io_stream_get_output_stream (io_stream);
>
> @@ -251,6 +227,41 @@ error:
>        return NULL;
>      }
>
> +#if GLIB_CHECK_VERSION(2, 28, 0)
> +static GIOStream *
> +wocky_https_proxy_connect (GProxy *proxy,
> +                           GIOStream *io_stream,
> +                           GProxyAddress *proxy_address,
> +                           GCancellable *cancellable,
> +                           GError **error)
> +{
> +    {
> +        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;
> +    }
> +    return wocky_http_proxy_connect(proxy, io_stream, proxy_address, cancellable, error);
> +
> +error:
> +    return NULL;
> +}
> +#endif
> +
>
>      typedef struct
>      {
> @@ -351,8 +362,9 @@ handshake_completed (GObject *source_object,
>      }
>  #endif
>
> -static void
> -wocky_http_proxy_connect_async (GProxy *proxy,
> +    static ConnectAsyncData *
> +    wocky_http_proxy_connect_async_data_new (
> +        GProxy *proxy,
>          GIOStream *io_stream,
>          GProxyAddress *proxy_address,
>          GCancellable *cancellable,
> @@ -364,7 +376,7 @@ wocky_http_proxy_connect_async (GProxy *proxy,
>
>        simple = g_simple_async_result_new (G_OBJECT (proxy),
>                                            callback, user_data,
> -                                      wocky_http_proxy_connect_async);
> +                                          wocky_http_proxy_connect_async_data_new);
>
>        data = g_slice_new0 (ConnectAsyncData);
>        if (cancellable != NULL)
> @@ -378,12 +390,30 @@ wocky_http_proxy_connect_async (GProxy *proxy,
>        g_simple_async_result_set_op_res_gpointer (simple, data,
>                                                   (GDestroyNotify) free_connect_data);
>
> +      return data;
> +    }
> +
> +
>  #if GLIB_CHECK_VERSION(2, 28, 0)
> -  if (WOCKY_IS_HTTPS_PROXY (proxy))
> +static void
> +wocky_https_proxy_connect_async (GProxy *proxy,
> +                                 GIOStream *io_stream,
> +                                 GProxyAddress *proxy_address,
> +                                 GCancellable *cancellable,
> +                                 GAsyncReadyCallback callback,
> +                                 gpointer user_data)
>  {
> +    ConnectAsyncData *data;
>      GError *error = NULL;
>      GIOStream *tlsconn;
>
> +    data = wocky_http_proxy_connect_async_data_new(proxy,
> +                                                   io_stream,
> +                                                   proxy_address,
> +                                                   cancellable,
> +                                                   callback,
> +                                                   user_data);
> +
>      tlsconn = g_tls_client_connection_new (io_stream,
>                                             G_SOCKET_CONNECTABLE(proxy_address),
>                                             &error);
> @@ -405,12 +435,26 @@ wocky_http_proxy_connect_async (GProxy *proxy,
>                                        G_PRIORITY_DEFAULT, cancellable,
>                                        handshake_completed, data);
>  }
> -  else
>  #endif
> +
> +static void
> +wocky_http_proxy_connect_async (GProxy *proxy,
> +                                GIOStream *io_stream,
> +                                GProxyAddress *proxy_address,
> +                                GCancellable *cancellable,
> +                                GAsyncReadyCallback callback,
> +                                gpointer user_data)
>  {
> +    ConnectAsyncData *data;
> +    data = wocky_http_proxy_connect_async_data_new(proxy,
> +                                                   io_stream,
> +                                                   proxy_address,
> +                                                   cancellable,
> +                                                   callback,
> +                                                   user_data);
> +
>      stream_connected(data, io_stream);
>  }
> -}
>
>      static void
>      request_write_cb (GObject *source,
> @@ -510,6 +554,16 @@ wocky_http_proxy_iface_init (GProxyInterface *proxy_iface)
>    proxy_iface->supports_hostname = wocky_http_proxy_supports_hostname;
>  }
>
> +static void
> +wocky_https_proxy_iface_init (GProxyInterface *proxy_iface)
> +{
> +  proxy_iface->connect  = wocky_https_proxy_connect;
> +  proxy_iface->connect_async = wocky_https_proxy_connect_async;
> +  proxy_iface->connect_finish = wocky_http_proxy_connect_finish;
> +  proxy_iface->supports_hostname = wocky_http_proxy_supports_hostname;
> +}
> +
> +
>  #if GLIB_CHECK_VERSION(2, 28, 0)
>  struct _WockyHttpsProxy
>  {
> @@ -524,7 +578,7 @@ struct _WockyHttpsProxyClass
>  #define wocky_https_proxy_get_type _wocky_https_proxy_get_type
>  G_DEFINE_TYPE_WITH_CODE (WockyHttpsProxy, wocky_https_proxy, WOCKY_TYPE_HTTP_PROXY,
>    G_IMPLEMENT_INTERFACE (G_TYPE_PROXY,
> -    wocky_http_proxy_iface_init)
> +    wocky_https_proxy_iface_init)
>    g_io_extension_point_set_required_type (
>      g_io_extension_point_register (G_PROXY_EXTENSION_POINT_NAME),
>      G_TYPE_PROXY);
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]