GNOME.org
 

libsoup r1110 - in trunk: . libsoup tests

Author: danw
Date: Sun Mar 16 02:28:36 2008
New Revision: 1110
URL: http://svn.gnome.org/viewvc/libsoup?rev=1110&view=rev

Log:
        * libsoup/soup-session.c: Define two new signals, request_queued
        and request_unqueued, to provided a clearer (and
        clearly-documented) lifecycle for messages, helping us (and other
        people) avoid bugs like #522601, SoupSession::authenticate signal
        emitted multiple times per message (reported and analyzed by Tommi
        Komulainen).

        * libsoup/soup-logger.c:
        * libsoup/soup-auth-manager.c:
        * libsoup/soup-auth-manager-ntlm.c: Use request_queued/unqueued

        * tests/auth-test.c (do_async_auth_test): add a regression test


Modified:
   trunk/ChangeLog
   trunk/libsoup/soup-auth-manager-ntlm.c
   trunk/libsoup/soup-auth-manager.c
   trunk/libsoup/soup-logger.c
   trunk/libsoup/soup-session.c
   trunk/tests/auth-test.c

Modified: trunk/libsoup/soup-auth-manager-ntlm.c
==============================================================================
--- trunk/libsoup/soup-auth-manager-ntlm.c	(original)
+++ trunk/libsoup/soup-auth-manager-ntlm.c	Sun Mar 16 02:28:36 2008
@@ -45,8 +45,12 @@
 	GHashTable *connections_by_id;
 };
 
+static void ntlm_request_queued (SoupSession *session, SoupMessage *msg,
+				 gpointer ntlm);
 static void ntlm_request_started (SoupSession *session, SoupMessage *msg,
 				  SoupSocket *socket, gpointer ntlm);
+static void ntlm_request_unqueued (SoupSession *session, SoupMessage *msg,
+				   gpointer ntlm);
 
 static char     *soup_ntlm_request         (void);
 static gboolean  soup_ntlm_parse_challenge (const char  *challenge,
@@ -67,8 +71,12 @@
 	ntlm->session = session;
 	ntlm->connections_by_id = g_hash_table_new (NULL, NULL);
 	ntlm->connections_by_msg = g_hash_table_new (NULL, NULL);
+	g_signal_connect (session, "request_queued",
+			  G_CALLBACK (ntlm_request_queued), ntlm);
 	g_signal_connect (session, "request_started",
 			  G_CALLBACK (ntlm_request_started), ntlm);
+	g_signal_connect (session, "request_unqueued",
+			  G_CALLBACK (ntlm_request_unqueued), ntlm);
 	return ntlm;
 }
 
@@ -97,7 +105,11 @@
 	g_hash_table_destroy (ntlm->connections_by_id);
 	g_hash_table_destroy (ntlm->connections_by_msg);
 	g_signal_handlers_disconnect_by_func (ntlm->session,
+					      ntlm_request_queued, ntlm);
+	g_signal_handlers_disconnect_by_func (ntlm->session,
 					      ntlm_request_started, ntlm);
+	g_signal_handlers_disconnect_by_func (ntlm->session,
+					      ntlm_request_unqueued, ntlm);
 
 	g_slice_free (SoupAuthManagerNTLM, ntlm);
 }
@@ -253,13 +265,16 @@
 }
 
 static void
-ntlm_cleanup_msg (SoupMessage *msg, gpointer ntlm)
+ntlm_request_queued (SoupSession *session, SoupMessage *msg, gpointer ntlm)
 {
-	/* Do this when the message is restarted, in case it's
-	 * restarted on a different connection.
-	 */
-	g_signal_handlers_disconnect_by_func (msg, ntlm_authorize_pre, ntlm);
-	g_signal_handlers_disconnect_by_func (msg, ntlm_authorize_post, ntlm);
+	soup_message_add_status_code_handler (msg, "got_headers",
+					      SOUP_STATUS_UNAUTHORIZED,
+					      G_CALLBACK (ntlm_authorize_pre),
+					      ntlm);
+	soup_message_add_status_code_handler (msg, "got_body",
+					      SOUP_STATUS_UNAUTHORIZED,
+					      G_CALLBACK (ntlm_authorize_post),
+					      ntlm);
 }
 
 static void
@@ -292,20 +307,14 @@
 					      "Authorization", header);
 		g_free (header);
 	}
+}
 
-	soup_message_add_status_code_handler (msg, "got_headers",
-					      SOUP_STATUS_UNAUTHORIZED,
-					      G_CALLBACK (ntlm_authorize_pre),
-					      ntlm);
-	soup_message_add_status_code_handler (msg, "got_body",
-					      SOUP_STATUS_UNAUTHORIZED,
-					      G_CALLBACK (ntlm_authorize_post),
-					      ntlm);
-	g_signal_connect (msg, "restarted",
-			  G_CALLBACK (ntlm_cleanup_msg), ntlm);
-	g_signal_connect (msg, "finished",
-			  G_CALLBACK (ntlm_cleanup_msg), ntlm);
-
+static void
+ntlm_request_unqueued (SoupSession *session, SoupMessage *msg,
+		       gpointer ntlm)
+{
+	g_signal_handlers_disconnect_by_func (msg, ntlm_authorize_pre, ntlm);
+	g_signal_handlers_disconnect_by_func (msg, ntlm_authorize_post, ntlm);
 }
 
 

Modified: trunk/libsoup/soup-auth-manager.c
==============================================================================
--- trunk/libsoup/soup-auth-manager.c	(original)
+++ trunk/libsoup/soup-auth-manager.c	Sun Mar 16 02:28:36 2008
@@ -19,8 +19,12 @@
 #include "soup-session-private.h"
 #include "soup-uri.h"
 
+static void session_request_queued (SoupSession *session, SoupMessage *msg,
+				    gpointer data);
 static void session_request_started (SoupSession *session, SoupMessage *msg,
 				     SoupSocket *socket, gpointer data);
+static void session_request_unqueued (SoupSession *session, SoupMessage *msg,
+				      gpointer data);
 
 struct SoupAuthManager {
 	SoupSession *session;
@@ -53,8 +57,12 @@
 	manager->auth_hosts = g_hash_table_new (soup_uri_host_hash,
 						soup_uri_host_equal);
 
+	g_signal_connect (session, "request_queued",
+			  G_CALLBACK (session_request_queued), manager);
 	g_signal_connect (session, "request_started",
 			  G_CALLBACK (session_request_started), manager);
+	g_signal_connect (session, "request_unqueued",
+			  G_CALLBACK (session_request_unqueued), manager);
 	return manager;
 }
 
@@ -81,7 +89,13 @@
 
 	g_signal_handlers_disconnect_by_func (
 		manager->session,
+		G_CALLBACK (session_request_queued), manager);
+	g_signal_handlers_disconnect_by_func (
+		manager->session,
 		G_CALLBACK (session_request_started), manager);
+	g_signal_handlers_disconnect_by_func (
+		manager->session,
+		G_CALLBACK (session_request_unqueued), manager);
 
 	for (i = 0; i < manager->auth_types->len; i++)
 		g_type_class_unref (manager->auth_types->pdata[i]);
@@ -431,16 +445,11 @@
 }
 
 static void
-session_request_started (SoupSession *session, SoupMessage *msg,
-			 SoupSocket *socket, gpointer data)
+session_request_queued (SoupSession *session, SoupMessage *msg,
+			gpointer data)
 {
 	SoupAuthManager *manager = data;
-	SoupAuth *auth;
 
-	auth = lookup_auth (manager, msg);
-	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, FALSE))
-		auth = NULL;
-	soup_message_set_auth (msg, auth);
 	soup_message_add_status_code_handler (
 		msg, "got_headers", SOUP_STATUS_UNAUTHORIZED,
 		G_CALLBACK (update_auth), manager);
@@ -448,10 +457,6 @@
 		msg, "got_body", SOUP_STATUS_UNAUTHORIZED,
 		G_CALLBACK (requeue_if_authenticated), manager);
 
-	auth = manager->proxy_auth;
-	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, TRUE))
-		auth = NULL;
-	soup_message_set_proxy_auth (msg, auth);
 	soup_message_add_status_code_handler (
 		msg, "got_headers", SOUP_STATUS_PROXY_UNAUTHORIZED,
 		G_CALLBACK (update_proxy_auth), manager);
@@ -459,3 +464,31 @@
 		msg, "got_body", SOUP_STATUS_PROXY_UNAUTHORIZED,
 		G_CALLBACK (requeue_if_proxy_authenticated), manager);
 }
+
+static void
+session_request_started (SoupSession *session, SoupMessage *msg,
+			 SoupSocket *socket, gpointer data)
+{
+	SoupAuthManager *manager = data;
+	SoupAuth *auth;
+
+	auth = lookup_auth (manager, msg);
+	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, FALSE))
+		auth = NULL;
+	soup_message_set_auth (msg, auth);
+
+	auth = manager->proxy_auth;
+	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, TRUE))
+		auth = NULL;
+	soup_message_set_proxy_auth (msg, auth);
+}
+
+static void
+session_request_unqueued (SoupSession *session, SoupMessage *msg,
+			  gpointer data)
+{
+	SoupAuthManager *manager = data;
+
+	g_signal_handlers_disconnect_matched (msg, G_SIGNAL_MATCH_DATA,
+					      0, 0, NULL, NULL, manager);
+}

Modified: trunk/libsoup/soup-logger.c
==============================================================================
--- trunk/libsoup/soup-logger.c	(original)
+++ trunk/libsoup/soup-logger.c	Sun Mar 16 02:28:36 2008
@@ -321,8 +321,12 @@
 	g_object_set_qdata (object, priv->tag, NULL);
 }
 
+static void request_queued (SoupSession *session, SoupMessage *msg,
+			    gpointer user_data);
 static void request_started (SoupSession *session, SoupMessage *msg,
 			     SoupSocket *socket, gpointer user_data);
+static void request_unqueued (SoupSession *session, SoupMessage *msg,
+			      gpointer user_data);
 
 static void
 weak_notify_unref (gpointer logger, GObject *ex_session)
@@ -348,8 +352,12 @@
 {
 	if (!soup_logger_get_id (logger, session))
 		soup_logger_set_id (logger, session);
+	g_signal_connect (session, "request_queued",
+			  G_CALLBACK (request_queued), logger);
 	g_signal_connect (session, "request_started",
 			  G_CALLBACK (request_started), logger);
+	g_signal_connect (session, "request_unqueued",
+			  G_CALLBACK (request_unqueued), logger);
 
 	g_object_weak_ref (G_OBJECT (session),
 			   weak_notify_unref, g_object_ref (logger));
@@ -366,7 +374,9 @@
 soup_logger_detach (SoupLogger  *logger,
 		    SoupSession *session)
 {
+	g_signal_handlers_disconnect_by_func (session, request_queued, logger);
 	g_signal_handlers_disconnect_by_func (session, request_started, logger);
+	g_signal_handlers_disconnect_by_func (session, request_unqueued, logger);
 
 	g_object_weak_unref (G_OBJECT (session),
 			     weak_notify_unref, logger);
@@ -600,15 +610,16 @@
 }
 
 static void
-finished_handler (SoupMessage *msg, gpointer user_data)
+request_queued (SoupSession *session, SoupMessage *msg, gpointer user_data)
 {
 	SoupLogger *logger = user_data;
 
-	g_signal_handlers_disconnect_by_func (msg, got_informational, logger);
-	g_signal_handlers_disconnect_by_func (msg, got_body, logger);
-	g_signal_handlers_disconnect_by_func (msg, finished_handler, logger);
-
-	soup_logger_clear_id (logger, msg);
+	g_signal_connect (msg, "got-informational",
+			  G_CALLBACK (got_informational),
+			  logger);
+	g_signal_connect (msg, "got-body",
+			  G_CALLBACK (got_body),
+			  logger);
 }
 
 static void
@@ -623,18 +634,8 @@
 	if (msg_id)
 		restarted = TRUE;
 	else {
-		msg_id = soup_logger_set_id (logger, msg);
+		soup_logger_set_id (logger, msg);
 		restarted = FALSE;
-
-		g_signal_connect (msg, "got-informational",
-				  G_CALLBACK (got_informational),
-				  logger);
-		g_signal_connect (msg, "got-body",
-				  G_CALLBACK (got_body),
-				  logger);
-		g_signal_connect (msg, "finished",
-				  G_CALLBACK (finished_handler),
-				  logger);
 	}
 
 	if (!soup_logger_get_id (logger, socket))
@@ -643,3 +644,14 @@
 	print_request (logger, msg, session, socket, restarted);
 	soup_logger_print (logger, SOUP_LOGGER_LOG_MINIMAL, ' ', "");
 }
+
+static void
+request_unqueued (SoupSession *session, SoupMessage *msg, gpointer user_data)
+{
+	SoupLogger *logger = user_data;
+
+	g_signal_handlers_disconnect_by_func (msg, got_informational, logger);
+	g_signal_handlers_disconnect_by_func (msg, got_body, logger);
+
+	soup_logger_clear_id (logger, msg);
+}

Modified: trunk/libsoup/soup-session.c
==============================================================================
--- trunk/libsoup/soup-session.c	(original)
+++ trunk/libsoup/soup-session.c	Sun Mar 16 02:28:36 2008
@@ -121,7 +121,9 @@
 G_DEFINE_TYPE (SoupSession, soup_session, G_TYPE_OBJECT)
 
 enum {
+	REQUEST_QUEUED,
 	REQUEST_STARTED,
+	REQUEST_UNQUEUED,
 	AUTHENTICATE,
 	LAST_SIGNAL
 };
@@ -255,12 +257,65 @@
 	/* signals */
 
 	/**
+	 * SoupSession::request-queued:
+	 * @session: the session
+	 * @msg: the request that was queued
+	 *
+	 * Emitted when a request is queued on @session. (Note that
+	 * "queued" doesn't just mean soup_session_queue_message();
+	 * soup_session_send_message() implicitly queues the message
+	 * as well.)
+	 *
+	 * When sending a request, first #SoupSession::request_queued
+	 * is emitted, indicating that the session has become aware of
+	 * the request.
+	 *
+	 * Once a connection is available to send the request on, the
+	 * session emits #SoupSession::request_started. Then, various
+	 * #SoupMessage signals are emitted as the message is
+	 * processed. If the message is requeued, it will emit
+	 * #SoupMessage::restarted, which will then be followed by
+	 * another #SoupSession::request_started and another set of
+	 * #SoupMessage signals when the message is re-sent.
+	 *
+	 * Eventually, the message will emit #SoupMessage::finished.
+	 * Normally, this signals the completion of message
+	 * processing. However, it is possible that the application
+	 * will requeue the message from the "finished" handler (or
+	 * equivalently, from the soup_session_queue_message()
+	 * callback). In that case, the process will loop back to
+	 * #SoupSession::request_started.
+	 *
+	 * Eventually, a message will reach "finished" and not be
+	 * requeued. At that point, the session will emit
+	 * #SoupSession::request_unqueued to indicate that it is done
+	 * with the message.
+	 *
+	 * To sum up: #SoupSession::request_queued and
+	 * #SoupSession::request_unqueued are guaranteed to be emitted
+	 * exactly once, but #SoupSession::request_started and
+	 * #SoupMessage::finished (and all of the other #SoupMessage
+	 * signals) may be invoked multiple times for a given message.
+	 **/
+	signals[REQUEST_QUEUED] =
+		g_signal_new ("request-queued",
+			      G_OBJECT_CLASS_TYPE (object_class),
+			      G_SIGNAL_RUN_FIRST,
+			      0, /* FIXME? */
+			      NULL, NULL,
+			      soup_marshal_NONE__OBJECT,
+			      G_TYPE_NONE, 1,
+			      SOUP_TYPE_MESSAGE);
+
+	/**
 	 * SoupSession::request-started:
 	 * @session: the session
 	 * @msg: the request being sent
 	 * @socket: the socket the request is being sent on
 	 *
-	 * Emitted just before a request is sent.
+	 * Emitted just before a request is sent. See
+	 * #SoupSession::request_queued for a detailed description of
+	 * the message lifecycle within a session.
 	 **/
 	signals[REQUEST_STARTED] =
 		g_signal_new ("request-started",
@@ -274,6 +329,26 @@
 			      SOUP_TYPE_SOCKET);
 
 	/**
+	 * SoupSession::request-unqueued:
+	 * @session: the session
+	 * @msg: the request that was unqueued
+	 *
+	 * Emitted when a request is removed from @session's queue,
+	 * indicating that @session is done with it. See
+	 * #SoupSession::request_queued for a detailed description of the
+	 * message lifecycle within a session.
+	 **/
+	signals[REQUEST_UNQUEUED] =
+		g_signal_new ("request-unqueued",
+			      G_OBJECT_CLASS_TYPE (object_class),
+			      G_SIGNAL_RUN_FIRST,
+			      0, /* FIXME? */
+			      NULL, NULL,
+			      soup_marshal_NONE__OBJECT,
+			      G_TYPE_NONE, 1,
+			      SOUP_TYPE_MESSAGE);
+
+	/**
 	 * SoupSession::authenticate:
 	 * @session: the session
 	 * @msg: the #SoupMessage being sent
@@ -716,6 +791,12 @@
 					      "User-Agent", priv->user_agent);
 	}
 
+	/* Kludge to deal with the fact that CONNECT msgs come from the
+	 * SoupConnection rather than being queued normally.
+	 */
+	if (msg->method == SOUP_METHOD_CONNECT)
+		g_signal_emit (session, signals[REQUEST_QUEUED], 0, msg);
+
 	g_signal_emit (session, signals[REQUEST_STARTED], 0,
 		       msg, soup_connection_get_socket (conn));
 }
@@ -984,6 +1065,8 @@
 	if (!SOUP_MESSAGE_IS_STARTING (msg)) {
 		soup_message_queue_remove_message (priv->queue, msg);
 		g_signal_handlers_disconnect_by_func (msg, message_finished, session);
+		g_signal_handlers_disconnect_by_func (msg, redirect_handler, session);
+		g_signal_emit (session, signals[REQUEST_UNQUEUED], 0, msg);
 	}
 }
 
@@ -1004,6 +1087,8 @@
 
 	soup_message_set_io_status (msg, SOUP_MESSAGE_IO_STATUS_QUEUED);
 	soup_message_queue_append (priv->queue, msg);
+
+	g_signal_emit (session, signals[REQUEST_QUEUED], 0, msg);
 }
 
 /**

Modified: trunk/tests/auth-test.c
==============================================================================
--- trunk/tests/auth-test.c	(original)
+++ trunk/tests/auth-test.c	Sun Mar 16 02:28:36 2008
@@ -398,6 +398,24 @@
 }
 
 static void
+async_authenticate_522601 (SoupSession *session, SoupMessage *msg,
+			   SoupAuth *auth, gboolean retrying, gpointer data)
+{
+	gboolean *been_here = data;
+
+	debug_printf (2, "  async_authenticate_522601\n");
+
+	if (*been_here) {
+		debug_printf (1, "  ERROR: async_authenticate_522601 called twice\n");
+		errors++;
+	}
+	*been_here = TRUE;
+
+	soup_session_pause_message (session, msg);
+	g_main_loop_quit (loop);
+}
+
+static void
 do_async_auth_test (const char *base_uri)
 {
 	SoupSession *session;
@@ -406,6 +424,7 @@
 	char *uri;
 	SoupAuth *auth = NULL;
 	int finished = 0;
+	gboolean been_there = FALSE;
 
 	debug_printf (1, "\nTesting async auth:\n");
 
@@ -457,6 +476,7 @@
 	/* Now do the auth, and restart */
 	if (auth) {
 		soup_auth_authenticate (auth, "user1", "realm1");
+		g_object_unref (auth);
 		soup_session_unpause_message (session, msg1);
 		soup_session_unpause_message (session, msg3);
 
@@ -490,7 +510,38 @@
 	g_object_unref (msg3);
 	memcpy (msg2, &msg2_bak, sizeof (SoupMessage));
 	g_object_unref (msg2);
+
+	/* Test that giving the wrong password doesn't cause multiple
+	 * authenticate signals the second time.
+	 */
+	debug_printf (1, "\nTesting async auth with wrong password (#522601):\n");
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+	auth = NULL;
+
+	msg1 = soup_message_new ("GET", uri);
+	g_object_set_data (G_OBJECT (msg1), "id", GINT_TO_POINTER (1));
+	auth_id = g_signal_connect (session, "authenticate",
+				    G_CALLBACK (async_authenticate), &auth);
+	g_object_ref (msg1);
+	soup_session_queue_message (session, msg1, async_finished, &finished);
+	g_main_loop_run (loop);
+	g_signal_handler_disconnect (session, auth_id);
+	soup_auth_authenticate (auth, "user1", "wrong");
 	g_object_unref (auth);
+	soup_session_unpause_message (session, msg1);
+
+	auth_id = g_signal_connect (session, "authenticate",
+				    G_CALLBACK (async_authenticate_522601),
+				    &been_there);
+	g_main_loop_run (loop);
+	g_signal_handler_disconnect (session, auth_id);
+
+	soup_session_abort (session);
+	g_object_unref (session);
+
+	g_object_unref (msg1);
+
 	g_free (uri);
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]