[libsoup] Don't crash when a request is redirected to an invalid/non-http URI

[Dan Winship <danw src gnome org>]

commit becb3a0ab8aa986f829efb77a52d53e75419c413
Author: Dan Winship <danw gnome org>
Date:   Mon Jul 27 21:27:27 2009 -0400

    Don't crash when a request is redirected to an invalid/non-http URI
    
    Make redirect_handler() reject such responses, and also make soup-uri
    g_return_if_fail rather than crashing. Add a regression test.
    
    http://bugzilla.gnome.org/show_bug.cgi?id=528882

 libsoup/soup-session.c |    2 +-
 libsoup/soup-uri.c     |    5 +++
 tests/redirect-test.c  |   68 +++++++++++++++++++++++++++++------------------
 3 files changed, 48 insertions(+), 27 deletions(-)
---
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index 3c824d5..824b010 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -887,7 +887,7 @@ redirect_handler (SoupMessage *msg, gpointer user_data)
 	 * are lame, so we use soup_uri_new_with_base().
 	 */
 	new_uri = soup_uri_new_with_base (soup_message_get_uri (msg), new_loc);
-	if (!new_uri) {
+	if (!new_uri || !new_uri->host) {
 		soup_message_set_status_full (msg,
 					      SOUP_STATUS_MALFORMED,
 					      "Invalid Redirect URL");
diff --git a/libsoup/soup-uri.c b/libsoup/soup-uri.c
index 72a900d..876b175 100644
--- a/libsoup/soup-uri.c
+++ b/libsoup/soup-uri.c
@@ -965,6 +965,8 @@ soup_uri_host_hash (gconstpointer key)
 {
 	const SoupURI *uri = key;
 
+	g_return_val_if_fail (uri != NULL && uri->host != NULL, 0);
+
 	return GPOINTER_TO_UINT (uri->scheme) + uri->port +
 		soup_str_case_hash (uri->host);
 }
@@ -987,6 +989,9 @@ soup_uri_host_equal (gconstpointer v1, gconstpointer v2)
 	const SoupURI *one = v1;
 	const SoupURI *two = v2;
 
+	g_return_val_if_fail (one != NULL && two != NULL, one == two);
+	g_return_val_if_fail (one->host != NULL && two->host != NULL, one->host == two->host);
+
 	if (one->scheme != two->scheme)
 		return FALSE;
 	if (one->port != two->port)
diff --git a/tests/redirect-test.c b/tests/redirect-test.c
index 9e21bd1..20e9d48 100644
--- a/tests/redirect-test.c
+++ b/tests/redirect-test.c
@@ -22,85 +22,89 @@ typedef struct {
 
 static struct {
 	TestRequest requests[3];
+	guint final_status;
 } tests[] = {
 	/* A redirecty response to a GET or HEAD should cause a redirect */
 
 	{ { { "GET", "/301", 301 },
 	    { "GET", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "GET", "/302", 302 },
 	    { "GET", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "GET", "/303", 303 },
 	    { "GET", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "GET", "/307", 307 },
 	    { "GET", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "HEAD", "/301", 301 },
 	    { "HEAD", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "HEAD", "/302", 302 },
 	    { "HEAD", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	/* 303 is a nonsensical response to HEAD, so we don't care
 	 * what happens there.
 	 */
 	{ { { "HEAD", "/307", 307 },
 	    { "HEAD", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 
 	/* A non-redirecty response to a GET or HEAD should not */
 
 	{ { { "GET", "/300", 300 },
-	    { NULL } } },
+	    { NULL } }, 300 },
 	{ { { "GET", "/304", 304 },
-	    { NULL } } },
+	    { NULL } }, 304 },
 	{ { { "GET", "/305", 305 },
-	    { NULL } } },
+	    { NULL } }, 305 },
 	{ { { "GET", "/306", 306 },
-	    { NULL } } },
+	    { NULL } }, 306 },
 	{ { { "GET", "/308", 308 },
-	    { NULL } } },
+	    { NULL } }, 308 },
 	{ { { "HEAD", "/300", 300 },
-	    { NULL } } },
+	    { NULL } }, 300 },
 	{ { { "HEAD", "/304", 304 },
-	    { NULL } } },
+	    { NULL } }, 304 },
 	{ { { "HEAD", "/305", 305 },
-	    { NULL } } },
+	    { NULL } }, 305 },
 	{ { { "HEAD", "/306", 306 },
-	    { NULL } } },
+	    { NULL } }, 306 },
 	{ { { "HEAD", "/308", 308 },
-	    { NULL } } },
+	    { NULL } }, 308 },
 	
 	/* Test double-redirect */
 
 	{ { { "GET", "/301/302", 301 },
 	    { "GET", "/302", 302 },
-	    { "GET", "/", 200 } } },
+	    { "GET", "/", 200 } }, 200 },
 	{ { { "HEAD", "/301/302", 301 },
 	    { "HEAD", "/302", 302 },
-	    { "HEAD", "/", 200 } } },
+	    { "HEAD", "/", 200 } }, 200 },
 
 	/* POST should only automatically redirect on 301, 302 and 303 */
 
 	{ { { "POST", "/301", 301 },
 	    { "GET", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "POST", "/302", 302 },
 	    { "GET", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "POST", "/303", 303 },
 	    { "GET", "/", 200 },
-	    { NULL } } },
+	    { NULL } }, 200 },
 	{ { { "POST", "/307", 307 },
-	    { NULL } } },
+	    { NULL } }, 307 },
 
-	/* Test behavior with recoverably-bad Location header
-	 */
+	/* Test behavior with recoverably-bad Location header */
 	{ { { "GET", "/bad", 302 },
 	    { "GET", "/bad%20with%20spaces", 200 },
-	    { NULL } } }
+	    { NULL } }, 200 },
+
+	/* Test behavior with irrecoverably-bad Location header */
+	{ { { "GET", "/bad-no-host", 302 },
+	    { NULL } }, SOUP_STATUS_MALFORMED }
 };
 static const int n_tests = G_N_ELEMENTS (tests);
 
@@ -183,6 +187,13 @@ do_test (SoupSession *session, SoupURI *base_uri, int n)
 			  G_CALLBACK (restarted), &req);
 
 	soup_session_send_message (session, msg);
+
+	if (msg->status_code != tests[n].final_status) {
+		debug_printf (1, "    - Expected final status of %d, got %d !\n",
+			      tests[n].final_status, msg->status_code);
+		errors++;
+	}
+
 	g_object_unref (msg);
 	debug_printf (2, "\n");
 }
@@ -220,6 +231,11 @@ server_callback (SoupServer *server, SoupMessage *msg,
 			soup_message_headers_replace (msg->response_headers,
 						      "Location",
 						      "/bad with spaces");
+		} else if (!strcmp (path, "/bad-no-host")) {
+			soup_message_set_status (msg, SOUP_STATUS_FOUND);
+			soup_message_headers_replace (msg->response_headers,
+						      "Location",
+						      "about:blank");
 		} else if (!strcmp (path, "/bad with spaces"))
 			soup_message_set_status (msg, SOUP_STATUS_OK);
 		else