From ef359ad7565e3b46691582358fd07f1148c933f7 Mon Sep 17 00:00:00 2001
From: Dan Winship <danw@gnome.org>
Date: Wed, 6 Mar 2013 12:58:29 -0500
Subject: SoupSession: break infinite loops

Every year or two some bug comes up that makes libsoup retry a request
infinitely. (Also, apps can do this on their own by not paying
attention to the "retrying" flag in SoupSession::authenticate.) Move
the "too many redirects" code and rework it to handle all possible
cases of "message gets resent a suspicious number of times".
---
 libsoup/soup-message-queue.h |  2 +-
 libsoup/soup-session.c       | 29 +++++++++++--------------
 tests/auth-test.c            | 51 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 18 deletions(-)

diff --git a/libsoup/soup-message-queue.h b/libsoup/soup-message-queue.h
index 848ecd02..135a6aac 100644
--- a/libsoup/soup-message-queue.h
+++ b/libsoup/soup-message-queue.h
@@ -46,7 +46,7 @@ struct _SoupMessageQueueItem {
 	guint new_api           : 1;
 	guint io_started        : 1;
 	guint async             : 1;
-	guint redirection_count : 28;
+	guint resend_count      : 28;
 
 	SoupMessageQueueItemState state;
 
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index ee470019..a153cde5 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -149,7 +149,7 @@ static void async_send_request_running (SoupSession *session, SoupMessageQueueIt
 #define SOUP_SESSION_MAX_CONNS_DEFAULT 10
 #define SOUP_SESSION_MAX_CONNS_PER_HOST_DEFAULT 2
 
-#define SOUP_SESSION_MAX_REDIRECTION_COUNT 20
+#define SOUP_SESSION_MAX_RESEND_COUNT 20
 
 #define SOUP_SESSION_USER_AGENT_BASE "libsoup/" PACKAGE_VERSION
 
@@ -1088,27 +1088,12 @@ soup_session_would_redirect (SoupSession *session, SoupMessage *msg)
 gboolean
 soup_session_redirect_message (SoupSession *session, SoupMessage *msg)
 {
-	SoupMessageQueueItem *item;
 	SoupURI *new_uri;
 
 	new_uri = redirection_uri (msg);
 	if (!new_uri)
 		return FALSE;
 
-	item = soup_message_queue_lookup (soup_session_get_queue (session), msg);
-	if (!item) {
-		soup_uri_free (new_uri);
-		return FALSE;
-	}
-	if (item->redirection_count >= SOUP_SESSION_MAX_REDIRECTION_COUNT) {
-		soup_uri_free (new_uri);
-		soup_session_cancel_message (session, msg, SOUP_STATUS_TOO_MANY_REDIRECTS);
-		soup_message_queue_item_unref (item);
-		return FALSE;
-	}
-	item->redirection_count++;
-	soup_message_queue_item_unref (item);
-
 	if (SOUP_SESSION_WOULD_REDIRECT_AS_GET (session, msg)) {
 		if (msg->method != SOUP_METHOD_HEAD) {
 			g_object_set (msg,
@@ -2008,7 +1993,17 @@ soup_session_real_requeue_message (SoupSession *session, SoupMessage *msg)
 
 	item = soup_message_queue_lookup (priv->queue, msg);
 	g_return_if_fail (item != NULL);
-	item->state = SOUP_MESSAGE_RESTARTING;
+
+	if (item->resend_count >= SOUP_SESSION_MAX_RESEND_COUNT) {
+		if (SOUP_STATUS_IS_REDIRECTION (msg->status_code))
+			soup_message_set_status (msg, SOUP_STATUS_TOO_MANY_REDIRECTS);
+		else
+			g_warning ("SoupMessage %p stuck in infinite loop?", msg);
+	} else {
+		item->resend_count++;
+		item->state = SOUP_MESSAGE_RESTARTING;
+	}
+
 	soup_message_queue_item_unref (item);
 }
 
diff --git a/tests/auth-test.c b/tests/auth-test.c
index 2c443d63..fba6f1e1 100644
--- a/tests/auth-test.c
+++ b/tests/auth-test.c
@@ -1074,6 +1074,56 @@ do_auth_close_test (void)
 	soup_test_server_quit_unref (server);
 }
 
+static gboolean
+infinite_cancel (gpointer session)
+{
+	soup_session_abort (session);
+	return FALSE;
+}
+
+static void
+infinite_authenticate (SoupSession *session, SoupMessage *msg,
+		       SoupAuth *auth, gboolean retrying, gpointer data)
+{
+	soup_auth_authenticate (auth, "user", "bad");
+}
+
+static void
+do_infinite_auth_test (const char *base_uri)
+{
+	SoupSession *session;
+	SoupMessage *msg;
+	char *uri;
+	int timeout;
+
+	debug_printf (1, "\nTesting broken infinite-loop auth:\n");
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+	g_signal_connect (session, "authenticate",
+			  G_CALLBACK (infinite_authenticate), NULL);
+
+	uri = g_strconcat (base_uri, "Basic/realm1/", NULL);
+	msg = soup_message_new ("GET", uri);
+	g_free (uri);
+
+	timeout = g_timeout_add (500, infinite_cancel, session);
+	expect_warning = TRUE;
+	soup_session_send_message (session, msg);
+
+	if (msg->status_code == SOUP_STATUS_CANCELLED) {
+		debug_printf (1, "    FAILED: Got stuck in loop");
+		errors++;
+	} else if (msg->status_code != SOUP_STATUS_UNAUTHORIZED) {
+		debug_printf (1, "    Final status wrong: expected 401, got %u\n",
+			      msg->status_code);
+		errors++;
+	}
+
+	g_source_remove (timeout);
+	soup_test_session_abort_unref (session);
+	g_object_unref (msg);
+}
+
 static SoupAuthTest relogin_tests[] = {
 	{ "Auth provided via URL, should succeed",
 	  "Basic/realm12/", "1", TRUE, "01", SOUP_STATUS_OK },
@@ -1199,6 +1249,7 @@ main (int argc, char **argv)
 	do_async_auth_test (base_uri);
 	do_select_auth_test ();
 	do_auth_close_test ();
+	do_infinite_auth_test (base_uri);
 
 	test_cleanup ();
 	return errors != 0;
-- 
cgit v1.2.3