From 8ecf105cd52219ab4672a1e74b6e406250a19cc0 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 9 Dec 2006 15:03:08 +0200 Subject: [PATCH 1/7] Bug 886: Document why delete_cookie does not set cookies_dirty. --- src/cookies/cookies.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cookies/cookies.c b/src/cookies/cookies.c index cdf24dfa..9fe3c5e2 100644 --- a/src/cookies/cookies.c +++ b/src/cookies/cookies.c @@ -207,6 +207,10 @@ done_cookie(struct cookie *c) mem_free(c); } +/* The cookie @c can be either in @cookies or in @cookie_queries. + * Because changes in @cookie_queries should not affect the cookie + * file, this function does not set @cookies_dirty. Instead, the + * caller must do that if appropriate. */ void delete_cookie(struct cookie *c) { From b79210ea1b047ec06d6ee7de3a2c21006f3da42e Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 9 Dec 2006 16:24:24 +0200 Subject: [PATCH 2/7] Bug 886: Postpone resaving the cookies until a bottom half. --- src/cookies/cookies.c | 41 +++++++++++++++++++++++++++++------------ src/cookies/cookies.h | 1 + 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/cookies/cookies.c b/src/cookies/cookies.c index 9fe3c5e2..4e20e74d 100644 --- a/src/cookies/cookies.c +++ b/src/cookies/cookies.c @@ -29,6 +29,7 @@ #include "intl/gettext/libintl.h" #include "main/module.h" #include "main/object.h" +#include "main/select.h" #include "protocol/date.h" #include "protocol/header.h" #include "protocol/protocol.h" @@ -69,6 +70,7 @@ static INIT_LIST_HEAD(c_domains); * struct cookie_server. */ static INIT_LIST_HEAD(cookie_servers); +/* Only @set_cookies_dirty may make this nonzero. */ static int cookies_dirty = 0; enum cookies_option { @@ -506,7 +508,7 @@ accept_cookie(struct cookie *cookie) } add_to_list(cookies, cookie); - cookies_dirty = 1; + set_cookies_dirty(); /* XXX: This crunches CPU too. --pasky */ foreach (cd, c_domains) @@ -520,9 +522,6 @@ accept_cookie(struct cookie *cookie) memcpy(cd->domain, cookie->domain, domain_len + 1); add_to_list(c_domains, cd); - - if (get_cookies_save() && get_cookies_resave()) - save_cookies(); } #if 0 @@ -549,9 +548,6 @@ delete_cookie(struct cookie *c) end: del_from_list(c); done_cookie(c); - - if (get_cookies_save() && get_cookies_resave()) - save_cookies(); } @@ -679,7 +675,7 @@ send_cookies(struct uri *uri) #endif delete_cookie(c); - cookies_dirty = 1; + set_cookies_dirty(); continue; } @@ -700,9 +696,6 @@ send_cookies(struct uri *uri) mem_free(path); - if (cookies_dirty && get_cookies_save() && get_cookies_resave()) - save_cookies(); - if (!header.length) { done_string(&header); return NULL; @@ -772,7 +765,7 @@ load_cookies(void) { /* Skip expired cookies if any. */ expires = str_to_time_t(members[EXPIRES].pos); if (!expires || expires <= now) { - cookies_dirty = 1; + set_cookies_dirty(); continue; } @@ -803,6 +796,30 @@ load_cookies(void) { fclose(fp); } +static void +resave_cookies_bottom_half(void *always_null) +{ + if (get_cookies_save() && get_cookies_resave()) + save_cookies(); /* checks cookies_dirty */ +} + +/* Note that the cookies have been modified, and register a bottom + * half for saving them if appropriate. We use a bottom half so that + * if something makes multiple changes and calls this for each change, + * the cookies get saved only once at the end. */ +void +set_cookies_dirty(void) +{ + /* Do not check @cookies_dirty here. If the previous attempt + * to save cookies failed, @cookies_dirty can still be nonzero + * even though @resave_cookies_bottom_half is no longer in the + * queue. */ + cookies_dirty = 1; + /* If @resave_cookies_bottom_half is already in the queue, + * @register_bottom_half does nothing. */ + register_bottom_half(resave_cookies_bottom_half, NULL); +} + void save_cookies(void) { struct cookie *c; diff --git a/src/cookies/cookies.h b/src/cookies/cookies.h index db2f01ce..be60ae26 100644 --- a/src/cookies/cookies.h +++ b/src/cookies/cookies.h @@ -45,6 +45,7 @@ void delete_cookie(struct cookie *); void set_cookie(struct uri *, unsigned char *); void load_cookies(void); void save_cookies(void); +void set_cookies_dirty(void); /* Note that the returned value points to a static structure and thus the * string will be overwritten at the next call time. The string source From fad9c75cc6b04ea1f7513763d308dd1d96c5af74 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 9 Dec 2006 16:24:24 +0200 Subject: [PATCH 3/7] Bug 886: Update cookies_dirty after delete_cookie calls as appropriate. --- src/cookies/cookies.c | 9 +++++++++ src/cookies/dialogs.c | 6 +----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/cookies/cookies.c b/src/cookies/cookies.c index 4e20e74d..2f0530dd 100644 --- a/src/cookies/cookies.c +++ b/src/cookies/cookies.c @@ -504,6 +504,7 @@ accept_cookie(struct cookie *cookie) continue; delete_cookie(c); + /* @set_cookies_dirty will be called below. */ } } @@ -573,6 +574,7 @@ reject_cookie(void *idp) if (!c) return; delete_cookie(c); + set_cookies_dirty(); /* @find_cookie_id doesn't use @cookie_queries */ } @@ -860,6 +862,8 @@ init_cookies(struct module *module) load_cookies(); } +/* Like @delete_cookie, this function does not set @cookies_dirty. + * The caller must do that if appropriate. */ static void free_cookies_list(struct list_head *list) { @@ -880,6 +884,11 @@ done_cookies(struct module *module) free_cookies_list(&cookies); free_cookies_list(&cookie_queries); + /* If @save_cookies failed above, @cookies_dirty can still be + * nonzero. Now if @resave_cookies_bottom_half were in the + * queue, it could save the empty @cookies list to the file. + * Prevent that. */ + cookies_dirty = 0; } struct module cookies_module = struct_module( diff --git a/src/cookies/dialogs.c b/src/cookies/dialogs.c index ccae1a65..2263cdb9 100644 --- a/src/cookies/dialogs.c +++ b/src/cookies/dialogs.c @@ -188,12 +188,8 @@ delete_cookie_item(struct listbox_item *item, int last) assert(!is_object_used(cookie)); delete_cookie(cookie); + set_cookies_dirty(); } - - if (last - && get_opt_bool("cookies.save") - && get_opt_bool("cookies.resave")) - save_cookies(); } static struct listbox_ops_messages cookies_messages = { From 3cd0fbe5f07357268a215041979b6fa816e782a5 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 9 Dec 2006 14:01:39 +0200 Subject: [PATCH 4/7] Bug 886: Set cookies_dirty if a cookie is edited via the manager. If the appropriate option is set, this now causes the cookies to be immediately saved as well. --- src/cookies/dialogs.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/cookies/dialogs.c b/src/cookies/dialogs.c index 2263cdb9..3ff9183c 100644 --- a/src/cookies/dialogs.c +++ b/src/cookies/dialogs.c @@ -242,6 +242,7 @@ set_cookie_name(struct dialog_data *dlg_data, struct widget_data *widget_data) if (!value || !cookie) return EVENT_NOT_PROCESSED; mem_free_set(&cookie->name, stracpy(value)); + set_cookies_dirty(); return EVENT_PROCESSED; } @@ -253,6 +254,7 @@ set_cookie_value(struct dialog_data *dlg_data, struct widget_data *widget_data) if (!value || !cookie) return EVENT_NOT_PROCESSED; mem_free_set(&cookie->value, stracpy(value)); + set_cookies_dirty(); return EVENT_PROCESSED; } @@ -264,6 +266,7 @@ set_cookie_domain(struct dialog_data *dlg_data, struct widget_data *widget_data) if (!value || !cookie) return EVENT_NOT_PROCESSED; mem_free_set(&cookie->domain, stracpy(value)); + set_cookies_dirty(); return EVENT_PROCESSED; } @@ -282,6 +285,7 @@ set_cookie_expires(struct dialog_data *dlg_data, struct widget_data *widget_data if (errno || *end || number < 0) return EVENT_NOT_PROCESSED; cookie->expires = (time_t) number; + set_cookies_dirty(); return EVENT_PROCESSED; } @@ -300,6 +304,7 @@ set_cookie_secure(struct dialog_data *dlg_data, struct widget_data *widget_data) if (errno || *end) return EVENT_NOT_PROCESSED; cookie->secure = (number != 0); + set_cookies_dirty(); return EVENT_PROCESSED; } From 47a2fc19e122ce64236859c9b36889cadb19a679 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 9 Dec 2006 18:14:28 +0200 Subject: [PATCH 5/7] Bug 887: New function secsave_strerror. Extracted from write_config_dialog. --- src/config/dialogs.c | 32 +------------------------------- src/util/secsave.c | 28 ++++++++++++++++++++++++++++ src/util/secsave.h | 4 ++++ 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/config/dialogs.c b/src/config/dialogs.c index 49a7837d..e06e4a54 100644 --- a/src/config/dialogs.c +++ b/src/config/dialogs.c @@ -60,37 +60,7 @@ write_config_dialog(struct terminal *term, unsigned char *config_file, return; } - switch (secsave_error) { - case SS_ERR_OPEN_READ: - strerr = _("Cannot read the file", term); - break; - case SS_ERR_STAT: - strerr = _("Cannot get file status", term); - break; - case SS_ERR_ACCESS: - strerr = _("Cannot access the file", term); - break; - case SS_ERR_MKSTEMP: - strerr = _("Cannot create temp file", term); - break; - case SS_ERR_RENAME: - strerr = _("Cannot rename the file", term); - break; - case SS_ERR_DISABLED: - strerr = _("File saving disabled by option", term); - break; - case SS_ERR_OUT_OF_MEM: - strerr = _("Out of memory", term); - break; - case SS_ERR_OPEN_WRITE: - strerr = _("Cannot write the file", term); - break; - case SS_ERR_NONE: /* Impossible. */ - case SS_ERR_OTHER: - default: - strerr = _("Secure file saving error", term); - break; - } + strerr = secsave_strerror(secsave_error, term); if (stdio_error > 0) errmsg = straconcat(strerr, " (", strerror(stdio_error), ")", NULL); diff --git a/src/util/secsave.c b/src/util/secsave.c index 10ffd326..6e9d7666 100644 --- a/src/util/secsave.c +++ b/src/util/secsave.c @@ -18,6 +18,7 @@ #include "elinks.h" #include "config/options.h" +#include "intl/gettext/libintl.h" #include "osdep/osdep.h" /* Needed for mkstemp() on win32 */ #include "util/memory.h" #include "util/secsave.h" @@ -350,3 +351,30 @@ secure_fprintf(struct secure_save_info *ssi, const char *format, ...) return ret; } + +unsigned char * +secsave_strerror(enum secsave_errno secsave_error, struct terminal *term) +{ + switch (secsave_error) { + case SS_ERR_OPEN_READ: + return _("Cannot read the file", term); + case SS_ERR_STAT: + return _("Cannot get file status", term); + case SS_ERR_ACCESS: + return _("Cannot access the file", term); + case SS_ERR_MKSTEMP: + return _("Cannot create temp file", term); + case SS_ERR_RENAME: + return _("Cannot rename the file", term); + case SS_ERR_DISABLED: + return _("File saving disabled by option", term); + case SS_ERR_OUT_OF_MEM: + return _("Out of memory", term); + case SS_ERR_OPEN_WRITE: + return _("Cannot write the file", term); + case SS_ERR_NONE: /* Impossible. */ + case SS_ERR_OTHER: + default: + return _("Secure file saving error", term); + } +} diff --git a/src/util/secsave.h b/src/util/secsave.h index 001c1460..65506dc0 100644 --- a/src/util/secsave.h +++ b/src/util/secsave.h @@ -6,6 +6,8 @@ #include #include /* mode_t */ +struct terminal; + enum secsave_errno { SS_ERR_NONE = 0, SS_ERR_DISABLED, /* secsave is disabled. */ @@ -40,4 +42,6 @@ int secure_fputc(struct secure_save_info *, int); int secure_fprintf(struct secure_save_info *, const char *, ...); +unsigned char *secsave_strerror(enum secsave_errno, struct terminal *); + #endif From 7551be3194926ad5382c3291887188b27574ffa5 Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 9 Dec 2006 18:23:41 +0200 Subject: [PATCH 6/7] Bug 887: save_cookies ignores cookies_dirty if requested by the user. --- src/cookies/cookies.c | 12 ++++++++---- src/cookies/cookies.h | 2 +- src/cookies/dialogs.c | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/cookies/cookies.c b/src/cookies/cookies.c index 2f0530dd..4c7c32a1 100644 --- a/src/cookies/cookies.c +++ b/src/cookies/cookies.c @@ -802,7 +802,7 @@ static void resave_cookies_bottom_half(void *always_null) { if (get_cookies_save() && get_cookies_resave()) - save_cookies(); /* checks cookies_dirty */ + save_cookies(0); /* checks cookies_dirty */ } /* Note that the cookies have been modified, and register a bottom @@ -822,14 +822,18 @@ set_cookies_dirty(void) register_bottom_half(resave_cookies_bottom_half, NULL); } +/* @interactive is 1 if the user told ELinks to save cookies, or 0 if + * ELinks decided that on its own. In the latter case, this function + * does not save the cookies if it thinks the file is already up to + * date. */ void -save_cookies(void) { +save_cookies(int interactive) { struct cookie *c; unsigned char *cookfile; struct secure_save_info *ssi; time_t now; - if (cookies_nosave || !elinks_home || !cookies_dirty + if (cookies_nosave || !elinks_home || !(cookies_dirty || interactive) || get_cmd_opt_bool("anonymous")) return; @@ -880,7 +884,7 @@ done_cookies(struct module *module) free_list(c_domains); if (!cookies_nosave && get_cookies_save()) - save_cookies(); + save_cookies(0); free_cookies_list(&cookies); free_cookies_list(&cookie_queries); diff --git a/src/cookies/cookies.h b/src/cookies/cookies.h index be60ae26..bf67ae7d 100644 --- a/src/cookies/cookies.h +++ b/src/cookies/cookies.h @@ -44,7 +44,7 @@ void done_cookie(struct cookie *); void delete_cookie(struct cookie *); void set_cookie(struct uri *, unsigned char *); void load_cookies(void); -void save_cookies(void); +void save_cookies(int interactive); void set_cookies_dirty(void); /* Note that the returned value points to a static structure and thus the diff --git a/src/cookies/dialogs.c b/src/cookies/dialogs.c index 3ff9183c..9d6bdf21 100644 --- a/src/cookies/dialogs.c +++ b/src/cookies/dialogs.c @@ -465,7 +465,7 @@ push_add_server_button(struct dialog_data *dlg_data, struct widget_data *button) static widget_handler_status_T push_save_button(struct dialog_data *dlg_data, struct widget_data *button) { - save_cookies(); + save_cookies(1); return EVENT_PROCESSED; } From e815e071797aa5aa97261a10eb1a373c2e4c308c Mon Sep 17 00:00:00 2001 From: Kalle Olavi Niemitalo Date: Sat, 9 Dec 2006 18:27:40 +0200 Subject: [PATCH 7/7] Bug 887: save_cookies reports errors if requested by the user. --- src/cookies/cookies.c | 59 +++++++++++++++++++++++++++++++++++-------- src/cookies/cookies.h | 3 ++- src/cookies/dialogs.c | 2 +- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/cookies/cookies.c b/src/cookies/cookies.c index 4c7c32a1..ab5810b2 100644 --- a/src/cookies/cookies.c +++ b/src/cookies/cookies.c @@ -802,7 +802,7 @@ static void resave_cookies_bottom_half(void *always_null) { if (get_cookies_save() && get_cookies_resave()) - save_cookies(0); /* checks cookies_dirty */ + save_cookies(NULL); /* checks cookies_dirty */ } /* Note that the cookies have been modified, and register a bottom @@ -822,27 +822,58 @@ set_cookies_dirty(void) register_bottom_half(resave_cookies_bottom_half, NULL); } -/* @interactive is 1 if the user told ELinks to save cookies, or 0 if - * ELinks decided that on its own. In the latter case, this function - * does not save the cookies if it thinks the file is already up to - * date. */ +/* @term is non-NULL if the user told ELinks to save cookies, or NULL + * if ELinks decided that on its own. In the former case, this + * function reports errors to @term, unless CONFIG_SMALL is defined. + * In the latter case, this function does not save the cookies if it + * thinks the file is already up to date. */ void -save_cookies(int interactive) { +save_cookies(struct terminal *term) { struct cookie *c; unsigned char *cookfile; struct secure_save_info *ssi; time_t now; - if (cookies_nosave || !elinks_home || !(cookies_dirty || interactive) - || get_cmd_opt_bool("anonymous")) +#ifdef CONFIG_SMALL +# define CANNOT_SAVE_COOKIES(message) +#else +# define CANNOT_SAVE_COOKIES(flags, message) \ + do { \ + if (term) \ + info_box(term, flags, N_("Cannot save cookies"),\ + ALIGN_LEFT, message); \ + } while (0) +#endif + + if (cookies_nosave) { + assert(term == NULL); + if_assert_failed {} return; + } + if (!elinks_home) { + CANNOT_SAVE_COOKIES(0, N_("ELinks was started without a home directory.")); + return; + } + if (!cookies_dirty && !term) + return; + if (get_cmd_opt_bool("anonymous")) { + CANNOT_SAVE_COOKIES(0, N_("ELinks was started with the -anonymous option.")); + return; + } cookfile = straconcat(elinks_home, COOKIES_FILENAME, NULL); - if (!cookfile) return; + if (!cookfile) { + CANNOT_SAVE_COOKIES(0, N_("Out of memory")); + return; + } ssi = secure_open(cookfile); mem_free(cookfile); - if (!ssi) return; + if (!ssi) { + CANNOT_SAVE_COOKIES(MSGBOX_NO_TEXT_INTL, + secsave_strerror(secsave_errno, term)); + return; + } now = time(NULL); foreach (c, cookies) { @@ -856,7 +887,13 @@ save_cookies(int interactive) { break; } + secsave_errno = SS_ERR_OTHER; /* @secure_close doesn't always set it */ if (!secure_close(ssi)) cookies_dirty = 0; + else { + CANNOT_SAVE_COOKIES(MSGBOX_NO_TEXT_INTL, + secsave_strerror(secsave_errno, term)); + } +#undef CANNOT_SAVE_COOKIES } static void @@ -884,7 +921,7 @@ done_cookies(struct module *module) free_list(c_domains); if (!cookies_nosave && get_cookies_save()) - save_cookies(0); + save_cookies(NULL); free_cookies_list(&cookies); free_cookies_list(&cookie_queries); diff --git a/src/cookies/cookies.h b/src/cookies/cookies.h index bf67ae7d..4a7dfa48 100644 --- a/src/cookies/cookies.h +++ b/src/cookies/cookies.h @@ -8,6 +8,7 @@ #include "util/time.h" struct listbox_item; +struct terminal; enum cookies_accept { COOKIES_ACCEPT_NONE, @@ -44,7 +45,7 @@ void done_cookie(struct cookie *); void delete_cookie(struct cookie *); void set_cookie(struct uri *, unsigned char *); void load_cookies(void); -void save_cookies(int interactive); +void save_cookies(struct terminal *); void set_cookies_dirty(void); /* Note that the returned value points to a static structure and thus the diff --git a/src/cookies/dialogs.c b/src/cookies/dialogs.c index 9d6bdf21..3af9c3dc 100644 --- a/src/cookies/dialogs.c +++ b/src/cookies/dialogs.c @@ -465,7 +465,7 @@ push_add_server_button(struct dialog_data *dlg_data, struct widget_data *button) static widget_handler_status_T push_save_button(struct dialog_data *dlg_data, struct widget_data *button) { - save_cookies(1); + save_cookies(dlg_data->win->term); return EVENT_PROCESSED; }