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.

3 files changed, 43 insertions, 46 deletions

diff --git a/libsoup/soup-auth-negotiate.c b/libsoup/soup-auth-negotiate.c
index 94863d68..78c56b83 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 8362095e..f55cfe6b 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 f225f0f6..c5fbe7bd 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 */