[libsoup] Wait until IO finished before re-starting a request

[Carlos Garcia Campos <carlosgc src gnome org>]

commit 890766793f9d349c54a7437b32dba9ae79690579
Author: Carlos Garcia Campos <cgarcia igalia com>
Date:   Fri Jun 11 12:04:10 2021 +0200

    Wait until IO finished before re-starting a request
    
    In case of restarting it can happen that the new request is started
    before the previous one is fully completed. This is because restart
    is scheduled on SoupMessage::got-body that happens right before the
    stream is closed and the IO finished. In HTTP/1.x this is possible if a
    new request is made in got-body callback, for example. In HTTP/2 is
    easier to reproduce because we emit SoupMessage::got-body right after
    SoupClientInputStream::eof. This patch adds an intermediate state
    SOUP_MESSAGE_REQUEUED set by soup_session_requeue_item(). The transition
    to SOUP_MESSAGE_RESTARTING happens when the message IO completes to
    ensure the restarted request starts when message no longer has an active IO.

 libsoup/http1/soup-client-message-io-http1.c |  2 ++
 libsoup/soup-message-queue-item.h            |  1 +
 libsoup/soup-session.c                       |  9 ++++++-
 tests/misc-test.c                            | 40 ++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 1 deletion(-)
---
diff --git a/libsoup/http1/soup-client-message-io-http1.c b/libsoup/http1/soup-client-message-io-http1.c
index ab589e43..46029612 100644
--- a/libsoup/http1/soup-client-message-io-http1.c
+++ b/libsoup/http1/soup-client-message-io-http1.c
@@ -1043,6 +1043,8 @@ soup_client_message_io_http1_send_item (SoupClientMessageIO       *iface,
 #ifdef HAVE_SYSPROF
         msg_io->begin_time_nsec = SYSPROF_CAPTURE_CURRENT_TIME;
 #endif
+        if (io->msg_io)
+                g_warn_if_reached ();
 
         io->msg_io = msg_io;
         io->is_reusable = FALSE;
diff --git a/libsoup/soup-message-queue-item.h b/libsoup/soup-message-queue-item.h
index cb483668..9a958342 100644
--- a/libsoup/soup-message-queue-item.h
+++ b/libsoup/soup-message-queue-item.h
@@ -20,6 +20,7 @@ typedef enum {
         SOUP_MESSAGE_READY,
         SOUP_MESSAGE_RUNNING,
         SOUP_MESSAGE_CACHED,
+        SOUP_MESSAGE_REQUEUED,
         SOUP_MESSAGE_RESTARTING,
         SOUP_MESSAGE_FINISHING,
         SOUP_MESSAGE_FINISHED
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index d1df9226..7380ff0a 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -1205,7 +1205,7 @@ soup_session_requeue_item (SoupSession          *session,
                retval = FALSE;
        } else {
                item->resend_count++;
-               item->state = SOUP_MESSAGE_RESTARTING;
+               item->state = SOUP_MESSAGE_REQUEUED;
                retval = TRUE;
        }
 
@@ -1590,6 +1590,9 @@ message_completed (SoupMessage *msg, SoupMessageIOCompletion completion, gpointe
                return;
        }
 
+        if (item->state == SOUP_MESSAGE_REQUEUED)
+                item->state = SOUP_MESSAGE_RESTARTING;
+
        if (item->state != SOUP_MESSAGE_RESTARTING) {
                item->state = SOUP_MESSAGE_FINISHING;
 
@@ -1646,6 +1649,9 @@ tunnel_message_completed (SoupMessage *msg, SoupMessageIOCompletion completion,
        SoupSession *session = tunnel_item->session;
        guint status;
 
+        if (tunnel_item->state == SOUP_MESSAGE_REQUEUED)
+                tunnel_item->state = SOUP_MESSAGE_RESTARTING;
+
        if (tunnel_item->state == SOUP_MESSAGE_RESTARTING) {
                soup_message_restarted (msg);
                if (soup_message_get_connection (tunnel_item->msg)) {
@@ -2028,6 +2034,7 @@ soup_session_process_queue_item (SoupSession          *session,
 
                case SOUP_MESSAGE_CACHED:
                case SOUP_MESSAGE_TUNNELING:
+                case SOUP_MESSAGE_REQUEUED:
                        /* Will be handled elsewhere */
                        return;
 
diff --git a/tests/misc-test.c b/tests/misc-test.c
index 25d263ef..e32b18f5 100644
--- a/tests/misc-test.c
+++ b/tests/misc-test.c
@@ -760,6 +760,45 @@ do_remote_address_test (void)
         soup_test_session_abort_unref (session);
 }
 
+static void
+redirect_handler (SoupMessage *msg,
+                  SoupSession *session)
+{
+        SoupMessage *new_msg;
+        GBytes *body;
+
+        new_msg = soup_message_new_from_uri ("GET", base_uri);
+        body = soup_test_session_async_send (session, new_msg, NULL, NULL);
+        g_assert_nonnull (body);
+        g_assert_cmpstr (g_bytes_get_data (body, NULL), ==, "index");
+        g_object_unref (new_msg);
+}
+
+static void
+do_new_request_on_redirect_test (void)
+{
+        SoupSession *session;
+        GUri *uri;
+        SoupMessage *msg;
+        GBytes *body;
+
+        session = soup_test_session_new (NULL);
+
+        uri = g_uri_parse_relative (base_uri, "/redirect", SOUP_HTTP_URI_FLAGS, NULL);
+        msg = soup_message_new_from_uri ("GET", uri);
+        g_signal_connect_after (msg, "got-body",
+                                G_CALLBACK (redirect_handler),
+                                session);
+        body = soup_test_session_async_send (session, msg, NULL, NULL);
+        g_assert_nonnull (body);
+        g_assert_cmpstr (g_bytes_get_data (body, NULL), ==, "index");
+
+        g_bytes_unref (body);
+        g_object_unref (msg);
+        g_uri_unref (uri);
+        soup_test_session_abort_unref (session);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -794,6 +833,7 @@ main (int argc, char **argv)
        g_test_add_func ("/misc/msg-flags", do_msg_flags_test);
         g_test_add_func ("/misc/connection-id", do_connection_id_test);
         g_test_add_func ("/misc/remote-address", do_remote_address_test);
+        g_test_add_func ("/misc/new-request-on-redirect", do_new_request_on_redirect_test);
 
        ret = g_test_run ();