cookies: New function init_cookie has a monopoly.

All cookies are now constructed with the new function init_cookie.
Requested by Miciah Dashiel Butler Masters.

This also fixes a bug where the "Add cookie" button left cookie->path == NULL,
causing a crash later when deciding whether to send the cookie to the server.

src/cookies/cookies.c

@@ -280,10 +280,50 @@ is_domain_security_ok(unsigned char *domain, unsigned char *server, int server_l
 	return 1;
 }
 
+/* Allocate a struct cookie and initialize it with the specified
+ * values (rather than copies).  Returns NULL on error.  On success,
+ * the cookie is basically safe for @done_cookie or @accept_cookie,
+ * although you may also want to set the remaining members and check
+ * @get_cookies_accept_policy and @is_domain_security_ok.
+ *
+ * The unsigned char * arguments must be allocated with @mem_alloc or
+ * equivalent, because @done_cookie will @mem_free them.  Likewise,
+ * the caller must already have locked @server.  If @init_cookie
+ * fails, then it frees the strings itself, and unlocks @server.
+ *
+ * If any parameter is NULL, then @init_cookie fails and does not
+ * consider that a bug.  This means callers can use e.g. @stracpy
+ * and let @init_cookie check whether the call ran out of memory.  */
+struct cookie *
+init_cookie(unsigned char *name, unsigned char *value,
+	    unsigned char *path, unsigned char *domain,
+	    struct cookie_server *server)
+{
+	struct cookie *cookie = mem_calloc(1, sizeof(*cookie));
+	if (!cookie || !name || !value || !path || !domain || !server) {
+		mem_free_if(cookie);
+		mem_free_if(name);
+		mem_free_if(value);
+		mem_free_if(path);
+		mem_free_if(domain);
+		done_cookie_server(server);
+		return NULL;
+	}
+	object_nolock(cookie, "cookie"); /* Debugging purpose. */
+
+	cookie->name = name;
+	cookie->value = value;
+	cookie->domain = domain;
+	cookie->path = path;
+	cookie->server = server; /* the caller already locked it for us */
+
+	return cookie;
+}
+
 void
 set_cookie(struct uri *uri, unsigned char *str)
 {
-	unsigned char *path;
+	unsigned char *path, *domain;
 	struct cookie *cookie;
 	struct cookie_str cstr;
 	int max_age;
@@ -297,29 +337,48 @@ set_cookie(struct uri *uri, unsigned char *str)
 
 	if (!parse_cookie_str(&cstr, str)) return;
 
-	cookie = mem_calloc(1, sizeof(*cookie));
+	switch (parse_header_param(str, "path", &path)) {
-	if (!cookie) return;
+		unsigned char *path_end;
 
-	object_nolock(cookie, "cookie"); /* Debugging purpose. */
+	case HEADER_PARAM_FOUND:
+		if (!path[0]
+		    || path[strlen(path) - 1] != '/')
+			add_to_strn(&path, "/");
 
-	/* Fill main fields */
+		if (path[0] != '/') {
+			add_to_strn(&path, "x");
+			memmove(path + 1, path, strlen(path) - 1);
+			path[0] = '/';
+		}
+		break;
 
-	cookie->name = memacpy(str, cstr.nam_end - str);
+	case HEADER_PARAM_NOT_FOUND:
-	cookie->value = memacpy(cstr.val_start, cstr.val_end - cstr.val_start);
+		path = get_uri_string(uri, URI_PATH);
-	cookie->server = get_cookie_server(uri->host, uri->hostlen);
+		if (!path)
-	if (parse_header_param(str, "domain", &cookie->domain)
+			return;
-	    == HEADER_PARAM_NOT_FOUND)
-		cookie->domain = memacpy(uri->host, uri->hostlen);
 
-	/* Now check that all is well */
+		path_end = strrchr(path, '/');
-	if (!cookie->domain
+		if (path_end)
-	    || !cookie->name
+			path_end[1] = '\0';
-	    || !cookie->value
+		break;
-	    || !cookie->server) {
+
-		done_cookie(cookie);
+	default: /* error */
 		return;
 	}
 
+	if (parse_header_param(str, "domain", &domain) == HEADER_PARAM_NOT_FOUND)
+		domain = memacpy(uri->host, uri->hostlen);
+	if (domain && domain[0] == '.')
+		memmove(domain, domain + 1, strlen(domain));
+
+	cookie = init_cookie(memacpy(str, cstr.nam_end - str),
+			     memacpy(cstr.val_start, cstr.val_end - cstr.val_start),
+			     path,
+			     domain,
+			     get_cookie_server(uri->host, uri->hostlen));
+	if (!cookie) return;
+	/* @cookie now owns @path and @domain.  */
+
 #if 0
 	/* We don't actually set ->accept at the moment. But I have kept it
 	 * since it will maybe help to fix bug 77 - Support for more
@@ -378,43 +437,6 @@ set_cookie(struct uri *uri, unsigned char *str)
 		}
 	}
 
-	switch (parse_header_param(str, "path", &path)) {
-		unsigned char *path_end;
-
-	case HEADER_PARAM_FOUND:
-		if (!path[0]
-		    || path[strlen(path) - 1] != '/')
-			add_to_strn(&path, "/");
-
-		if (path[0] != '/') {
-			add_to_strn(&path, "x");
-			memmove(path + 1, path, strlen(path) - 1);
-			path[0] = '/';
-		}
-		break;
-
-	case HEADER_PARAM_NOT_FOUND:
-		path = get_uri_string(uri, URI_PATH);
-		if (!path) {
-			done_cookie(cookie);
-			return;
-		}
-
-		path_end = strrchr(path, '/');
-		if (path_end)
-			path_end[1] = '\0';
-		break;
-
-	default: /* error */
-		done_cookie(cookie);
-		return;
-	}
-	cookie->path = path;
-
-	if (cookie->domain[0] == '.')
-		memmove(cookie->domain, cookie->domain + 1,
-			strlen(cookie->domain));
-
 	cookie->secure = (parse_header_param(str, "secure", NULL)
 	                  == HEADER_PARAM_FOUND);
 

src/cookies/cookies.h

@@ -36,6 +36,9 @@ struct cookie {
 };
 
 struct cookie_server *get_cookie_server(unsigned char *host, int hostlen);
+struct cookie *init_cookie(unsigned char *name, unsigned char *value,
+			   unsigned char *path, unsigned char *domain,
+			   struct cookie_server *server);
 void accept_cookie(struct cookie *);
 void done_cookie(struct cookie *);
 void delete_cookie(struct cookie *);

src/cookies/dialogs.c

@@ -387,9 +387,6 @@ push_add_button(struct dialog_data *dlg_data, struct widget_data *button)
 
 	if (!box->sel || !box->sel->udata) return EVENT_PROCESSED;
 
-	new_cookie = mem_calloc(1, sizeof(*new_cookie));
-	if (!new_cookie) return EVENT_PROCESSED;
-
 	if (box->sel->type == BI_FOLDER) {
 		assert(box->sel->depth == 0);
 		server = box->sel->udata;
@@ -399,12 +396,15 @@ push_add_button(struct dialog_data *dlg_data, struct widget_data *button)
 		server = cookie->server;
 	}
 
-	object_lock(server);
+	object_lock(server);	/* ref consumed by init_cookie */
-	new_cookie->server = server;
+
+	new_cookie = init_cookie(stracpy("") /* name */,
+				 stracpy("") /* value */,
+				 stracpy("/") /* path */,
+				 stracpy(server->host) /* domain */,
+				 server);
+	if (!new_cookie) return EVENT_PROCESSED;
 
-	new_cookie->name = stracpy("");
-	new_cookie->value = stracpy("");
-	new_cookie->domain = stracpy("");
 	accept_cookie(new_cookie);
 	build_edit_dialog(term, new_cookie);
 	return EVENT_PROCESSED;
@@ -417,24 +417,16 @@ static void
 add_server_do(void *data)
 {
 	unsigned char *value = data;
-	struct cookie *dummy_cookie = NULL;
+	struct cookie *dummy_cookie;
 
 	if (!value) return;
 
-	dummy_cookie = mem_calloc(1, sizeof(*dummy_cookie));
+	dummy_cookie = init_cookie(stracpy("empty") /* name */,
-	if (dummy_cookie == NULL) return;
+				   stracpy("1") /* value */,
-
+				   stracpy("/") /* path */,
-	dummy_cookie->name = stracpy("empty");
+				   stracpy(value) /* domain */,
-	dummy_cookie->value = stracpy("1");
+				   get_cookie_server(value, strlen(value)));
-	dummy_cookie->path = stracpy("/");
+	if (!dummy_cookie) return;
-	dummy_cookie->domain = stracpy(value);
-	dummy_cookie->server = get_cookie_server(value, strlen(value));
-	if (dummy_cookie->name == NULL || dummy_cookie->value == NULL
-	    || dummy_cookie->path == NULL || dummy_cookie->domain == NULL
-	    || dummy_cookie->server == NULL) {
-		done_cookie(dummy_cookie);
-		return;
-	}
 
 	accept_cookie(dummy_cookie);
 }