[libsoup] Rework some of the SoupAuthNegotiate internals

[Tomas Popela <tpopela src gnome org>]

commit 05a88c3a8e961e1394b9ccd269f03c7230004498
Author: Tomas Popela <tpopela redhat com>
Date:   Tue Jun 20 13:03:59 2017 +0200

    Rework some of the SoupAuthNegotiate internals
    
    There are several problems with the current state. The main problem is
    that the SoupMessage can outlive the SoupAuthNegotiate object and if the
    SoupMessage signals handlers are active then they could be called with invalid
    SoupAuthNegotiate object. To avoid that use the g_signal_connect_data() and
    increase the reference on the SoupAuthNegotiate object. Also rework how
    we are connecting the 'got_headers' signal handler to the SoupMessage object, so
    they are really connected only once, even if the GSS mechanism involves
    multiple rounds.
    
    The whole concept of how we are working with the
    SoupAuthNegotiateConnectionState is also wrong. When the connection state is
    created it's saved to the private structure and then accessed from
    there. The problem is that another state for different message could be
    created in the mean time and that one would overwrite the currently set (or if
    one would be freed then it would erase the one that is currently set). To solve
    this expose the SoupConnectionAuth's get_connection_state_for_message() and
    call it when we need the connection state object.

 libsoup/soup-auth-negotiate.c  |   56 ++++++++++-----------------------------
 libsoup/soup-connection-auth.c |   29 +++++++++++++++++---
 libsoup/soup-connection-auth.h |    4 +++
 3 files changed, 43 insertions(+), 46 deletions(-)
---
diff --git a/libsoup/soup-auth-negotiate.c b/libsoup/soup-auth-negotiate.c
index 94863d6..78c56b8 100644
--- a/libsoup/soup-auth-negotiate.c
+++ b/libsoup/soup-auth-negotiate.c
@@ -68,11 +68,6 @@ typedef struct {
 
 typedef struct {
        gboolean is_authenticated;
-
-       gulong message_finished_signal_id;
-       gulong message_got_headers_signal_id;
-
-       SoupNegotiateConnectionState *conn_state;
 } SoupAuthNegotiatePrivate;
 
 /**
@@ -108,7 +103,6 @@ static GSList *blacklisted_uris;
 static void parse_uris_from_env_variable (const gchar *env_variable, GSList **list);
 
 static void check_server_response (SoupMessage *msg, gpointer auth);
-static void remove_server_response_handler (SoupMessage *msg, gpointer auth);
 
 static const char spnego_OID[] = "\x2b\x06\x01\x05\x05\x02";
 static const gss_OID_desc gss_mech_spnego = { sizeof (spnego_OID) - 1, (void *) &spnego_OID };
@@ -116,12 +110,10 @@ static const gss_OID_desc gss_mech_spnego = { sizeof (spnego_OID) - 1, (void *)
 static gpointer
 soup_auth_negotiate_create_connection_state (SoupConnectionAuth *auth)
 {
-       SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (SOUP_AUTH_NEGOTIATE 
(auth));
        SoupNegotiateConnectionState *conn;
 
        conn = g_slice_new0 (SoupNegotiateConnectionState);
        conn->state = SOUP_NEGOTIATE_NEW;
-       priv->conn_state = conn;
 
        return conn;
 }
@@ -137,14 +129,11 @@ static void
 soup_auth_negotiate_free_connection_state (SoupConnectionAuth *auth,
                                           gpointer state)
 {
-       SoupAuthNegotiate *negotiate = SOUP_AUTH_NEGOTIATE (auth);
-       SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (negotiate);
        SoupNegotiateConnectionState *conn = state;
 
        free_connection_state_data (conn);
 
        g_slice_free (SoupNegotiateConnectionState, conn);
-       priv->conn_state = NULL;
 }
 
 static GSList *
@@ -226,7 +215,6 @@ soup_auth_negotiate_update_connection (SoupConnectionAuth *auth, SoupMessage *ms
 #ifdef LIBSOUP_HAVE_GSSAPI
        gboolean success = TRUE;
        SoupNegotiateConnectionState *conn = state;
-       SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (SOUP_AUTH_NEGOTIATE 
(auth));
        GError *err = NULL;
 
        if (!check_auth_trusted_uri (auth, msg)) {
@@ -245,24 +233,19 @@ soup_auth_negotiate_update_connection (SoupConnectionAuth *auth, SoupMessage *ms
 
                conn->state = SOUP_NEGOTIATE_RECEIVED_CHALLENGE;
                if (soup_gss_build_response (conn, SOUP_AUTH (auth), &err)) {
-                       /* Register the callbacks just once */
-                       if (priv->message_finished_signal_id == 0) {
-                               gulong id = 0;
-                               id = g_signal_connect (msg,
-                                                      "finished",
-                                                      G_CALLBACK (remove_server_response_handler),
-                                                      auth);
-                               priv->message_finished_signal_id = id;
-                       }
-
-                       if (priv->message_got_headers_signal_id == 0) {
-                               gulong id = 0;
+                       /* Connect the signal only once per message */
+                       if (!g_object_get_data (G_OBJECT (msg), "negotiate-got-headers-connected")) {
                                /* Wait for the 2xx response to verify server response */
-                               id = g_signal_connect (msg,
+                               g_signal_connect_data (msg,
                                                       "got_headers",
                                                       G_CALLBACK (check_server_response),
-                                                      auth);
-                               priv->message_got_headers_signal_id = id;
+                                                      g_object_ref (auth),
+                                                      (GClosureNotify) g_object_unref,
+                                                      0);
+                               /* Mark that the signal was connected */
+                               g_object_set_data (G_OBJECT (msg),
+                                                  "negotiate-got-headers-connected",
+                                                  GINT_TO_POINTER (1));
                        }
                        goto out;
                } else {
@@ -331,7 +314,11 @@ check_server_response (SoupMessage *msg, gpointer auth)
        GError *err = NULL;
        SoupAuthNegotiate *negotiate = auth;
        SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (negotiate);
-       SoupNegotiateConnectionState *conn = priv->conn_state;
+       SoupNegotiateConnectionState *conn;
+
+       conn = soup_connection_auth_get_connection_state_for_message (SOUP_CONNECTION_AUTH (auth), msg);
+       if (!conn)
+               return;
 
        if (auth != soup_message_get_auth (msg))
                return;
@@ -363,19 +350,6 @@ check_server_response (SoupMessage *msg, gpointer auth)
        g_clear_error (&err);
 }
 
-static void
-remove_server_response_handler (SoupMessage *msg, gpointer auth)
-{
-       SoupAuthNegotiate *negotiate = auth;
-       SoupAuthNegotiatePrivate *priv = soup_auth_negotiate_get_instance_private (negotiate);
-
-       g_signal_handler_disconnect (msg, priv->message_got_headers_signal_id);
-       priv->message_got_headers_signal_id = 0;
-
-       g_signal_handler_disconnect (msg, priv->message_finished_signal_id);
-       priv->message_finished_signal_id = 0;
-}
-
 /* Check if scheme://host:port from message matches the given URI. */
 static gint
 match_base_uri (SoupURI *list_uri, SoupURI *msg_uri)
diff --git a/libsoup/soup-connection-auth.c b/libsoup/soup-connection-auth.c
index 8362095..f55cfe6 100644
--- a/libsoup/soup-connection-auth.c
+++ b/libsoup/soup-connection-auth.c
@@ -71,12 +71,31 @@ soup_connection_auth_finalize (GObject *object)
        G_OBJECT_CLASS (soup_connection_auth_parent_class)->finalize (object);
 }
 
-static gpointer
-get_connection_state_for_message (SoupConnectionAuth *auth, SoupMessage *msg)
+
+/**
+ * soup_connection_auth_get_connection_state_for_message:
+ * @auth: a #SoupConnectionAuth
+ * @msg: a #SoupMessage
+ *
+ * Returns an associated connection state object for the given @auth and @msg.
+ *
+ * This function is only useful from within implementations of SoupConnectionAuth
+ * subclasses.
+ *
+ * Return value: (transfer none): the connection state
+ *
+ * Since: 2.58
+ **/
+gpointer
+soup_connection_auth_get_connection_state_for_message (SoupConnectionAuth *auth,
+                                                      SoupMessage *msg)
 {
        SoupConnection *conn;
        gpointer state;
 
+       g_return_val_if_fail (SOUP_IS_CONNECTION_AUTH (auth), NULL);
+       g_return_val_if_fail (SOUP_IS_MESSAGE (msg), NULL);
+
        conn = soup_message_get_connection (msg);
        state = g_hash_table_lookup (auth->priv->conns, conn);
        if (state)
@@ -98,7 +117,7 @@ soup_connection_auth_update (SoupAuth    *auth,
                             GHashTable  *auth_params)
 {
        SoupConnectionAuth *cauth = SOUP_CONNECTION_AUTH (auth);
-       gpointer conn = get_connection_state_for_message (cauth, msg);
+       gpointer conn = soup_connection_auth_get_connection_state_for_message (cauth, msg);
        GHashTableIter iter;
        GString *auth_header;
        gpointer key, value;
@@ -140,7 +159,7 @@ soup_connection_auth_get_authorization (SoupAuth    *auth,
                                        SoupMessage *msg)
 {
        SoupConnectionAuth *cauth = SOUP_CONNECTION_AUTH (auth);
-       gpointer conn = get_connection_state_for_message (cauth, msg);
+       gpointer conn = soup_connection_auth_get_connection_state_for_message (cauth, msg);
 
        return SOUP_CONNECTION_AUTH_GET_CLASS (auth)->
                get_connection_authorization (cauth, msg, conn);
@@ -151,7 +170,7 @@ soup_connection_auth_is_ready (SoupAuth    *auth,
                               SoupMessage *msg)
 {
        SoupConnectionAuth *cauth = SOUP_CONNECTION_AUTH (auth);
-       gpointer conn = get_connection_state_for_message (cauth, msg);
+       gpointer conn = soup_connection_auth_get_connection_state_for_message (cauth, msg);
 
        return SOUP_CONNECTION_AUTH_GET_CLASS (auth)->
                is_connection_ready (SOUP_CONNECTION_AUTH (auth), msg, conn);
diff --git a/libsoup/soup-connection-auth.h b/libsoup/soup-connection-auth.h
index f225f0f..c5fbe7b 100644
--- a/libsoup/soup-connection-auth.h
+++ b/libsoup/soup-connection-auth.h
@@ -46,6 +46,10 @@ typedef struct {
 
 GType soup_connection_auth_get_type (void);
 
+SOUP_AVAILABLE_IN_2_58
+gpointer       soup_connection_auth_get_connection_state_for_message
+                                               (SoupConnectionAuth *auth,
+                                                SoupMessage *message);
 G_END_DECLS
 
 #endif /* SOUP_CONNECTION_AUTH_H */