Force some SoupMessages to use a fresh SoupConnection
authorDan Winship <danw@gnome.org>
Wed, 7 Dec 2011 21:53:26 +0000 (16:53 -0500)
committerDan Winship <danw@gnome.org>
Thu, 22 Dec 2011 18:49:14 +0000 (13:49 -0500)
Add a new SOUP_MESSAGE_NEW_CONNECTION flag, and insist on getting a
brand new SoupConnection for any message that has that flag set, or
that uses a non-idempotent method.

Add a test to misc-test for this.

https://bugzilla.gnome.org/show_bug.cgi?id=578990

libsoup/soup-auth-manager-ntlm.c
libsoup/soup-message.c
libsoup/soup-message.h
libsoup/soup-session.c
tests/misc-test.c

index a1b76c7..33043e7 100644 (file)
@@ -449,6 +449,7 @@ ntlm_authorize_post (SoupMessage *msg, gpointer ntlm)
        SoupNTLMConnection *conn;
        const char *username = NULL, *password = NULL;
        char *slash, *domain = NULL;
+       SoupMessageFlags flags;
 
        conn = get_connection_for_msg (priv, msg);
        if (!conn || !conn->auth)
@@ -496,6 +497,9 @@ ssofailure:
        conn->response_header = soup_ntlm_response (conn->nonce,
                                                    username, password,
                                                    NULL, domain);
+
+       flags = soup_message_get_flags (msg);
+       soup_message_set_flags (msg, flags & ~SOUP_MESSAGE_NEW_CONNECTION);
        soup_session_requeue_message (priv->session, msg);
 
 done:
index dc99f38..6186087 100644 (file)
@@ -1430,6 +1430,10 @@ soup_message_cleanup_response (SoupMessage *req)
  * @SOUP_MESSAGE_CERTIFICATE_TRUSTED: if set after an https response
  *   has been received, indicates that the server's SSL certificate is
  *   trusted according to the session's CA.
+ * @SOUP_MESSAGE_NEW_CONNECTION: The message should be sent on a
+ *   newly-created connection, not reusing an existing persistent
+ *   connection. Note that messages with non-idempotent
+ *   #SoupMessage:method<!-- -->s behave this way by default.
  *
  * Various flags that can be set on a #SoupMessage to alter its
  * behavior.
index f1a8c6d..d3c7e3c 100644 (file)
@@ -118,7 +118,8 @@ typedef enum {
        SOUP_MESSAGE_OVERWRITE_CHUNKS     = (1 << 3),
 #endif
        SOUP_MESSAGE_CONTENT_DECODED      = (1 << 4),
-       SOUP_MESSAGE_CERTIFICATE_TRUSTED  = (1 << 5)
+       SOUP_MESSAGE_CERTIFICATE_TRUSTED  = (1 << 5),
+       SOUP_MESSAGE_NEW_CONNECTION       = (1 << 6)
 } SoupMessageFlags;
 
 void             soup_message_set_flags           (SoupMessage           *msg,
index 32c1b30..e0d540c 100644 (file)
@@ -1509,11 +1509,22 @@ auth_manager_authenticate (SoupAuthManager *manager, SoupMessage *msg,
                session, msg, auth, retrying);
 }
 
+/* At some point it might be possible to mark additional methods
+ * safe or idempotent...
+ */
 #define SOUP_METHOD_IS_SAFE(method) (method == SOUP_METHOD_GET || \
                                     method == SOUP_METHOD_HEAD || \
                                     method == SOUP_METHOD_OPTIONS || \
                                     method == SOUP_METHOD_PROPFIND)
 
+#define SOUP_METHOD_IS_IDEMPOTENT(method) (method == SOUP_METHOD_GET || \
+                                          method == SOUP_METHOD_HEAD || \
+                                          method == SOUP_METHOD_OPTIONS || \
+                                          method == SOUP_METHOD_PROPFIND || \
+                                          method == SOUP_METHOD_PUT || \
+                                          method == SOUP_METHOD_DELETE)
+
+
 #define SOUP_SESSION_WOULD_REDIRECT_AS_GET(session, msg) \
        ((msg)->status_code == SOUP_STATUS_SEE_OTHER || \
         ((msg)->status_code == SOUP_STATUS_FOUND && \
@@ -1850,17 +1861,22 @@ soup_session_get_connection (SoupSession *session,
        GSList *conns;
        int num_pending = 0;
        SoupURI *uri;
+       gboolean need_new_connection;
 
        if (item->conn) {
                g_return_val_if_fail (soup_connection_get_state (item->conn) != SOUP_CONNECTION_DISCONNECTED, FALSE);
                return TRUE;
        }
 
+       need_new_connection =
+               (soup_message_get_flags (item->msg) & SOUP_MESSAGE_NEW_CONNECTION) ||
+               !SOUP_METHOD_IS_IDEMPOTENT (item->msg->method);
+
        g_mutex_lock (&priv->host_lock);
 
        host = get_host_for_message (session, item->msg);
        for (conns = host->connections; conns; conns = conns->next) {
-               if (soup_connection_get_state (conns->data) == SOUP_CONNECTION_IDLE) {
+               if (!need_new_connection && soup_connection_get_state (conns->data) == SOUP_CONNECTION_IDLE) {
                        soup_connection_set_state (conns->data, SOUP_CONNECTION_IN_USE);
                        g_mutex_unlock (&priv->host_lock);
                        item->conn = g_object_ref (conns->data);
@@ -1879,6 +1895,8 @@ soup_session_get_connection (SoupSession *session,
        }
 
        if (host->num_conns >= priv->max_conns_per_host) {
+               if (need_new_connection)
+                       *try_pruning = TRUE;
                g_mutex_unlock (&priv->host_lock);
                return FALSE;
        }
index 332e938..9637dcb 100644 (file)
@@ -136,7 +136,7 @@ server_callback (SoupServer *server, SoupMessage *msg,
                return;
        }
 
-       if (msg->method != SOUP_METHOD_GET) {
+       if (msg->method != SOUP_METHOD_GET && msg->method != SOUP_METHOD_POST) {
                soup_message_set_status (msg, SOUP_STATUS_NOT_IMPLEMENTED);
                return;
        }
@@ -748,8 +748,8 @@ do_accept_language_test (void)
 }
 
 static void
-timeout_test_request_started (SoupSession *session, SoupMessage *msg,
-                             SoupSocket *socket, gpointer user_data)
+request_started_socket_collector (SoupSession *session, SoupMessage *msg,
+                                 SoupSocket *socket, gpointer user_data)
 {
        SoupSocket **sockets = user_data;
        int i;
@@ -782,7 +782,7 @@ do_timeout_test_for_session (SoupSession *session)
        int i;
 
        g_signal_connect (session, "request-started",
-                         G_CALLBACK (timeout_test_request_started),
+                         G_CALLBACK (request_started_socket_collector),
                          &sockets);
 
        debug_printf (1, "    First message\n");
@@ -1103,6 +1103,72 @@ do_aliases_test (void)
        soup_test_session_abort_unref (session);
 }
 
+static void
+do_non_persistent_test_for_session (SoupSession *session)
+{
+       SoupMessage *msg;
+       SoupSocket *sockets[4] = { NULL, NULL, NULL, NULL };
+       int i;
+
+       g_signal_connect (session, "request-started",
+                         G_CALLBACK (request_started_socket_collector),
+                         &sockets);
+
+       debug_printf (2, "    GET\n");
+       msg = soup_message_new_from_uri ("GET", base_uri);
+       soup_session_send_message (session, msg);
+       if (msg->status_code != SOUP_STATUS_OK) {
+               debug_printf (1, "      Unexpected response: %d %s\n",
+                             msg->status_code, msg->reason_phrase);
+               errors++;
+       }
+       if (sockets[1]) {
+               debug_printf (1, "      Message was retried??\n");
+               errors++;
+               sockets[1] = sockets[2] = sockets[3] = NULL;
+       }
+       g_object_unref (msg);
+
+       debug_printf (2, "    POST\n");
+       msg = soup_message_new_from_uri ("POST", base_uri);
+       soup_session_send_message (session, msg);
+       if (msg->status_code != SOUP_STATUS_OK) {
+               debug_printf (1, "      Unexpected response: %d %s\n",
+                             msg->status_code, msg->reason_phrase);
+               errors++;
+       }
+       if (sockets[1] == sockets[0]) {
+               debug_printf (1, "      Message was sent on existing connection!\n");
+               errors++;
+       }
+       if (sockets[2]) {
+               debug_printf (1, "      Too many connections used...\n");
+               errors++;
+       }
+       g_object_unref (msg);
+
+       for (i = 0; sockets[i]; i++)
+               g_object_unref (sockets[i]);
+}
+
+static void
+do_non_persistent_connection_test (void)
+{
+       SoupSession *session;
+
+       debug_printf (1, "\nNon-idempotent methods are always sent on new connections\n");
+
+       debug_printf (1, "  Async session\n");
+       session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+       do_non_persistent_test_for_session (session);
+       soup_test_session_abort_unref (session);
+
+       debug_printf (1, "  Sync session\n");
+       session = soup_test_session_new (SOUP_TYPE_SESSION_SYNC, NULL);
+       do_non_persistent_test_for_session (session);
+       soup_test_session_abort_unref (session);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -1139,6 +1205,7 @@ main (int argc, char **argv)
        do_max_conns_test ();
        do_cancel_while_reading_test ();
        do_aliases_test ();
+       do_non_persistent_connection_test ();
 
        soup_uri_free (base_uri);
        soup_uri_free (ssl_base_uri);