[libsoup] SoupSession: break infinite loops

[Dan Winship <danw src gnome org>]

commit ef359ad7565e3b46691582358fd07f1148c933f7
Author: Dan Winship <danw gnome org>
Date:   Wed Mar 6 12:58:29 2013 -0500

    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 848ecd0..135a6aa 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 ee47001..a153cde 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 2c443d6..fba6f1e 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;