Fix for proxies that close the connection when 407'ing a CONNECT

If we try to CONNECT through a proxy, and it returns a "407 Proxy
Authentication Required" but then closes the connection, we have to
create a new connection before we can try CONNECTing again. The old
soup-connection.c tunnel code did this, but it got accidentally lost
in the soup-session.c version.

Fix up tests/proxy-test to test both the connection-close and the
persistent-connection cases.

This was the underlying bug in
https://bugzilla.gnome.org/show_bug.cgi?id=611663, and is at least
part of https://bugzilla.gnome.org/show_bug.cgi?id=611539.

diff --git a/libsoup/soup-session-async.c b/libsoup/soup-session-async.c
index 9416a7a..53e1c29 100644 (file)
--- a/libsoup/soup-session-async.c
+++ b/libsoup/soup-session-async.c
@@ -221,11 +221,6 @@ tunnel_connected (SoupMessage *msg, gpointer user_data)
 {
        SoupSessionAsyncTunnelData *data = user_data;
 
-       if (SOUP_MESSAGE_IS_STARTING (msg)) {
-               soup_session_send_queue_item (data->session, data->item, data->conn);
-               return;
-       }
-
        if (!SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)) {
                soup_session_connection_failed (data->session, data->conn,
                                                msg->status_code);
@@ -242,14 +237,22 @@ tunnel_connected (SoupMessage *msg, gpointer user_data)
                          G_CALLBACK (connection_closed), data->session);
        soup_connection_set_state (data->conn, SOUP_CONNECTION_IDLE);
 
-       do_idle_run_queue (data->session);
-
 done:
+       do_idle_run_queue (data->session);
        soup_message_queue_item_unref (data->item);
        g_slice_free (SoupSessionAsyncTunnelData, data);
 }
 
 static void
+tunnel_connect_restarted (SoupMessage *msg, gpointer user_data)
+{
+       SoupSessionAsyncTunnelData *data = user_data;
+
+       if (SOUP_MESSAGE_IS_STARTING (msg))
+               soup_session_send_queue_item (data->session, data->item, data->conn);
+}
+
+static void
 got_connection (SoupConnection *conn, guint status, gpointer session)
 {
        SoupAddress *tunnel_addr;
@@ -278,7 +281,7 @@ got_connection (SoupConnection *conn, guint status, gpointer session)
                g_signal_connect (data->item->msg, "finished",
                                  G_CALLBACK (tunnel_connected), data);
                g_signal_connect (data->item->msg, "restarted",
-                                 G_CALLBACK (tunnel_connected), data);
+                                 G_CALLBACK (tunnel_connect_restarted), data);
                soup_session_send_queue_item (session, data->item, conn);
                return;
        }

diff --git a/libsoup/soup-session-sync.c b/libsoup/soup-session-sync.c
index e1ba325..100ce38 100644 (file)
--- a/libsoup/soup-session-sync.c
+++ b/libsoup/soup-session-sync.c
@@ -135,13 +135,15 @@ soup_session_sync_new_with_options (const char *optname1, ...)
        return session;
 }
 
-static gboolean
+static guint
 tunnel_connect (SoupSession *session, SoupConnection *conn,
                SoupAddress *tunnel_addr)
 {
        SoupMessageQueueItem *item;
        guint status;
 
+       g_object_ref (conn);
+
        g_signal_emit_by_name (session, "tunneling", conn);
        item = soup_session_make_connect_message (session, tunnel_addr);
        do
@@ -156,12 +158,11 @@ tunnel_connect (SoupSession *session, SoupConnection *conn,
                        status = SOUP_STATUS_SSL_FAILED;
        }
 
-       if (SOUP_STATUS_IS_SUCCESSFUL (status))
-               return TRUE;
-       else {
+       if (!SOUP_STATUS_IS_SUCCESSFUL (status))
                soup_session_connection_failed (session, conn, status);
-               return FALSE;
-       }
+
+       g_object_unref (conn);
+       return status;
 }
 
 static SoupConnection *
@@ -222,8 +223,11 @@ wait_for_connection (SoupMessageQueueItem *item)
                                soup_connection_disconnect (conn);
                                conn = NULL;
                        } else if ((tunnel_addr = soup_connection_get_tunnel_addr (conn))) {
-                               if (!tunnel_connect (session, conn, tunnel_addr))
+                               status = tunnel_connect (session, conn, tunnel_addr);
+                               if (!SOUP_STATUS_IS_SUCCESSFUL (status)) {
                                        conn = NULL;
+                                       goto try_again;
+                               }
                         }
                }
 

diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index 4068804..46dd8ee 100644 (file)
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -1271,6 +1271,9 @@ soup_session_connection_failed (SoupSession *session,
                        return;
        }
 
+       if (status == SOUP_STATUS_TRY_AGAIN)
+               return;
+
        /* Cancel any other messages waiting for a connection to it,
         * since they're out of luck.
         */
@@ -1304,6 +1307,31 @@ tunnel_connected (SoupMessage *msg, gpointer user_data)
        }
 }
 
+static void
+tunnel_connect_restarted (SoupMessage *msg, gpointer session)
+{
+       SoupSessionPrivate *priv = SOUP_SESSION_GET_PRIVATE (session);
+       SoupMessageQueueItem *item;
+
+       if (msg->status_code != SOUP_STATUS_PROXY_AUTHENTICATION_REQUIRED)
+               return;
+
+       item = soup_message_queue_lookup (priv->queue, msg);
+       if (!item)
+               return;
+       if (soup_connection_get_state (item->conn) == SOUP_CONNECTION_DISCONNECTED) {
+               /* We got a 407, and the session provided auth and
+                * restarted the message, but the proxy closed the
+                * connection, so we need to create a new one. The
+                * easiest way to do this is to just give up on the
+                * current msg and conn, and re-run the queue.
+                */
+               soup_session_cancel_message (session, msg,
+                                            SOUP_STATUS_TRY_AGAIN);
+       }
+       soup_message_queue_item_unref (item);
+}
+
 SoupMessageQueueItem *
 soup_session_make_connect_message (SoupSession *session,
                                   SoupAddress *server_addr)
@@ -1330,6 +1358,8 @@ soup_session_make_connect_message (SoupSession *session,
         */
        g_signal_connect (msg, "finished",
                          G_CALLBACK (tunnel_connected), session);
+       g_signal_connect (msg, "restarted",
+                         G_CALLBACK (tunnel_connect_restarted), session);
        queue_message (session, msg, NULL, NULL);
        item = soup_message_queue_lookup (priv->queue, msg);
        g_object_unref (msg);

diff --git a/libsoup/soup-status.c b/libsoup/soup-status.c
index 11422ac..2fa309a 100644 (file)
--- a/libsoup/soup-status.c
+++ b/libsoup/soup-status.c
@@ -77,7 +77,7 @@
  * @SOUP_STATUS_IO_ERROR: A network error occurred, or the other end
  * closed the connection unexpectedly
  * @SOUP_STATUS_MALFORMED: Malformed data (usually a programmer error)
- * @SOUP_STATUS_TRY_AGAIN: Formerly used internally. Now unused.
+ * @SOUP_STATUS_TRY_AGAIN: Used internally
  * @SOUP_STATUS_CONTINUE: 100 Continue (HTTP)
  * @SOUP_STATUS_SWITCHING_PROTOCOLS: 101 Switching Protocols (HTTP)
  * @SOUP_STATUS_PROCESSING: 102 Processing (WebDAV)

diff --git a/tests/proxy-test.c b/tests/proxy-test.c
index d0c348e..d23e7d1 100644 (file)
--- a/tests/proxy-test.c
+++ b/tests/proxy-test.c
@@ -69,13 +69,29 @@ authenticate (SoupSession *session, SoupMessage *msg,
 }
 
 static void
-test_url (const char *url, int proxy, guint expected, gboolean sync)
+set_close_on_connect (SoupSession *session, SoupMessage *msg,
+                     SoupSocket *sock, gpointer user_data)
+{
+       /* This is used to test that we can handle the server closing
+        * the connection when returning a 407 in response to a
+        * CONNECT. (Rude!)
+        */
+       if (msg->method == SOUP_METHOD_CONNECT) {
+               soup_message_headers_append (msg->request_headers,
+                                            "Connection", "close");
+       }
+}
+
+static void
+test_url (const char *url, int proxy, guint expected,
+         gboolean sync, gboolean close)
 {
        SoupSession *session;
        SoupURI *proxy_uri;
        SoupMessage *msg;
 
-       debug_printf (1, "  GET %s via %s\n", url, proxy_names[proxy]);
+       debug_printf (1, "  GET %s via %s%s\n", url, proxy_names[proxy],
+                     close ? " (with Connection: close)" : "");
        if (proxy == UNAUTH_PROXY && expected != SOUP_STATUS_FORBIDDEN)
                expected = SOUP_STATUS_PROXY_UNAUTHORIZED;
 
@@ -89,6 +105,10 @@ test_url (const char *url, int proxy, guint expected, gboolean sync)
        soup_uri_free (proxy_uri);
        g_signal_connect (session, "authenticate",
                          G_CALLBACK (authenticate), NULL);
+       if (close) {
+               g_signal_connect (session, "request-started",
+                                 G_CALLBACK (set_close_on_connect), NULL);
+       }
 
        msg = soup_message_new (SOUP_METHOD_GET, url);
        if (!msg) {
@@ -124,17 +144,18 @@ run_test (int i, gboolean sync)
                http_url = g_strconcat (HTTP_SERVER, tests[i].url, NULL);
                https_url = g_strconcat (HTTPS_SERVER, tests[i].url, NULL);
        }
-       test_url (http_url, SIMPLE_PROXY, tests[i].final_status, sync);
+       test_url (http_url, SIMPLE_PROXY, tests[i].final_status, sync, FALSE);
 #ifdef HAVE_SSL
-       test_url (https_url, SIMPLE_PROXY, tests[i].final_status, sync);
+       test_url (https_url, SIMPLE_PROXY, tests[i].final_status, sync, FALSE);
 #endif
-       test_url (http_url, AUTH_PROXY, tests[i].final_status, sync);
+       test_url (http_url, AUTH_PROXY, tests[i].final_status, sync, FALSE);
 #ifdef HAVE_SSL
-       test_url (https_url, AUTH_PROXY, tests[i].final_status, sync);
+       test_url (https_url, AUTH_PROXY, tests[i].final_status, sync, FALSE);
+       test_url (https_url, AUTH_PROXY, tests[i].final_status, sync, TRUE);
 #endif
-       test_url (http_url, UNAUTH_PROXY, tests[i].final_status, sync);
+       test_url (http_url, UNAUTH_PROXY, tests[i].final_status, sync, FALSE);
 #ifdef HAVE_SSL
-       test_url (https_url, UNAUTH_PROXY, tests[i].final_status, sync);
+       test_url (https_url, UNAUTH_PROXY, tests[i].final_status, sync, FALSE);
 #endif
 
        g_free (http_url);