[libsoup] Don't emit "authenticate" until a message is actually sent and fails.

[Dan Winship <danw src gnome org>]

commit dfdde36f28e3092c6ac20ecae3774903be92a153
Author: Dan Winship <danw gnome org>
Date:   Thu May 21 18:36:17 2009 -0300

    Don't emit "authenticate" until a message is actually sent and fails.
    
    In some cases, we're reasonably sure it's going to fail if we don't
    emit authenticate, but forcing callers to deal with both the
    pre-sending and post-sending cases in their authenticate handlers
    makes writing them tricky. (In particular, calling
    soup_message_io_pause() in the pre-sending case won't work, since the
    message hasn't actually started yet.) This also solves the problem
    that we were previously emitting "authenticate" both before and after
    the message was sent in some cases.
    
    Add a regression test for this case (written by Gustavo Noronha, who
    figured out the circumstances where the bug was happening), and tweak
    the existing regression tests to match the new rules.
    
    http://bugzilla.gnome.org/show_bug.cgi?id=583462
---
 libsoup/soup-auth-manager.c |   17 ++++++----
 tests/auth-test.c           |   72 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/libsoup/soup-auth-manager.c b/libsoup/soup-auth-manager.c
index 92ffae8..19c475f 100644
--- a/libsoup/soup-auth-manager.c
+++ b/libsoup/soup-auth-manager.c
@@ -354,7 +354,7 @@ lookup_auth (SoupAuthManagerPrivate *priv, SoupMessage *msg)
 static gboolean
 authenticate_auth (SoupAuthManager *manager, SoupAuth *auth,
 		   SoupMessage *msg, gboolean prior_auth_failed,
-		   gboolean proxy)
+		   gboolean proxy, gboolean can_interact)
 {
 	SoupAuthManagerPrivate *priv = SOUP_AUTH_MANAGER_GET_PRIVATE (manager);
 	SoupURI *uri;
@@ -379,8 +379,11 @@ authenticate_auth (SoupAuthManager *manager, SoupAuth *auth,
 	}
 	soup_uri_free (uri);
 
-	soup_auth_manager_emit_authenticate (manager, msg, auth,
-					     prior_auth_failed);
+	if (can_interact) {
+		soup_auth_manager_emit_authenticate (manager, msg, auth,
+						     prior_auth_failed);
+	}
+
 	return soup_auth_is_authenticated (auth);
 }
 
@@ -449,7 +452,7 @@ update_auth (SoupMessage *msg, gpointer manager)
 
 	/* If we need to authenticate, try to do it. */
 	authenticate_auth (manager, auth, msg,
-			   prior_auth_failed, FALSE);
+			   prior_auth_failed, FALSE, TRUE);
 }
 
 static void
@@ -484,7 +487,7 @@ update_proxy_auth (SoupMessage *msg, gpointer manager)
 
 	/* If we need to authenticate, try to do it. */
 	authenticate_auth (manager, priv->proxy_auth, msg,
-			   prior_auth_failed, TRUE);
+			   prior_auth_failed, TRUE, TRUE);
 }
 
 static void
@@ -525,12 +528,12 @@ request_started (SoupSessionFeature *feature, SoupSession *session,
 	SoupAuth *auth;
 
 	auth = lookup_auth (priv, msg);
-	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, FALSE))
+	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, FALSE, FALSE))
 		auth = NULL;
 	soup_message_set_auth (msg, auth);
 
 	auth = priv->proxy_auth;
-	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, TRUE))
+	if (!auth || !authenticate_auth (manager, auth, msg, FALSE, TRUE, FALSE))
 		auth = NULL;
 	soup_message_set_proxy_auth (msg, auth);
 }
diff --git a/tests/auth-test.c b/tests/auth-test.c
index cc992b1..b090e23 100644
--- a/tests/auth-test.c
+++ b/tests/auth-test.c
@@ -50,8 +50,8 @@ static SoupAuthTest tests[] = {
 	{ "Should fail with no auth, fail again with bad password, and give up",
 	  "Basic/realm2/", "4", "04", SOUP_STATUS_UNAUTHORIZED },
 
-	{ "Known realm, auth provided, so should succeed immediately",
-	  "Basic/realm1/", "1", "1", SOUP_STATUS_OK },
+	{ "Auth provided this time, so should succeed",
+	  "Basic/realm1/", "1", "01", SOUP_STATUS_OK },
 
 	{ "Now should automatically reuse previous auth",
 	  "Basic/realm1/", "", "1", SOUP_STATUS_OK },
@@ -62,8 +62,8 @@ static SoupAuthTest tests[] = {
 	{ "Subdir should retry last auth, but will fail this time",
 	  "Basic/realm1/realm2/", "", "1", SOUP_STATUS_UNAUTHORIZED },
 
-	{ "Now should use provided auth on first try",
-	  "Basic/realm1/realm2/", "2", "2", SOUP_STATUS_OK },
+	{ "Now should use provided auth",
+	  "Basic/realm1/realm2/", "2", "02", SOUP_STATUS_OK },
 
 	{ "Reusing last auth. Should succeed on first try",
 	  "Basic/realm1/realm2/", "", "2", SOUP_STATUS_OK },
@@ -84,8 +84,8 @@ static SoupAuthTest tests[] = {
 	{ "Should fail with no auth, fail again with bad password, and give up",
 	  "Digest/realm2/", "4", "04", SOUP_STATUS_UNAUTHORIZED },
 
-	{ "Known realm, auth provided, so should succeed immediately",
-	  "Digest/realm1/", "1", "1", SOUP_STATUS_OK },
+	{ "Known realm, auth provided, so should succeed",
+	  "Digest/realm1/", "1", "01", SOUP_STATUS_OK },
 
 	{ "Now should automatically reuse previous auth",
 	  "Digest/realm1/", "", "1", SOUP_STATUS_OK },
@@ -93,6 +93,9 @@ static SoupAuthTest tests[] = {
 	{ "Subdir should also automatically reuse auth",
 	  "Digest/realm1/subdir/", "", "1", SOUP_STATUS_OK },
 
+	{ "Password provided, should succeed",
+	  "Digest/realm2/", "2", "02", SOUP_STATUS_OK },
+
 	{ "Should already know correct domain and use provided auth on first try",
 	  "Digest/realm1/realm2/", "2", "2", SOUP_STATUS_OK },
 
@@ -102,9 +105,6 @@ static SoupAuthTest tests[] = {
 	{ "Should succeed on first try because of earlier domain directive",
 	  "Digest/realm1/realm2/realm1/", "", "1", SOUP_STATUS_OK },
 
-	{ "Should succeed on first try. (Known realm with cached password)",
-	  "Digest/realm2/", "", "2", SOUP_STATUS_OK },
-
 	{ "Fail once, then use typoed password, then use right password",
 	  "Digest/realm3/", "43", "043", SOUP_STATUS_OK },
 
@@ -398,15 +398,15 @@ async_finished (SoupSession *session, SoupMessage *msg, gpointer user_data)
 }
 
 static void
-async_authenticate_522601 (SoupSession *session, SoupMessage *msg,
-			   SoupAuth *auth, gboolean retrying, gpointer data)
+async_authenticate_assert_once (SoupSession *session, SoupMessage *msg,
+                                SoupAuth *auth, gboolean retrying, gpointer data)
 {
 	gboolean *been_here = data;
 
-	debug_printf (2, "  async_authenticate_522601\n");
+	debug_printf (2, "  async_authenticate_assert_once\n");
 
 	if (*been_here) {
-		debug_printf (1, "  ERROR: async_authenticate_522601 called twice\n");
+		debug_printf (1, "  ERROR: async_authenticate_assert_once called twice\n");
 		errors++;
 	}
 	*been_here = TRUE;
@@ -531,8 +531,52 @@ do_async_auth_test (const char *base_uri)
 	soup_session_unpause_message (session, msg1);
 
 	auth_id = g_signal_connect (session, "authenticate",
-				    G_CALLBACK (async_authenticate_522601),
+				    G_CALLBACK (async_authenticate_assert_once),
+				    &been_there);
+	g_main_loop_run (loop);
+	g_signal_handler_disconnect (session, auth_id);
+
+	soup_test_session_abort_unref (session);
+
+	g_object_unref (msg1);
+
+
+	/* Test that giving no password doesn't cause multiple
+	 * authenticate signals the second time.
+	 */
+	debug_printf (1, "\nTesting async auth with no password (#583462):\n");
+
+	/* For this test, our first message will not finish twice */
+	finished = 1;
+	been_there = FALSE;
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+
+	/* Send a message that doesn't actually authenticate
+	 */
+	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), NULL);
+	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_session_unpause_message (session, msg1);
+	g_main_loop_run (loop);
+	g_object_unref(msg1);
+
+	/* Now send a second message */
+	finished = 1;
+	msg1 = soup_message_new ("GET", uri);
+	g_object_set_data (G_OBJECT (msg1), "id", GINT_TO_POINTER (2));
+	g_object_ref (msg1);
+	auth_id = g_signal_connect (session, "authenticate",
+				    G_CALLBACK (async_authenticate_assert_once),
 				    &been_there);
+	soup_session_queue_message (session, msg1, async_finished, &finished);
+	g_main_loop_run (loop);
+	soup_session_unpause_message (session, msg1);
+
 	g_main_loop_run (loop);
 	g_signal_handler_disconnect (session, auth_id);