a 302 response to HEAD (or any other safe method) should be treated like a

	* libsoup/soup-session.c (redirect_handler): a 302 response to
	HEAD (or any other safe method) should be treated like a 307, not
	a 303. #551190, Jonathan Matthew.

	* tests/redirect-test.c: test that

svn path=/trunk/; revision=1158

diff --git a/ChangeLog b/ChangeLog
index 3c56c60..0fb16a9 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2008-09-07  Dan Winship  <danw@gnome.org>
+
+       * libsoup/soup-session.c (redirect_handler): a 302 response to
+       HEAD (or any other safe method) should be treated like a 307, not
+       a 303. #551190, Jonathan Matthew.
+
+       * tests/redirect-test.c: test that
+
 2008-09-01  Dan Winship  <danw@gnome.org>
 
        * configure.in: 2.23.91

diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index 1d69646..3d80205 100644 (file)
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -762,6 +762,11 @@ auth_manager_authenticate (SoupAuthManager *manager, SoupMessage *msg,
        g_signal_emit (session, signals[AUTHENTICATE], 0, msg, auth, retrying);
 }
 
+#define SOUP_METHOD_IS_SAFE(method) (method == SOUP_METHOD_GET || \
+                                    method == SOUP_METHOD_HEAD || \
+                                    method == SOUP_METHOD_OPTIONS || \
+                                    method == SOUP_METHOD_PROPFIND)
+
 static void
 redirect_handler (SoupMessage *msg, gpointer user_data)
 {
@@ -772,16 +777,9 @@ redirect_handler (SoupMessage *msg, gpointer user_data)
        new_loc = soup_message_headers_get (msg->response_headers, "Location");
        g_return_if_fail (new_loc != NULL);
 
-       if (msg->status_code == SOUP_STATUS_MOVED_PERMANENTLY ||
-           msg->status_code == SOUP_STATUS_TEMPORARY_REDIRECT) {
-               /* Don't redirect non-safe methods */
-               if (msg->method != SOUP_METHOD_GET &&
-                   msg->method != SOUP_METHOD_HEAD &&
-                   msg->method != SOUP_METHOD_OPTIONS &&
-                   msg->method != SOUP_METHOD_PROPFIND)
-                       return;
-       } else if (msg->status_code == SOUP_STATUS_SEE_OTHER ||
-                  msg->status_code == SOUP_STATUS_FOUND) {
+       if (msg->status_code == SOUP_STATUS_SEE_OTHER ||
+           (msg->status_code == SOUP_STATUS_FOUND &&
+            !SOUP_METHOD_IS_SAFE (msg->method))) {
                /* Redirect using a GET */
                g_object_set (msg,
                              SOUP_MESSAGE_METHOD, SOUP_METHOD_GET,
@@ -790,6 +788,12 @@ redirect_handler (SoupMessage *msg, gpointer user_data)
                                          SOUP_MEMORY_STATIC, NULL, 0);
                soup_message_headers_set_encoding (msg->request_headers,
                                                   SOUP_ENCODING_NONE);
+       } else if (msg->status_code == SOUP_STATUS_MOVED_PERMANENTLY ||
+                  msg->status_code == SOUP_STATUS_TEMPORARY_REDIRECT ||
+                  msg->status_code == SOUP_STATUS_FOUND) {
+               /* Don't redirect non-safe methods */
+               if (!SOUP_METHOD_IS_SAFE (msg->method))
+                       return;
        } else {
                /* Three possibilities:
                 *

diff --git a/tests/redirect-test.c b/tests/redirect-test.c
index 904036f..048104e 100644 (file)
--- a/tests/redirect-test.c
+++ b/tests/redirect-test.c
@@ -23,7 +23,7 @@ typedef struct {
 static struct {
        TestRequest requests[3];
 } tests[] = {
-       /* A redirecty response to a GET should cause a redirect */
+       /* A redirecty response to a GET or HEAD should cause a redirect */
 
        { { { "GET", "/301", 301 },
            { "GET", "/", 200 },
@@ -37,8 +37,20 @@ static struct {
        { { { "GET", "/307", 307 },
            { "GET", "/", 200 },
            { NULL } } },
+       { { { "HEAD", "/301", 301 },
+           { "HEAD", "/", 200 },
+           { NULL } } },
+       { { { "HEAD", "/302", 302 },
+           { "HEAD", "/", 200 },
+           { NULL } } },
+       /* 303 is a nonsensical response to HEAD, so we don't care
+        * what happens there.
+        */
+       { { { "HEAD", "/307", 307 },
+           { "HEAD", "/", 200 },
+           { NULL } } },
 
-       /* A non-redirecty response to a GET should not */
+       /* A non-redirecty response to a GET or HEAD should not */
 
        { { { "GET", "/300", 300 },
            { NULL } } },
@@ -50,12 +62,25 @@ static struct {
            { NULL } } },
        { { { "GET", "/308", 308 },
            { NULL } } },
+       { { { "HEAD", "/300", 300 },
+           { NULL } } },
+       { { { "HEAD", "/304", 304 },
+           { NULL } } },
+       { { { "HEAD", "/305", 305 },
+           { NULL } } },
+       { { { "HEAD", "/306", 306 },
+           { NULL } } },
+       { { { "HEAD", "/308", 308 },
+           { NULL } } },
        
        /* Test double-redirect */
 
        { { { "GET", "/301/302", 301 },
            { "GET", "/302", 302 },
            { "GET", "/", 200 } } },
+       { { { "HEAD", "/301/302", 301 },
+           { "HEAD", "/302", 302 },
+           { "HEAD", "/", 200 } } },
 
        /* POST should only automatically redirect on 302 and 303 */
 
@@ -184,7 +209,8 @@ server_callback (SoupServer *server, SoupMessage *msg,
        guint status_code;
 
        if (!strcmp (path, "/")) {
-               if (msg->method != SOUP_METHOD_GET) {
+               if (msg->method != SOUP_METHOD_GET &&
+                   msg->method != SOUP_METHOD_HEAD) {
                        soup_message_set_status (msg, SOUP_STATUS_METHOD_NOT_ALLOWED);
                        return;
                }
@@ -204,9 +230,17 @@ server_callback (SoupServer *server, SoupMessage *msg,
                }
 
                soup_message_set_status (msg, SOUP_STATUS_OK);
-               soup_message_set_response (msg, "text/plain",
-                                          SOUP_MEMORY_STATIC,
-                                          "OK\r\n", 4);
+
+               /* FIXME: this is wrong, though it doesn't matter for
+                * the purposes of this test, and to do the right
+                * thing currently we'd have to set Content-Length by
+                * hand.
+                */
+               if (msg->method != SOUP_METHOD_HEAD) {
+                       soup_message_set_response (msg, "text/plain",
+                                                  SOUP_MEMORY_STATIC,
+                                                  "OK\r\n", 4);
+               }
                return;
        }