[libsoup] SoupCookie: ignore values on httponly and secure attributes

[Dan Winship <danw src gnome org>]

commit 48feb24ee7a73e7e964f1f185bdc1947d757e10e
Author: Dan Winship <danw gnome org>
Date:   Tue Jul 10 20:35:08 2012 -0400

    SoupCookie: ignore values on httponly and secure attributes
    
    The cookie parsing algorithm says to ignore values on attributes that
    aren't supposed to have them, so do that. Also add a test for this.
    Based on a patch from "Basavaraj".
    
    Also, drive-by bugfix in soup_cookie_to_set_cookie_header() /
    soup_cookies_to_response() (the SoupServer side of the cookie APIs),
    which were outputting "HttpOnly" if the cookie had the "secure" flag
    set, rather than the "http_only" flag.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=678753

 libsoup/soup-cookie.c |   26 +++++++----
 tests/cookies-test.c  |  112 ++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 127 insertions(+), 11 deletions(-)
---
diff --git a/libsoup/soup-cookie.c b/libsoup/soup-cookie.c
old mode 100644
new mode 100755
index b10eba8..6f05f35
--- a/libsoup/soup-cookie.c
+++ b/libsoup/soup-cookie.c
@@ -176,7 +176,7 @@ unskip_lws (const char *s, const char *start)
 #define is_value_ender(ch) ((ch) < ' ' || (ch) == ';')
 
 static char *
-parse_value (const char **val_p)
+parse_value (const char **val_p, gboolean copy)
 {
 	const char *start, *end, *p;
 	char *value;
@@ -188,7 +188,11 @@ parse_value (const char **val_p)
 	for (p = start; !is_value_ender (*p); p++)
 		;
 	end = unskip_lws (p, start);
-	value = g_strndup (start, end - start);
+
+	if (copy)
+		value = g_strndup (start, end - start);
+	else
+		value = NULL;
 
 	*val_p = p;
 	return value;
@@ -200,7 +204,7 @@ parse_date (const char **val_p)
 	char *value;
 	SoupDate *date;
 
-	value = parse_value (val_p);
+	value = parse_value (val_p, TRUE);
 	date = soup_date_new_from_string (value);
 	g_free (value);
 	return date;
@@ -233,7 +237,7 @@ parse_one_cookie (const char *header, SoupURI *origin)
 	}
 
 	/* Parse the VALUE */
-	cookie->value = parse_value (&p);
+	cookie->value = parse_value (&p, TRUE);
 
 	/* Parse attributes */
 	while (*p == ';') {
@@ -246,7 +250,7 @@ parse_one_cookie (const char *header, SoupURI *origin)
 #define MATCH_NAME(name) ((end - start == strlen (name)) && !g_ascii_strncasecmp (start, name, end - start))
 
 		if (MATCH_NAME ("domain") && has_value) {
-			cookie->domain = parse_value (&p);
+			cookie->domain = parse_value (&p, TRUE);
 			if (!*cookie->domain) {
 				g_free (cookie->domain);
 				cookie->domain = NULL;
@@ -255,8 +259,10 @@ parse_one_cookie (const char *header, SoupURI *origin)
 			cookie->expires = parse_date (&p);
 		} else if (MATCH_NAME ("httponly")) {
 			cookie->http_only = TRUE;
+			if (has_value)
+				parse_value (&p, FALSE);
 		} else if (MATCH_NAME ("max-age") && has_value) {
-			char *max_age_str = parse_value (&p), *mae;
+			char *max_age_str = parse_value (&p, TRUE), *mae;
 			long max_age = strtol (max_age_str, &mae, 10);
 			if (!*mae) {
 				if (max_age < 0)
@@ -265,19 +271,21 @@ parse_one_cookie (const char *header, SoupURI *origin)
 			}
 			g_free (max_age_str);
 		} else if (MATCH_NAME ("path") && has_value) {
-			cookie->path = parse_value (&p);
+			cookie->path = parse_value (&p, TRUE);
 			if (*cookie->path != '/') {
 				g_free (cookie->path);
 				cookie->path = NULL;
 			}
 		} else if (MATCH_NAME ("secure")) {
 			cookie->secure = TRUE;
+			if (has_value)
+				parse_value (&p, FALSE);
 		} else {
 			/* Ignore unknown attributes, but we still have
 			 * to skip over the value.
 			 */
 			if (has_value)
-				g_free (parse_value (&p));
+				parse_value (&p, FALSE);
 		}
 	}
 
@@ -772,7 +780,7 @@ serialize_cookie (SoupCookie *cookie, GString *header, gboolean set_cookie)
 	}
 	if (cookie->secure)
 		g_string_append (header, "; secure");
-	if (cookie->secure)
+	if (cookie->http_only)
 		g_string_append (header, "; HttpOnly");
 }
 
diff --git a/tests/cookies-test.c b/tests/cookies-test.c
index e233372..87f4673 100644
--- a/tests/cookies-test.c
+++ b/tests/cookies-test.c
@@ -3,6 +3,8 @@
  * Copyright (C) 2010 Igalia S.L.
  */
 
+#include <string.h>
+
 #include <glib.h>
 #include <libsoup/soup.h>
 
@@ -26,8 +28,13 @@ server_callback (SoupServer *server, SoupMessage *msg,
 		soup_message_headers_replace (msg->response_headers,
 					      "Set-Cookie",
 					      "baz=qux");
-	} else
-		g_return_if_reached ();
+	} else if (soup_message_headers_get_one (msg->request_headers,
+						 "Echo-Set-Cookie")) {
+		soup_message_headers_replace (msg->response_headers,
+					      "Set-Cookie",
+					      soup_message_headers_get_one (msg->request_headers,
+									    "Echo-Set-Cookie"));
+	}
 
 	soup_message_set_status (msg, SOUP_STATUS_OK);
 }
@@ -53,6 +60,8 @@ do_cookies_accept_policy_test (void)
 	GSList *l, *p;
 	int i;
 
+	debug_printf (1, "SoupCookieJarAcceptPolicy test\n");
+
 	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
 	soup_session_add_feature_by_type (session, SOUP_TYPE_COOKIE_JAR);
 	jar = SOUP_COOKIE_JAR (soup_session_get_feature (session, SOUP_TYPE_COOKIE_JAR));
@@ -98,6 +107,104 @@ do_cookies_accept_policy_test (void)
 	soup_test_session_abort_unref (session);
 }
 
+/* FIXME: moar tests! */
+static void
+do_cookies_parsing_test (void)
+{
+	SoupSession *session;
+	SoupMessage *msg;
+	SoupCookieJar *jar;
+	GSList *cookies, *iter;
+	SoupCookie *cookie;
+	gboolean got1, got2, got3;
+
+	debug_printf (1, "\nSoupCookie parsing test\n");
+
+	session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
+	soup_session_add_feature_by_type (session, SOUP_TYPE_COOKIE_JAR);
+	jar = SOUP_COOKIE_JAR (soup_session_get_feature (session, SOUP_TYPE_COOKIE_JAR));
+
+	/* "httponly" is case-insensitive, and its value (if any) is ignored */
+	msg = soup_message_new_from_uri ("GET", first_party_uri);
+	soup_message_headers_append (msg->request_headers, "Echo-Set-Cookie",
+				     "one=1; httponly; max-age=100");
+	soup_session_send_message (session, msg);
+	g_object_unref (msg);
+
+	msg = soup_message_new_from_uri ("GET", first_party_uri);
+	soup_message_headers_append (msg->request_headers, "Echo-Set-Cookie",
+				     "two=2; HttpOnly; max-age=100");
+	soup_session_send_message (session, msg);
+	g_object_unref (msg);
+
+	msg = soup_message_new_from_uri ("GET", first_party_uri);
+	soup_message_headers_append (msg->request_headers, "Echo-Set-Cookie",
+				     "three=3; httpONLY=Wednesday; max-age=100");
+	soup_session_send_message (session, msg);
+	g_object_unref (msg);
+
+	cookies = soup_cookie_jar_get_cookie_list (jar, first_party_uri, TRUE);
+	got1 = got2 = got3 = FALSE;
+
+	for (iter = cookies; iter; iter = iter->next) {
+		cookie = iter->data;
+
+		if (!strcmp (soup_cookie_get_name (cookie), "one")) {
+			got1 = TRUE;
+			if (!soup_cookie_get_http_only (cookie)) {
+				debug_printf (1, "  cookie 1 is not HttpOnly!\n");
+				errors++;
+			}
+			if (!soup_cookie_get_expires (cookie)) {
+				debug_printf (1, "  cookie 1 did not fully parse!\n");
+				errors++;
+			}
+		} else if (!strcmp (soup_cookie_get_name (cookie), "two")) {
+			got2 = TRUE;
+			if (!soup_cookie_get_http_only (cookie)) {
+				debug_printf (1, "  cookie 2 is not HttpOnly!\n");
+				errors++;
+			}
+			if (!soup_cookie_get_expires (cookie)) {
+				debug_printf (1, "  cookie 3 did not fully parse!\n");
+				errors++;
+			}
+		} else if (!strcmp (soup_cookie_get_name (cookie), "three")) {
+			got3 = TRUE;
+			if (!soup_cookie_get_http_only (cookie)) {
+				debug_printf (1, "  cookie 3 is not HttpOnly!\n");
+				errors++;
+			}
+			if (!soup_cookie_get_expires (cookie)) {
+				debug_printf (1, "  cookie 3 did not fully parse!\n");
+				errors++;
+			}
+		} else {
+			debug_printf (1, "  got unexpected cookie '%s'\n",
+				      soup_cookie_get_name (cookie));
+			errors++;
+		}
+
+		soup_cookie_free (cookie);
+	}
+	g_slist_free (cookies);
+
+	if (!got1) {
+		debug_printf (1, "  didn't get cookie 1\n");
+		errors++;
+	}
+	if (!got2) {
+		debug_printf (1, "  didn't get cookie 2\n");
+		errors++;
+	}
+	if (!got3) {
+		debug_printf (1, "  didn't get cookie 3\n");
+		errors++;
+	}
+
+	soup_test_session_abort_unref (session);
+}	
+
 int
 main (int argc, char **argv)
 {
@@ -111,6 +218,7 @@ main (int argc, char **argv)
 	soup_uri_set_port (third_party_uri, soup_server_get_port (server));
 
 	do_cookies_accept_policy_test ();
+	do_cookies_parsing_test ();
 
 	soup_uri_free (first_party_uri);
 	soup_uri_free (third_party_uri);