libsoup r1069 - in trunk: . libsoup tests

[danw svn gnome org]

Author: danw
Date: Sun Feb  3 01:19:59 2008
New Revision: 1069
URL: http://svn.gnome.org/viewvc/libsoup?rev=1069&view=rev

Log:
	* libsoup/soup-session.c (redirect_handler): Misc fixes: don't
	redirect on "300 Multiple Choices", "304 Not Modified", "305 Use
	Proxy", or any unrecognized status code. Don't redirect unsafe
	methods on 301, 302, or 307. Redirect POST to GET on 303.

	* tests/redirect-test.c: test of redirection handling behavior.


Added:
   trunk/tests/redirect-test.c
Modified:
   trunk/ChangeLog
   trunk/libsoup/soup-session.c
   trunk/tests/   (props changed)
   trunk/tests/Makefile.am

Modified: trunk/libsoup/soup-session.c
==============================================================================
--- trunk/libsoup/soup-session.c	(original)
+++ trunk/libsoup/soup-session.c	Sun Feb  3 01:19:59 2008
@@ -563,12 +563,46 @@
 	const char *new_loc;
 	SoupURI *new_uri;
 
-	if (!SOUP_STATUS_IS_REDIRECTION (msg->status_code))
-		return;
-
 	new_loc = soup_message_headers_get (msg->response_headers, "Location");
-	if (!new_loc)
+	g_return_if_fail (new_loc != NULL);
+
+	if (msg->status_code == SOUP_STATUS_MOVED_PERMANENTLY ||
+	    msg->status_code == SOUP_STATUS_FOUND ||
+	    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)
+			return;
+	} else if (msg->status_code == SOUP_STATUS_SEE_OTHER) {
+		/* Redirect using a GET */
+		g_object_set (msg,
+			      SOUP_MESSAGE_METHOD, SOUP_METHOD_GET,
+			      NULL);
+		soup_message_set_request (msg, NULL,
+					  SOUP_MEMORY_STATIC, NULL, 0);
+		soup_message_headers_set_encoding (msg->request_headers,
+						   SOUP_ENCODING_NONE);
+	} else {
+		/* Three possibilities:
+		 *
+		 *   1) This was a non-3xx response that happened to
+		 *      have a "Location" header
+		 *   2) It's a non-redirecty 3xx response (300, 304,
+		 *      305, 306)
+		 *   3) It's some newly-defined 3xx response (308+)
+		 *
+		 * We ignore all of these cases. In the first two,
+		 * redirecting would be explicitly wrong, and in the
+		 * last case, we have no clue if the 3xx response is
+		 * supposed to be redirecty or non-redirecty. Plus,
+		 * 2616 says unrecognized status codes should be
+		 * treated as the equivalent to the x00 code, and we
+		 * don't redirect on 300, so therefore we shouldn't
+		 * redirect on 308+ either.
+		 */
 		return;
+	}
 
 	/* Location is supposed to be an absolute URI, but some sites
 	 * are lame, so we use soup_uri_new_with_base().

Modified: trunk/tests/Makefile.am
==============================================================================
--- trunk/tests/Makefile.am	(original)
+++ trunk/tests/Makefile.am	Sun Feb  3 01:19:59 2008
@@ -17,6 +17,7 @@
 	getbug		\
 	header-parsing  \
 	ntlm-test	\
+	redirect-test	\
 	simple-httpd	\
 	simple-proxy	\
 	uri-parsing	\
@@ -38,6 +39,7 @@
 ntlm_test_SOURCES = ntlm-test.c $(TEST_SRCS)
 proxy_test_SOURCES = proxy-test.c $(TEST_SRCS)
 pull_api_SOURCES = pull-api.c $(TEST_SRCS)
+redirect_test_SOURCES = redirect-test.c $(TEST_SRCS)
 query_test_SOURCES = query-test.c $(TEST_SRCS)
 server_auth_test_SOURCES = server-auth-test.c $(TEST_SRCS)
 simple_httpd_SOURCES = simple-httpd.c

Added: trunk/tests/redirect-test.c
==============================================================================
--- (empty file)
+++ trunk/tests/redirect-test.c	Sun Feb  3 01:19:59 2008
@@ -0,0 +1,266 @@
+/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */
+/*
+ * Copyright (C) 2008 Red Hat, Inc.
+ */
+
+#include "config.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <glib.h>
+#include <libsoup/soup.h>
+
+#include "test-utils.h"
+
+GMainLoop *loop;
+
+typedef struct {
+	const char *method;
+	const char *path;
+	guint status_code;
+} TestRequest;
+
+struct {
+	TestRequest requests[3];
+} tests[] = {
+	/* A redirecty response to a GET should could a redirect */
+
+	{ { { "GET", "/301", 301 },
+	    { "GET", "/", 200 },
+	    { NULL } } },
+	{ { { "GET", "/302", 302 },
+	    { "GET", "/", 200 },
+	    { NULL } } },
+	{ { { "GET", "/303", 303 },
+	    { "GET", "/", 200 },
+	    { NULL } } },
+	{ { { "GET", "/307", 307 },
+	    { "GET", "/", 200 },
+	    { NULL } } },
+
+	/* A non-redirecty response to a GET should not */
+
+	{ { { "GET", "/300", 300 },
+	    { NULL } } },
+	{ { { "GET", "/304", 304 },
+	    { NULL } } },
+	{ { { "GET", "/305", 305 },
+	    { NULL } } },
+	{ { { "GET", "/306", 306 },
+	    { NULL } } },
+	{ { { "GET", "/308", 308 },
+	    { NULL } } },
+	
+	/* Test double-redirect */
+
+	{ { { "GET", "/301/302", 301 },
+	    { "GET", "/302", 302 },
+	    { "GET", "/", 200 } } },
+
+	/* POST should only automatically redirect on 303 */
+
+	{ { { "POST", "/301", 301 },
+	    { NULL } } },
+	{ { { "POST", "/302", 302 },
+	    { NULL } } },
+	{ { { "POST", "/303", 303 },
+	    { "GET", "/", 200 },
+	    { NULL } } },
+	{ { { "POST", "/307", 307 },
+	    { NULL } } }
+};
+static const int n_tests = G_N_ELEMENTS (tests);
+
+static void
+got_headers (SoupMessage *msg, gpointer user_data)
+{
+	TestRequest **req = user_data;
+	const char *location;
+
+	debug_printf (2, "    -> %d %s\n", msg->status_code,
+		      msg->reason_phrase);
+	location = soup_message_headers_get (msg->response_headers, "Location");
+	if (location)
+		debug_printf (2, "       Location: %s\n", location);
+
+	if (!(*req)->method)
+		return;
+
+	if (msg->status_code != (*req)->status_code) {
+		debug_printf (1, "    - Expected %d !\n",
+			      (*req)->status_code);
+		errors++;
+	}
+}
+
+static void
+restarted (SoupMessage *msg, gpointer user_data)
+{
+	TestRequest **req = user_data;
+	SoupURI *uri = soup_message_get_uri (msg);
+
+	debug_printf (2, "    %s %s\n", msg->method, uri->path);
+
+	if ((*req)->method)
+		(*req)++;
+
+	if (!(*req)->method) {
+		debug_printf (1, "    - Expected to be done!\n");
+		errors++;
+		return;
+	}
+
+	if (strcmp (msg->method, (*req)->method) != 0) {
+		debug_printf (1, "    - Expected %s !\n", (*req)->method);
+		errors++;
+	}
+	if (strcmp (uri->path, (*req)->path) != 0) {
+		debug_printf (1, "    - Expected %s !\n", (*req)->path);
+		errors++;
+	}
+}
+
+static void
+do_test (SoupSession *session, SoupURI *base_uri, int n)
+{
+	SoupURI *uri;
+	SoupMessage *msg;
+	TestRequest *req;
+
+	debug_printf (1, "%2d. %s %s\n", n + 1,
+		      tests[n].requests[0].method,
+		      tests[n].requests[0].path);
+
+	uri = soup_uri_new_with_base (base_uri, tests[n].requests[0].path);
+	msg = soup_message_new_from_uri (tests[n].requests[0].method, uri);
+	soup_uri_free (uri);
+
+	if (msg->method == SOUP_METHOD_POST) {
+		soup_message_set_request (msg, "text/plain",
+					  SOUP_MEMORY_STATIC,
+					  "post body",
+					  strlen ("post body"));
+	}
+
+	req = &tests[n].requests[0];
+	g_signal_connect (msg, "got_headers",
+			  G_CALLBACK (got_headers), &req);
+	g_signal_connect (msg, "restarted",
+			  G_CALLBACK (restarted), &req);
+
+	soup_session_send_message (session, msg);
+	g_object_unref (msg);
+	debug_printf (2, "\n");
+}
+
+static void
+do_redirect_tests (SoupURI *base_uri)
+{
+	SoupSession *session;
+	int n;
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+	debug_printf (1, "Async session\n");
+	for (n = 0; n < n_tests; n++)
+		do_test (session, base_uri, n);
+	soup_session_abort (session);
+	g_object_unref (session);
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION_SYNC, NULL);
+	debug_printf (1, "Sync session\n");
+	for (n = 0; n < n_tests; n++)
+		do_test (session, base_uri, n);
+	soup_session_abort (session);
+	g_object_unref (session);
+}
+
+GThread *server_thread;
+
+static void
+server_callback (SoupServer *server, SoupMessage *msg,
+		 const char *path, GHashTable *query,
+		 SoupClientContext *context, gpointer data)
+{
+	char *remainder;
+	guint status_code;
+
+	if (!strcmp (path, "/")) {
+		if (msg->method != SOUP_METHOD_GET) {
+			soup_message_set_status (msg, SOUP_STATUS_METHOD_NOT_ALLOWED);
+			return;
+		}
+
+		/* Make sure that redirecting a POST clears the body */
+		if (msg->request_body->length) {
+			soup_message_set_status (msg, SOUP_STATUS_BAD_REQUEST);
+			return;
+		}
+
+		soup_message_set_status (msg, SOUP_STATUS_OK);
+		soup_message_set_response (msg, "text/plain",
+					   SOUP_MEMORY_STATIC,
+					   "OK\r\n", 4);
+		return;
+	}
+
+	status_code = strtoul (path + 1, &remainder, 10);
+	if (!SOUP_STATUS_IS_REDIRECTION (status_code) ||
+	    (*remainder && *remainder != '/')) {
+		soup_message_set_status (msg, SOUP_STATUS_NOT_FOUND);
+		return;
+	}
+
+	soup_message_set_status (msg, status_code);
+	if (*remainder) {
+		soup_message_headers_replace (msg->response_headers,
+					      "Location", remainder);
+	} else {
+		soup_message_headers_replace (msg->response_headers,
+					      "Location", "/");
+	}
+}
+
+gboolean run_tests = TRUE;
+
+static GOptionEntry no_test_entry[] = {
+        { "no-tests", 'n', G_OPTION_FLAG_NO_ARG | G_OPTION_FLAG_REVERSE,
+          G_OPTION_ARG_NONE, &run_tests,
+          "Don't run tests, just run the test server", NULL },
+        { NULL }
+};
+
+int
+main (int argc, char **argv)
+{
+	GMainLoop *loop;
+	SoupServer *server;
+	guint port;
+	SoupURI *base_uri;
+
+	test_init (argc, argv, no_test_entry);
+
+	server = soup_test_server_new (TRUE);
+	soup_server_add_handler (server, NULL,
+				 server_callback, NULL, NULL);
+	port = 	soup_server_get_port (server);
+
+	loop = g_main_loop_new (NULL, TRUE);
+
+	if (run_tests) {
+		base_uri = soup_uri_new ("http://localhost";);
+		soup_uri_set_port (base_uri, port);
+		do_redirect_tests (base_uri);
+		soup_uri_free (base_uri);
+	} else {
+		printf ("Listening on port %d\n", port);
+		g_main_loop_run (loop);
+	}
+
+	g_main_loop_unref (loop);
+
+	if (run_tests)
+		test_cleanup ();
+	return errors != 0;
+}