From 4ac1c81ed710c8b27dc23821330b2005f7122214 Mon Sep 17 00:00:00 2001 From: Mehdi ABAAKOUK Date: Wed, 25 Oct 2017 18:10:53 +0200 Subject: [PATCH] url: make notmuch query string parser generic (#812) * Notmuch: remove custom UriTag linked list Use STAILQ helper for UriTag linked list. * url: make notmuch query string parser generic Enhance the url.c to handle query strings. This permits to remove the notmuch custom url parser. * url: always parse query string This removes url_parse_with_qs methods. * fix some problems - double free in nm_normalize_uri() - set path to NULL in url_parse() If we're setting all the other members, we should set the path too - tidy some doxygen docs * notmuch: remove useless starting / from db_filename --- browser.c | 1 + imap/util.c | 3 + mutt_notmuch.c | 189 +++++++++++-------------------------------------- newsrc.c | 3 + nntp.c | 4 +- pop_lib.c | 4 +- smtp.c | 3 + url.c | 125 +++++++++++++++++++++++++------- url.h | 20 +++++- 9 files changed, 176 insertions(+), 176 deletions(-) diff --git a/browser.c b/browser.c index 9396582ba..f3652b192 100644 --- a/browser.c +++ b/browser.c @@ -1400,6 +1400,7 @@ void _mutt_select_file(char *f, size_t flen, int flags, char ***files, int *numf LastDir[n] = '\0'; LastDir[n - 1] = state.entry[menu->current].delim; } + url_free(&url); } #endif else diff --git a/imap/util.c b/imap/util.c index 005902671..04d6badef 100644 --- a/imap/util.c +++ b/imap/util.c @@ -325,6 +325,7 @@ int imap_parse_path(const char *path, struct ImapMbox *mx) { if (mutt_account_fromurl(&mx->account, &url) < 0 || !*mx->account.host) { + url_free(&url); FREE(&c); return -1; } @@ -334,11 +335,13 @@ int imap_parse_path(const char *path, struct ImapMbox *mx) if (url.scheme == U_IMAPS) mx->account.flags |= MUTT_ACCT_SSL; + url_free(&url); FREE(&c); } /* old PINE-compatibility code */ else { + url_free(&url); FREE(&c); if (sscanf(path, "{%127[^}]}", tmp) != 1) return -1; diff --git a/mutt_notmuch.c b/mutt_notmuch.c index 6ea5bd08a..eff01e2fb 100644 --- a/mutt_notmuch.c +++ b/mutt_notmuch.c @@ -85,20 +85,6 @@ enum NmQueryType NM_QUERY_TYPE_THREADS /**< Whole threads */ }; -/** - * struct UriTag - Parsed Notmuch-URI arguments - * - * The arguments in a URI are saved in a linked list. - * - * @sa NmCtxData#query_items - */ -struct UriTag -{ - char *name; - char *value; - struct UriTag *next; -}; - /** * struct NmHdrData - Notmuch data attached to an email * @@ -125,13 +111,13 @@ struct NmCtxData { notmuch_database_t *db; - char *db_filename; /**< Filename of the Notmuch database */ + struct Url db_url; /**< Parsed view url of the Notmuch database */ + char *db_url_holder; /**< The storage string used by db_url, we keep it + * to be able to free db_url */ char *db_query; /**< Previous query */ int db_limit; /**< Maximum number of results to return */ enum NmQueryType query_type; /**< Messages or Threads */ - struct UriTag *query_items; - struct Progress progress; /**< A progress bar */ int oldmsgcount; int ignmsgcount; /**< Ignored messages */ @@ -183,108 +169,13 @@ static void debug_print_tags(notmuch_message_t *msg) #endif /** - * url_free_tags - Free a list of tags - * @param tags List of tags - * - * Tags are stored as a singly-linked list. - * Free all the strings and the list, itself. - */ -static void url_free_tags(struct UriTag *tags) -{ - while (tags) - { - struct UriTag *next = tags->next; - FREE(&tags->name); - FREE(&tags->value); - FREE(&tags); - tags = next; - } -} - -/** - * url_parse_query - Extract the tokens from a query URI - * @param[in] url URI to parse - * @param[out] filename Save the filename - * @param[out] tags Save the list of tags - * @retval true Success - * @retval false Error Bad format - * - * Parse a Notmuch URI, such as: - * * notmuch:///path/to/db?query=tag:lkml&limit=1000 - * * notmuch://?query=neomutt + * free_hdrdata - Free header data attached to an email + * @param data Header data * - * Extract the database filename (optional) and any search parameters (tags). - * The tags will be saved in a linked list (#UriTag). + * Each email can have an attached nm_hdrdata struct, which contains things + * like the tags (labels). This function frees all the resources and the + * nm_hdrdata struct itself. */ -static bool url_parse_query(const char *url, char **filename, struct UriTag **tags) -{ - char *p = strstr(url, "://"); /* remote unsupported */ - char *e = NULL; - struct UriTag *tag, *last = NULL; - - *filename = NULL; - *tags = NULL; - - if (!p || !*(p + 3)) - return false; - - p += 3; - *filename = p; - - e = strchr(p, '?'); - - *filename = e ? (e == p) ? NULL : mutt_substrdup(p, e) : safe_strdup(p); - if (!e) - return true; /* only filename */ - - if (*filename && (url_pct_decode(*filename) < 0)) - goto err; - - e++; /* skip '?' */ - p = e; - - while (p && *p) - { - tag = safe_calloc(1, sizeof(struct UriTag)); - - if (!*tags) - last = *tags = tag; - else - { - last->next = tag; - last = tag; - } - - e = strchr(p, '='); - if (!e) - e = strchr(p, '&'); - tag->name = e ? mutt_substrdup(p, e) : safe_strdup(p); - if (!tag->name || (url_pct_decode(tag->name) < 0)) - goto err; - if (!e) - break; - - p = e + 1; - - if (*e == '&') - continue; - - e = strchr(p, '&'); - tag->value = e ? mutt_substrdup(p, e) : safe_strdup(p); - if (!tag->value || (url_pct_decode(tag->value) < 0)) - goto err; - if (!e) - break; - p = e + 1; - } - - return true; -err: - FREE(&(*filename)); - url_free_tags(*tags); - return false; -} - static void free_hdrdata(struct NmHdrData *data) { if (!data) @@ -320,9 +211,9 @@ static void free_ctxdata(struct NmCtxData *data) #endif data->db = NULL; - FREE(&data->db_filename); + url_free(&data->db_url); + FREE(&data->db_url_holder); FREE(&data->db_query); - url_free_tags(data->query_items); FREE(&data); } @@ -345,14 +236,14 @@ static struct NmCtxData *new_ctxdata(const char *uri) mutt_debug(1, "nm: initialize context data %p\n", (void *) data); data->db_limit = NmDbLimit; + data->db_url_holder = safe_strdup(uri); - if (!url_parse_query(uri, &data->db_filename, &data->query_items)) + if (url_parse(&data->db_url, data->db_url_holder) < 0) { mutt_error(_("failed to parse notmuch uri: %s"), uri); FREE(&data); return NULL; } - return data; } @@ -549,7 +440,7 @@ static char *get_query_string(struct NmCtxData *data, bool window) { mutt_debug(2, "nm: get_query_string(%s)\n", window ? "true" : "false"); - struct UriTag *item = NULL; + struct UrlQueryString *item = NULL; if (!data) return NULL; @@ -558,7 +449,7 @@ static char *get_query_string(struct NmCtxData *data, bool window) data->query_type = string_to_query_type(NmQueryType); /* user's default */ - for (item = data->query_items; item; item = item->next) + STAILQ_FOREACH(item, &data->db_url.query_strings, entries) { if (!item->value || !item->name) continue; @@ -611,7 +502,7 @@ static const char *get_db_filename(struct NmCtxData *data) if (!data) return NULL; - db_filename = data->db_filename ? data->db_filename : NmDefaultUri; + db_filename = data->db_url.path ? data->db_url.path : NmDefaultUri; if (!db_filename) db_filename = Folder; if (!db_filename) @@ -1799,6 +1690,7 @@ bool nm_normalize_uri(char *new_uri, const char *orig_uri, size_t new_uri_sz) { mutt_debug(2, "nm_normalize_uri (%s)\n", orig_uri); char buf[LONG_STRING]; + int rc = -1; struct Context tmp_ctx; memset(&tmp_ctx, 0, sizeof(tmp_ctx)); @@ -1813,30 +1705,30 @@ bool nm_normalize_uri(char *new_uri, const char *orig_uri, size_t new_uri_sz) mutt_debug(2, "nm_normalize_uri #1 () -> db_query: %s\n", tmp_ctxdata->db_query); if (get_query_string(tmp_ctxdata, false) == NULL) - { - mutt_error(_("failed to parse notmuch uri: %s"), orig_uri); - mutt_debug(2, "nm_normalize_uri () -> error #1\n"); - FREE(&tmp_ctxdata); - return false; - } + goto gone; mutt_debug(2, "nm_normalize_uri #2 () -> db_query: %s\n", tmp_ctxdata->db_query); strfcpy(buf, tmp_ctxdata->db_query, sizeof(buf)); if (nm_uri_from_query(&tmp_ctx, buf, sizeof(buf)) == NULL) - { - mutt_error(_("failed to parse notmuch uri: %s"), orig_uri); - mutt_debug(2, "nm_normalize_uri () -> error #2\n"); - FREE(&tmp_ctxdata); - return true; - } - - FREE(&tmp_ctxdata); + goto gone; strncpy(new_uri, buf, new_uri_sz); mutt_debug(2, "nm_normalize_uri #3 (%s) -> %s\n", orig_uri, new_uri); + + rc = 0; +gone: + url_free(&tmp_ctxdata->db_url); + FREE(&tmp_ctxdata->db_url_holder); + FREE(&tmp_ctxdata); + if (rc < 0) + { + mutt_error(_("failed to parse notmuch uri: %s"), orig_uri); + mutt_debug(2, "nm_normalize_uri () -> error\n"); + return false; + } return true; } @@ -2009,23 +1901,23 @@ int nm_update_filename(struct Context *ctx, const char *old, const char *new, int nm_nonctx_get_count(char *path, int *all, int *new) { - struct UriTag *query_items = NULL, *item = NULL; + struct UrlQueryString *item = NULL; + struct Url url; + char *url_holder = safe_strdup(path); char *db_filename = NULL, *db_query = NULL; notmuch_database_t *db = NULL; int rc = -1; - bool dflt = false; - mutt_debug(1, "nm: count\n"); - if (!url_parse_query(path, &db_filename, &query_items)) + memset(&url, 0, sizeof(struct Url)); + + if (url_parse(&url, url_holder) < 0) { mutt_error(_("failed to parse notmuch uri: %s"), path); goto done; } - if (!query_items) - goto done; - for (item = query_items; item; item = item->next) + STAILQ_FOREACH(item, &url.query_strings, entries) { if (item->value && (strcmp(item->name, "query") == 0)) { @@ -2037,6 +1929,7 @@ int nm_nonctx_get_count(char *path, int *all, int *new) if (!db_query) goto done; + db_filename = url.path; if (!db_filename) { if (NmDefaultUri) @@ -2048,7 +1941,6 @@ int nm_nonctx_get_count(char *path, int *all, int *new) } else if (Folder) db_filename = Folder; - dflt = true; } /* don't be verbose about connection, as we're called from @@ -2082,9 +1974,8 @@ done: #endif mutt_debug(1, "nm: count close DB\n"); } - if (!dflt) - FREE(&db_filename); - url_free_tags(query_items); + url_free(&url); + FREE(&url_holder); mutt_debug(1, "nm: count done [rc=%d]\n", rc); return rc; diff --git a/newsrc.c b/newsrc.c index 6deb8b0bd..159f9645c 100644 --- a/newsrc.c +++ b/newsrc.c @@ -955,8 +955,10 @@ struct NntpServer *nntp_select_server(char *server, bool leave_lock) snprintf(file, sizeof(file), "%s%s", strstr(server, "://") ? "" : "news://", server); if (url_parse(&url, file) < 0 || (url.path && *url.path) || !(url.scheme == U_NNTP || url.scheme == U_NNTPS) || + !url.host || mutt_account_fromurl(&acct, &url) < 0) { + url_free(&url); mutt_error(_("%s is an invalid news server specification!"), server); mutt_sleep(2); return NULL; @@ -966,6 +968,7 @@ struct NntpServer *nntp_select_server(char *server, bool leave_lock) acct.flags |= MUTT_ACCT_SSL; acct.port = NNTP_SSL_PORT; } + url_free(&url); /* find connection by account */ conn = mutt_conn_find(NULL, &acct); diff --git a/nntp.c b/nntp.c index 4f8a40ef2..1dbde534f 100644 --- a/nntp.c +++ b/nntp.c @@ -1453,8 +1453,9 @@ static int nntp_open_mailbox(struct Context *ctx) struct Url url; strfcpy(buf, ctx->path, sizeof(buf)); - if (url_parse(&url, buf) < 0 || !url.path || !(url.scheme == U_NNTP || url.scheme == U_NNTPS)) + if (url_parse(&url, buf) < 0 || !url.host || !url.path || !(url.scheme == U_NNTP || url.scheme == U_NNTPS)) { + url_free(&url); mutt_error(_("%s is an invalid newsgroup specification!"), ctx->path); mutt_sleep(2); return -1; @@ -1464,6 +1465,7 @@ static int nntp_open_mailbox(struct Context *ctx) url.path = strchr(url.path, '\0'); url_tostring(&url, server, sizeof(server), 0); nserv = nntp_select_server(server, true); + url_free(&url); if (!nserv) return -1; CurrentNewsSrv = nserv; diff --git a/pop_lib.c b/pop_lib.c index 19d446ec4..827eb0d78 100644 --- a/pop_lib.c +++ b/pop_lib.c @@ -67,8 +67,9 @@ int pop_parse_path(const char *path, struct Account *acct) c = safe_strdup(path); url_parse(&url, c); - if ((url.scheme != U_POP && url.scheme != U_POPS) || mutt_account_fromurl(acct, &url) < 0) + if ((url.scheme != U_POP && url.scheme != U_POPS) || !url.host || mutt_account_fromurl(acct, &url) < 0) { + url_free(&url); FREE(&c); mutt_error(_("Invalid POP URL: %s\n"), path); mutt_sleep(1); @@ -88,6 +89,7 @@ int pop_parse_path(const char *path, struct Account *acct) ; } + url_free(&url); FREE(&c); return 0; } diff --git a/smtp.c b/smtp.c index 7e42e089a..9a9b615dc 100644 --- a/smtp.c +++ b/smtp.c @@ -293,13 +293,16 @@ static int smtp_fill_account(struct Account *account) urlstr = safe_strdup(SmtpUrl); url_parse(&url, urlstr); if ((url.scheme != U_SMTP && url.scheme != U_SMTPS) || + !url.host || mutt_account_fromurl(account, &url) < 0) { + url_free(&url); FREE(&urlstr); mutt_error(_("Invalid SMTP URL: %s"), SmtpUrl); mutt_sleep(1); return -1; } + url_free(&url); FREE(&urlstr); if (url.scheme == U_SMTPS) diff --git a/url.c b/url.c index 34972100a..3151be557 100644 --- a/url.c +++ b/url.c @@ -97,20 +97,75 @@ enum UrlScheme url_check_scheme(const char *s) return (enum UrlScheme) i; } + +static int parse_query_string(struct Url *u, char *src) +{ + struct UrlQueryString *qs = NULL; + char *k = NULL, *v = NULL; + + while (src && *src) + { + qs = safe_calloc(1, sizeof(struct UrlQueryString)); + if ((k = strchr(src, '&'))) + *k = '\0'; + + if ((v = strchr(src, '='))) + { + *v = '\0'; + qs->value = v + 1; + if (url_pct_decode(qs->value) < 0) + { + FREE(&qs); + return -1; + } + } + qs->name = src; + if (url_pct_decode(qs->name) < 0) + { + FREE(&qs); + return -1; + } + STAILQ_INSERT_TAIL(&u->query_strings, qs, entries); + + if (!k) + break; + src = k + 1; + } + return 0; +} + + /** - * parse_userhost - fill in components of Url with info from src + * url_parse - Fill in Url + * @param u Url where the result is stored + * @param src String to parse + * @retval 0 String is valid + * @retval -1 String is invalid + * + * char* elements are pointers into src, which is modified by this call + * (duplicate it first if you need to). + * + * This method doesn't allocated any additional char* of the Url and + * UrlQueryString structs. * - * Note: These are pointers into src, which is altered with '\0's. - * Port of 0 means no port given. + * To free Url, caller must free "src" and call url_free() */ -static int parse_userhost(struct Url *u, char *src) +int url_parse(struct Url *u, char *src) { char *t = NULL, *p = NULL; + u->scheme = url_check_scheme(src); + if (u->scheme == U_UNKNOWN) + return -1; + + src = strchr(src, ':') + 1; + u->user = NULL; u->pass = NULL; u->host = NULL; u->port = 0; + u->path = NULL; + STAILQ_INIT(&u->query_strings); if (strncmp(src, "//", 2) != 0) { @@ -120,9 +175,19 @@ static int parse_userhost(struct Url *u, char *src) src += 2; + if ((t = strchr(src, '?'))) + { + *t++ = '\0'; + if (parse_query_string(u, t) < 0) + goto err; + } + if ((u->path = strchr(src, '/'))) *u->path++ = '\0'; + if (u->path && url_pct_decode(u->path) < 0) + goto err; + if ((t = strrchr(src, '@'))) { *t = '\0'; @@ -131,11 +196,11 @@ static int parse_userhost(struct Url *u, char *src) *p = '\0'; u->pass = p + 1; if (url_pct_decode(u->pass) < 0) - return -1; + goto err; } u->user = src; if (url_pct_decode(u->user) < 0) - return -1; + goto err; src = t + 1; } @@ -155,33 +220,45 @@ static int parse_userhost(struct Url *u, char *src) int num; *p++ = '\0'; if (mutt_atoi(p, &num) < 0 || num < 0 || num > 0xffff) - return -1; + goto err; u->port = (unsigned short) num; } else u->port = 0; - u->host = src; - return url_pct_decode(u->host) >= 0 && (!u->path || url_pct_decode(u->path) >= 0) ? 0 : -1; -} -/** - * url_parse - Fill in Url - * - * char* elements are pointers into src, which is modified by this call - * (duplicate it first if you need to). - */ -int url_parse(struct Url *u, char *src) -{ - char *tmp = NULL; + if (mutt_strlen(src) != 0) + { + u->host = src; + if (url_pct_decode(u->host) < 0) + goto err; + } + else if (u->path) + { + /* No host are provided, we restore the / because this is absolute path */ + u->path = src; + *src++ = '/'; + } - u->scheme = url_check_scheme(src); - if (u->scheme == U_UNKNOWN) - return -1; + return 0; - tmp = strchr(src, ':') + 1; +err: + url_free(u); + return -1; +} - return parse_userhost(u, tmp); +void url_free(struct Url *u) +{ + struct UrlQueryString *np = STAILQ_FIRST(&u->query_strings), *next = NULL; + while (np) + { + next = STAILQ_NEXT(np, entries); + /* NOTE(sileht): We don't free members, they will be freed when + * the src char* passed to url_parse() is freed */ + FREE(&np); + np = next; + } + STAILQ_INIT(&u->query_strings); } void url_pct_encode(char *dst, size_t l, const char *src) diff --git a/url.h b/url.h index 84243a50e..2c1051d10 100644 --- a/url.h +++ b/url.h @@ -22,6 +22,7 @@ #define _MUTT_URL_H #include +#include "lib/queue.h" struct Envelope; @@ -50,7 +51,22 @@ enum UrlScheme #define U_PATH (1 << 1) /** - * struct Url - A parsed URL `proto://user:password@host:port/path` + * struct UrlQueryString - Parsed Query String + * + * The arguments in a URL are saved in a linked list. + * + */ + +STAILQ_HEAD(UrlQueryStringHead, UrlQueryString); +struct UrlQueryString +{ + char *name; + char *value; + STAILQ_ENTRY(UrlQueryString) entries; +}; + +/** + * struct Url - A parsed URL `proto://user:password@host:port/path?a=1&b=2` */ struct Url { @@ -60,10 +76,12 @@ struct Url char *host; unsigned short port; char *path; + struct UrlQueryStringHead query_strings; }; enum UrlScheme url_check_scheme(const char *s); int url_parse(struct Url *u, char *src); +void url_free(struct Url *u); int url_tostring(struct Url *u, char *dest, size_t len, int flags); int url_parse_mailto(struct Envelope *e, char **body, const char *src); int url_pct_decode(char *s); -- 2.40.0