From ab23505519c92260846aa6e86b80646fad13779c Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Thu, 1 Jun 2006 00:36:11 +0300 Subject: [PATCH] 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 | 130 ++++++++++++++++++++++++------------------ src/cookies/cookies.h | 3 + src/cookies/dialogs.c | 38 +++++------- 3 files changed, 94 insertions(+), 77 deletions(-) diff --git a/src/cookies/cookies.c b/src/cookies/cookies.c index 8dd370b6..77d24ac1 100644 --- a/src/cookies/cookies.c +++ b/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)); - if (!cookie) return; + switch (parse_header_param(str, "path", &path)) { + 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); - cookie->value = memacpy(cstr.val_start, cstr.val_end - cstr.val_start); - cookie->server = get_cookie_server(uri->host, uri->hostlen); - if (parse_header_param(str, "domain", &cookie->domain) - == HEADER_PARAM_NOT_FOUND) - cookie->domain = memacpy(uri->host, uri->hostlen); + case HEADER_PARAM_NOT_FOUND: + path = get_uri_string(uri, URI_PATH); + if (!path) + return; - /* Now check that all is well */ - if (!cookie->domain - || !cookie->name - || !cookie->value - || !cookie->server) { - done_cookie(cookie); + path_end = strrchr(path, '/'); + if (path_end) + path_end[1] = '\0'; + break; + + 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); diff --git a/src/cookies/cookies.h b/src/cookies/cookies.h index fc29a051..db2f01ce 100644 --- a/src/cookies/cookies.h +++ b/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 *); diff --git a/src/cookies/dialogs.c b/src/cookies/dialogs.c index 0efef568..ccae1a65 100644 --- a/src/cookies/dialogs.c +++ b/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); - new_cookie->server = server; + object_lock(server); /* ref consumed by init_cookie */ + + 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)); - if (dummy_cookie == NULL) return; - - dummy_cookie->name = stracpy("empty"); - dummy_cookie->value = stracpy("1"); - dummy_cookie->path = stracpy("/"); - 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; - } + dummy_cookie = init_cookie(stracpy("empty") /* name */, + stracpy("1") /* value */, + stracpy("/") /* path */, + stracpy(value) /* domain */, + get_cookie_server(value, strlen(value))); + if (!dummy_cookie) return; accept_cookie(dummy_cookie); }