libsoup r1158 - in trunk: . libsoup tests

[danw svn gnome org]

Author: danw
Date: Sun Sep  7 17:43:03 2008
New Revision: 1158
URL: http://svn.gnome.org/viewvc/libsoup?rev=1158&view=rev

Log:
	* 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


Modified:
   trunk/ChangeLog
   trunk/libsoup/soup-session.c
   trunk/tests/redirect-test.c

Modified: trunk/libsoup/soup-session.c
==============================================================================
--- trunk/libsoup/soup-session.c	(original)
+++ trunk/libsoup/soup-session.c	Sun Sep  7 17:43:03 2008
@@ -762,6 +762,11 @@
 	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 @@
 	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 @@
 					  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:
 		 *

Modified: trunk/tests/redirect-test.c
==============================================================================
--- trunk/tests/redirect-test.c	(original)
+++ trunk/tests/redirect-test.c	Sun Sep  7 17:43:03 2008
@@ -23,7 +23,7 @@
 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 @@
 	{ { { "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 @@
 	    { 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 @@
 	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 @@
 		}
 
 		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;
 	}