From d190af6db1f00d3b100de8037dc61b0cb6345dab Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sun, 29 Oct 2017 16:25:04 +0100 Subject: [PATCH 1/8] Restructure notification struct and add comments --- src/notification.h | 52 +++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/src/notification.h b/src/notification.h index c7da691..3318016 100644 --- a/src/notification.h +++ b/src/notification.h @@ -35,34 +35,44 @@ typedef struct _actions { } Actions; typedef struct _notification { + int id; + char *dbus_client; + char *appname; char *summary; char *body; - char *icon; - RawImage *raw_icon; - char *msg; /* formatted message */ char *category; - char *text_to_render; - const char *format; - char *dbus_client; - gint64 start; - gint64 timestamp; - gint64 timeout; enum urgency urgency; - enum markup_mode markup; - bool redisplayed; /* has been displayed before? */ - int id; - int dup_count; - int displayed_height; - const char *color_strings[3]; - bool first_render; - bool transient; - int progress; /* percentage (-1: undefined) */ - int history_ignore; - const char *script; - char *urls; + char *icon; /* plain icon information (may be a path or just a name) */ + RawImage *raw_icon; /* passed icon data of notification, takes precedence over icon */ + + gint64 start; /* begin of current display */ + gint64 timestamp; /* arrival time */ + gint64 timeout; /* time to display */ + Actions *actions; + + enum markup_mode markup; + const char *format; + const char *color_strings[3]; + const char *script; + + /* Hints */ + bool transient; /* timeout albeit user is idle */ + int progress; /* percentage (-1: undefined) */ + int history_ignore; /* push to history or free directly */ + + /* internal */ + bool redisplayed; /* has been displayed before? */ + bool first_render; /* markup has been rendered before? */ + int dup_count; /* amount of duplicate notifications stacked onto this */ + int displayed_height; + + /* derived fields */ + char *msg; /* formatted message */ + char *text_to_render; /* formatted message (with age and action indicators) */ + char *urls; /* urllist */ } notification; notification *notification_create(void); From eae071f3300937b7444a44e662dd3ccd7da09915 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sun, 29 Oct 2017 16:50:59 +0100 Subject: [PATCH 2/8] Rename color_strings to colors This is an antipattern and makes linelength increase for nothing --- src/dbus.c | 6 +++--- src/dunst.h | 2 +- src/notification.c | 18 +++++++++--------- src/notification.h | 2 +- src/rules.c | 4 ++-- src/x11/x.c | 30 +++++++++++++++--------------- src/x11/x.h | 2 +- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index bf9a815..d9b238c 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -284,10 +284,10 @@ static void on_notify(GDBusConnection *connection, n->actions = actions; for (int i = 0; i < ColLast; i++) { - n->color_strings[i] = NULL; + n->colors[i] = NULL; } - n->color_strings[ColFG] = fgcolor; - n->color_strings[ColBG] = bgcolor; + n->colors[ColFG] = fgcolor; + n->colors[ColBG] = bgcolor; notification_init(n); int id = queues_notification_insert(n, replaces_id); diff --git a/src/dunst.h b/src/dunst.h index 657cfdd..8667f44 100644 --- a/src/dunst.h +++ b/src/dunst.h @@ -17,7 +17,7 @@ #define ColBG 0 extern GSList *rules; -extern const char *color_strings[3][3]; +extern const char *colors[3][3]; /* return id of notification */ gboolean run(void *data); diff --git a/src/notification.c b/src/notification.c index 4459c58..ac77052 100644 --- a/src/notification.c +++ b/src/notification.c @@ -40,9 +40,9 @@ void notification_print(notification *n) printf("\turgency: %s\n", notification_urgency_to_string(n->urgency)); printf("\ttransient: %d\n", n->transient); printf("\tformatted: '%s'\n", n->msg); - printf("\tfg: %s\n", n->color_strings[ColFG]); - printf("\tbg: %s\n", n->color_strings[ColBG]); - printf("\tframe: %s\n", n->color_strings[ColFrame]); + printf("\tfg: %s\n", n->colors[ColFG]); + printf("\tbg: %s\n", n->colors[ColBG]); + printf("\tframe: %s\n", n->colors[ColFrame]); printf("\tid: %d\n", n->id); if (n->urls) { printf("\turls:\n"); @@ -447,16 +447,16 @@ void notification_init(notification *n) if (n->urgency > URG_MAX) n->urgency = URG_CRIT; - if (!n->color_strings[ColFG]) { - n->color_strings[ColFG] = xctx.color_strings[ColFG][n->urgency]; + if (!n->colors[ColFG]) { + n->colors[ColFG] = xctx.colors[ColFG][n->urgency]; } - if (!n->color_strings[ColBG]) { - n->color_strings[ColBG] = xctx.color_strings[ColBG][n->urgency]; + if (!n->colors[ColBG]) { + n->colors[ColBG] = xctx.colors[ColBG][n->urgency]; } - if (!n->color_strings[ColFrame]) { - n->color_strings[ColFrame] = xctx.color_strings[ColFrame][n->urgency]; + if (!n->colors[ColFrame]) { + n->colors[ColFrame] = xctx.colors[ColFrame][n->urgency]; } n->timeout = diff --git a/src/notification.h b/src/notification.h index 3318016..bffeec1 100644 --- a/src/notification.h +++ b/src/notification.h @@ -55,7 +55,7 @@ typedef struct _notification { enum markup_mode markup; const char *format; - const char *color_strings[3]; + const char *colors[3]; const char *script; /* Hints */ diff --git a/src/rules.c b/src/rules.c index 086c627..da64f9b 100644 --- a/src/rules.c +++ b/src/rules.c @@ -29,9 +29,9 @@ void rule_apply(rule_t *r, notification *n) n->raw_icon = NULL; } if (r->fg) - n->color_strings[ColFG] = r->fg; + n->colors[ColFG] = r->fg; if (r->bg) - n->color_strings[ColBG] = r->bg; + n->colors[ColBG] = r->bg; if (r->format) n->format = r->format; if (r->script) diff --git a/src/x11/x.c b/src/x11/x.c index 9a6c23a..ec9e8de 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -468,9 +468,9 @@ static colored_layout *r_init_shared(cairo_t *c, notification *n) cl->icon = NULL; } - cl->fg = x_string_to_color_t(n->color_strings[ColFG]); - cl->bg = x_string_to_color_t(n->color_strings[ColBG]); - cl->frame = x_string_to_color_t(n->color_strings[ColFrame]); + cl->fg = x_string_to_color_t(n->colors[ColFG]); + cl->bg = x_string_to_color_t(n->colors[ColBG]); + cl->frame = x_string_to_color_t(n->colors[ColFrame]); cl->n = n; @@ -985,26 +985,26 @@ void x_setup(void) x_shortcut_grab(&settings.context_ks); x_shortcut_ungrab(&settings.context_ks); - xctx.color_strings[ColFG][URG_LOW] = settings.lowfgcolor; - xctx.color_strings[ColFG][URG_NORM] = settings.normfgcolor; - xctx.color_strings[ColFG][URG_CRIT] = settings.critfgcolor; + xctx.colors[ColFG][URG_LOW] = settings.lowfgcolor; + xctx.colors[ColFG][URG_NORM] = settings.normfgcolor; + xctx.colors[ColFG][URG_CRIT] = settings.critfgcolor; - xctx.color_strings[ColBG][URG_LOW] = settings.lowbgcolor; - xctx.color_strings[ColBG][URG_NORM] = settings.normbgcolor; - xctx.color_strings[ColBG][URG_CRIT] = settings.critbgcolor; + xctx.colors[ColBG][URG_LOW] = settings.lowbgcolor; + xctx.colors[ColBG][URG_NORM] = settings.normbgcolor; + xctx.colors[ColBG][URG_CRIT] = settings.critbgcolor; if (settings.lowframecolor) - xctx.color_strings[ColFrame][URG_LOW] = settings.lowframecolor; + xctx.colors[ColFrame][URG_LOW] = settings.lowframecolor; else - xctx.color_strings[ColFrame][URG_LOW] = settings.frame_color; + xctx.colors[ColFrame][URG_LOW] = settings.frame_color; if (settings.normframecolor) - xctx.color_strings[ColFrame][URG_NORM] = settings.normframecolor; + xctx.colors[ColFrame][URG_NORM] = settings.normframecolor; else - xctx.color_strings[ColFrame][URG_NORM] = settings.frame_color; + xctx.colors[ColFrame][URG_NORM] = settings.frame_color; if (settings.critframecolor) - xctx.color_strings[ColFrame][URG_CRIT] = settings.critframecolor; + xctx.colors[ColFrame][URG_CRIT] = settings.critframecolor; else - xctx.color_strings[ColFrame][URG_CRIT] = settings.frame_color; + xctx.colors[ColFrame][URG_CRIT] = settings.frame_color; /* parse and set xctx.geometry and monitor position */ if (settings.geom[0] == '-') { diff --git a/src/x11/x.h b/src/x11/x.h index 1ccd9a3..3b08ca4 100644 --- a/src/x11/x.h +++ b/src/x11/x.h @@ -28,7 +28,7 @@ typedef struct _xctx { Window win; bool visible; dimension_t geometry; - const char *color_strings[3][3]; + const char *colors[3][3]; XScreenSaverInfo *screensaver_info; dimension_t window_dim; unsigned long sep_custom_col; From 4bfae81f18bd7739418d0a0b31d1623f4a22196f Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sun, 29 Oct 2017 18:45:29 +0100 Subject: [PATCH 3/8] Split and refactor notification_init Unboil the spaghetti code and split it into separate methods. --- src/dbus.c | 7 +-- src/notification.c | 142 +++++++++++++++++++++++++-------------------- 2 files changed, 80 insertions(+), 69 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index d9b238c..9af7c2e 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -264,14 +264,14 @@ static void on_notify(GDBusConnection *connection, fflush(stdout); notification *n = notification_create(); + n->appname = appname; n->summary = summary; n->body = body; n->icon = icon; n->raw_icon = raw_icon; n->timeout = timeout < 0 ? -1 : timeout * 1000; - n->markup = settings.markup; - n->progress = (progress < 0 || progress > 100) ? -1 : progress; + n->progress = progress; n->urgency = urgency; n->category = category; n->dbus_client = g_strdup(sender); @@ -283,9 +283,6 @@ static void on_notify(GDBusConnection *connection, } n->actions = actions; - for (int i = 0; i < ColLast; i++) { - n->colors[i] = NULL; - } n->colors[ColFG] = fgcolor; n->colors[ColBG] = bgcolor; diff --git a/src/notification.c b/src/notification.c index ac77052..7981a5d 100644 --- a/src/notification.c +++ b/src/notification.c @@ -23,6 +23,10 @@ #include "utils.h" #include "x11/x.h" +static void notification_extract_urls(notification *n); +static void notification_format_message(notification *n); +static void notification_dmenu_string(notification *n); + /* * print a human readable representation * of the given notification to stdout. @@ -289,54 +293,87 @@ char *notification_extract_markup_urls(char **str_ptr) } /* - * Create notification struct and initialise everything to NULL, - * this function is guaranteed to return a valid pointer. + * Create notification struct and initialise all fields with either + * - the default (if it's not needed to be freed later) + * - its undefined representation (NULL, -1) + * + * This function is guaranteed to return a valid pointer. + * @Returns: The generated notification */ notification *notification_create(void) { - return g_malloc0(sizeof(notification)); -} + notification *n = g_malloc0(sizeof(notification)); -void notification_init_defaults(notification *n) -{ - assert(n != NULL); - if(n->appname == NULL) n->appname = g_strdup("unknown"); - if(n->summary == NULL) n->summary = g_strdup(""); - if(n->body == NULL) n->body = g_strdup(""); - if(n->category == NULL) n->category = g_strdup(""); + /* Unparameterized default values */ + n->first_render = true; + n->markup = settings.markup; + n->format = settings.format; + + n->timestamp = g_get_monotonic_time(); + + n->urgency = URG_NORM; + n->timeout = -1; + + n->transient = false; + n->progress = -1; + + return n; } /* - * Initialize the given notification + * Sanitize values of notification, apply all matching rules + * and generate derived fields. * - * n should be a pointer to a notification allocated with - * notification_create, it is undefined behaviour to pass a notification - * allocated some other way. + * @n: the notification to sanitize */ void notification_init(notification *n) { - assert(n != NULL); + /* default to empty string to avoid further NULL faults */ + n->appname = n->appname ? n->appname : g_strdup("unknown"); + n->summary = n->summary ? n->summary : g_strdup(""); + n->body = n->body ? n->body : g_strdup(""); + n->category = n->category ? n->category : g_strdup(""); - //Prevent undefined behaviour by initialising required fields - notification_init_defaults(n); + /* sanitize urgency */ + if (n->urgency < URG_MIN) + n->urgency = URG_LOW; + if (n->urgency > URG_MAX) + n->urgency = URG_CRIT; - n->script = NULL; - n->text_to_render = NULL; + /* Timeout processing */ + if (n->timeout < 0) + n->timeout = settings.timeouts[n->urgency]; - n->format = settings.format; + /* Icon handling */ + if (n->icon && strlen(n->icon) <= 0) + g_clear_pointer(&n->icon, g_free); + if (!n->raw_icon && !n->icon) + n->icon = g_strdup(settings.icons[n->urgency]); + /* Color hints */ + if (!n->colors[ColFG]) + n->colors[ColFG] = xctx.colors[ColFG][n->urgency]; + if (!n->colors[ColBG]) + n->colors[ColBG] = xctx.colors[ColBG][n->urgency]; + if (!n->colors[ColFrame]) + n->colors[ColFrame] = xctx.colors[ColFrame][n->urgency]; + + /* Sanitize misc hints */ + if (n->progress < 0 || n->progress > 100) + n->progress = -1; + + /* Process rules */ rule_apply_all(n); - if (n->icon != NULL && strlen(n->icon) <= 0) { - g_free(n->icon); - n->icon = NULL; - } + /* UPDATE derived fields */ + notification_extract_urls(n); + notification_dmenu_string(n); + notification_format_message(n); +} - if (n->raw_icon == NULL && n->icon == NULL) { - n->icon = g_strdup(settings.icons[n->urgency]); - } - - n->urls = notification_extract_markup_urls(&(n->body)); +static void notification_format_message(notification *n) +{ + g_clear_pointer(&n->msg, g_free); n->msg = string_replace_all("\\n", "\n", g_strdup(n->format)); @@ -438,45 +475,25 @@ void notification_init(notification *n) g_free(n->msg); n->msg = buffer; } +} - n->dup_count = 0; - - /* urgency > URG_CRIT -> array out of range */ - if (n->urgency < URG_MIN) - n->urgency = URG_LOW; - if (n->urgency > URG_MAX) - n->urgency = URG_CRIT; - - if (!n->colors[ColFG]) { - n->colors[ColFG] = xctx.colors[ColFG][n->urgency]; - } - - if (!n->colors[ColBG]) { - n->colors[ColBG] = xctx.colors[ColBG][n->urgency]; - } - - if (!n->colors[ColFrame]) { - n->colors[ColFrame] = xctx.colors[ColFrame][n->urgency]; - } - - n->timeout = - n->timeout < 0 ? settings.timeouts[n->urgency] : n->timeout; - n->start = 0; - - n->timestamp = g_get_monotonic_time(); - - n->redisplayed = false; - - n->first_render = true; +static void notification_extract_urls(notification *n) +{ + // DO markup urls processing here until we split this out correctly + n->urls = notification_extract_markup_urls(&(n->body)); char *tmp = g_strconcat(n->summary, " ", n->body, NULL); char *tmp_urls = extract_urls(tmp); n->urls = string_append(n->urls, tmp_urls, "\n"); g_free(tmp_urls); + g_free(tmp); +} +static void notification_dmenu_string(notification *n) +{ if (n->actions) { - n->actions->dmenu_str = NULL; + g_clear_pointer(&n->actions->dmenu_str, g_free); for (int i = 0; i < n->actions->count; i += 2) { char *human_readable = n->actions->actions[i + 1]; string_replace_char('[', '(', human_readable); // kill square brackets @@ -489,14 +506,11 @@ void notification_init(notification *n) } } } - - g_free(tmp); } void notification_update_text_to_render(notification *n) { - g_free(n->text_to_render); - n->text_to_render = NULL; + g_clear_pointer(&n->text_to_render, g_free); char *buf = NULL; From acd8be51ab169debec751cb3da695c28b6295675 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Fri, 3 Nov 2017 02:02:44 +0100 Subject: [PATCH 4/8] Remove a and img tags from msg While the notification spec allows tags like ... and ..., pango cannot parse these tags and therefore these tags should be removed before passed to pango. Also the method notification_extract_markup_urls is not needed anymore, as markup_strip_a can return URLs optionally. This implies, that URL replacement is now indicated via show_indicators for URLs and the dmenu string is in the format of '[text between a tags] URL\n'. This is similarly handled for images, too. --- src/markup.c | 176 +++++++++++++++++++++++++++++++++++++++++++++ src/markup.h | 4 ++ src/notification.c | 65 +++++------------ src/notification.h | 2 +- test/markup.c | 96 +++++++++++++++++++++++++ 5 files changed, 295 insertions(+), 48 deletions(-) diff --git a/src/markup.c b/src/markup.c index cd91ff8..ad4484f 100644 --- a/src/markup.c +++ b/src/markup.c @@ -4,6 +4,8 @@ #include #include +#include +#include #include "settings.h" #include "utils.h" @@ -44,6 +46,178 @@ static char *markup_br2nl(char *str) return str; } +/* + * Remove HTML hyperlinks of a string. + * + * @str: The string to replace a tags + * @urls: (nullable): If any href-attributes found, an '\n' concatenated + * string of the URLs in format '[] ' + */ +void markup_strip_a(char **str, char **urls) +{ + char *tag1 = NULL; + + if (urls) + *urls = NULL; + + while ((tag1 = strstr(*str, ""); + char *tag2 = strstr(tag1, ""); + + // the tag is broken, ignore it + if (!tag1_end) { + fprintf(stderr, + "WARNING: Given link is broken: '%s'\n", + tag1); + string_replace_at(*str, tag1-*str, strlen(tag1), ""); + break; + } + if (tag2 && tag2 < tag1_end) { + int repl_len = (tag2 - tag1) + strlen(""); + fprintf(stderr, + "WARNING: Given link is broken: '%.*s.'\n", + repl_len, tag1); + string_replace_at(*str, tag1-*str, repl_len, ""); + break; + } + + // search contents of href attribute + char *plain_url = NULL; + if (href && href < tag1_end) { + + // shift href to the actual begin of the value + href = href+6; + + const char *quote = strstr(href, "\""); + + if (quote && quote < tag1_end) { + plain_url = g_strndup(href, quote-href); + } + } + + // text between a tags + int text_len; + if (tag2) + text_len = tag2 - (tag1_end+1); + else + text_len = strlen(tag1_end+1); + + char *text = g_strndup(tag1_end+1, text_len); + + int repl_len = text_len + (tag1_end-tag1) + 1; + repl_len += tag2 ? strlen("") : 0; + + *str = string_replace_at(*str, tag1-*str, repl_len, text); + + // if there had been a href attribute, + // add it to the URLs + if (plain_url && urls) { + text = string_replace_all("]", "", text); + text = string_replace_all("[", "", text); + + char *url = g_strdup_printf("[%s] %s", text, plain_url); + + *urls = string_append(*urls, url, "\n"); + g_free(url); + } + + g_free(plain_url); + g_free(text); + } +} + +/* + * Remove img-tags of a string. If alt attribute given, use this as replacement. + * + * @str: The string to replace img tags + * @urls: (nullable): If any src-attributes found, an '\n' concatenated string of + * the URLs in format '[] ' + */ +void markup_strip_img(char **str, char **urls) +{ + const char *start = *str; + + if (urls) + *urls = NULL; + + while ((start = strstr(*str, ""); + + // the tag is broken, ignore it + if (!end) { + fprintf(stderr, "WARNING: Given image is broken: '%s'\n", start); + string_replace_at(*str, start-*str, strlen(start), ""); + break; + } + + // use attribute=" as stated in the notification spec + const char *alt_s = strstr(start, "alt=\""); + const char *src_s = strstr(start, "src=\""); + + char *text_alt = NULL; + char *text_src = NULL; + + const char *src_e = NULL, *alt_e = NULL; + if (alt_s) + alt_e = strstr(alt_s + strlen("alt=\""), "\""); + if (src_s) + src_e = strstr(src_s + strlen("src=\""), "\""); + + // Move pointer to the actual start + alt_s = alt_s ? alt_s + strlen("alt=\"") : NULL; + src_s = src_s ? src_s + strlen("src=\"") : NULL; + + /* check if alt and src attribute are given + * If both given, check the alignment of all pointers */ + if ( alt_s && alt_e + && src_s && src_e + && ( (alt_s < src_s && alt_e < src_s-strlen("src=\"") && src_e < end) + ||(src_s < alt_s && src_e < alt_s-strlen("alt=\"") && alt_e < end)) ) { + + text_alt = g_strndup(alt_s, alt_e-alt_s); + text_src = g_strndup(src_s, src_e-src_s); + + /* check if single valid alt attribute is available */ + } else if (alt_s && alt_e && alt_e < end && (!src_s || src_s < alt_s || alt_e < src_s - strlen("src=\""))) { + text_alt = g_strndup(alt_s, alt_e-alt_s); + + /* check if single valid src attribute is available */ + } else if (src_s && src_e && src_e < end && (!alt_s || alt_s < src_s || src_e < alt_s - strlen("alt=\""))) { + text_src = g_strndup(src_s, src_e-src_s); + + } else { + fprintf(stderr, + "WARNING: Given image argument is broken: '%.*s'\n", + (int)(end-start), start); + } + + // replacement text for alt + int repl_len = end - start + 1; + + if (!text_alt) + text_alt = g_strdup("[image]"); + + *str = string_replace_at(*str, start-*str, repl_len, text_alt); + + // if there had been a href attribute, + // add it to the URLs + if (text_src && urls) { + text_alt = string_replace_all("]", "", text_alt); + text_alt = string_replace_all("[", "", text_alt); + + char *url = g_strdup_printf("[%s] %s", text_alt, text_src); + + *urls = string_append(*urls, url, "\n"); + g_free(url); + } + + g_free(text_src); + g_free(text_alt); + } +} + /* * Strip any markup from text; turn it in to plain text. * @@ -96,6 +270,8 @@ char *markup_transform(char *str, enum markup_mode markup_mode) break; case MARKUP_FULL: str = markup_br2nl(str); + markup_strip_a(&str, NULL); + markup_strip_img(&str, NULL); break; } diff --git a/src/markup.h b/src/markup.h index 8304e2d..9d5cda7 100644 --- a/src/markup.h +++ b/src/markup.h @@ -5,6 +5,10 @@ #include "settings.h" char *markup_strip(char *str); + +void markup_strip_a(char **str, char **urls); +void markup_strip_img(char **str, char **urls); + char *markup_transform(char *str, enum markup_mode markup_mode); #endif diff --git a/src/notification.c b/src/notification.c index 7981a5d..7ae2254 100644 --- a/src/notification.c +++ b/src/notification.c @@ -252,46 +252,6 @@ void notification_replace_single_field(char **haystack, g_free(input); } -char *notification_extract_markup_urls(char **str_ptr) -{ - char *start, *end, *replace_buf, *str, *urls = NULL, *url, *index_buf; - int linkno = 1; - - str = *str_ptr; - while ((start = strstr(str, ""); - if (end != NULL) { - replace_buf = g_strndup(start, end - start + 1); - url = extract_urls(replace_buf); - if (url != NULL) { - str = string_replace(replace_buf, "[", str); - - index_buf = g_strdup_printf("[#%d]", linkno++); - if (urls == NULL) { - urls = g_strconcat(index_buf, " ", url, NULL); - } else { - char *tmp = urls; - urls = g_strconcat(tmp, "\n", index_buf, " ", url, NULL); - g_free(tmp); - } - - index_buf[0] = ' '; - str = string_replace("", index_buf, str); - g_free(index_buf); - g_free(url); - } else { - str = string_replace(replace_buf, "", str); - str = string_replace("", "", str); - } - g_free(replace_buf); - } else { - break; - } - } - *str_ptr = str; - return urls; -} - /* * Create notification struct and initialise all fields with either * - the default (if it's not needed to be freed later) @@ -479,15 +439,26 @@ static void notification_format_message(notification *n) static void notification_extract_urls(notification *n) { - // DO markup urls processing here until we split this out correctly - n->urls = notification_extract_markup_urls(&(n->body)); + g_clear_pointer(&n->urls, g_free); - char *tmp = g_strconcat(n->summary, " ", n->body, NULL); + char *urls_in = string_append(g_strdup(n->summary), n->body, " "); - char *tmp_urls = extract_urls(tmp); - n->urls = string_append(n->urls, tmp_urls, "\n"); - g_free(tmp_urls); - g_free(tmp); + char *urls_a = NULL; + char *urls_img = NULL; + markup_strip_a(&urls_in, &urls_a); + markup_strip_img(&urls_in, &urls_img); + // remove links and images first to not confuse + // plain urls extraction + char *urls_text = extract_urls(urls_in); + + n->urls = string_append(n->urls, urls_a, "\n"); + n->urls = string_append(n->urls, urls_img, "\n"); + n->urls = string_append(n->urls, urls_text, "\n"); + + g_free(urls_in); + g_free(urls_a); + g_free(urls_img); + g_free(urls_text); } static void notification_dmenu_string(notification *n) diff --git a/src/notification.h b/src/notification.h index bffeec1..c83077a 100644 --- a/src/notification.h +++ b/src/notification.h @@ -72,7 +72,7 @@ typedef struct _notification { /* derived fields */ char *msg; /* formatted message */ char *text_to_render; /* formatted message (with age and action indicators) */ - char *urls; /* urllist */ + char *urls; /* urllist delimited by '\n' */ } notification; notification *notification_create(void); diff --git a/test/markup.c b/test/markup.c index cbd8bad..76f2b91 100644 --- a/test/markup.c +++ b/test/markup.c @@ -45,12 +45,108 @@ TEST test_markup_transform(void) ASSERT_STR_EQ("foo bar baz", (ptr=markup_transform(g_strdup("foo
bar\nbaz"), MARKUP_FULL))); g_free(ptr); + // Test replacement of img and a tags, not renderable by pango + ASSERT_STR_EQ("foo bar bar baz", (ptr=markup_transform(g_strdup("\"foo
bar\nbaz"), MARKUP_FULL))); + g_free(ptr); + ASSERT_STR_EQ("test ", (ptr=markup_transform(g_strdup("test \"foo image"), MARKUP_FULL))); + g_free(ptr); + ASSERT_STR_EQ("bar baz", (ptr=markup_transform(g_strdup("bar baz"), MARKUP_FULL))); + g_free(ptr); + + PASS(); +} + +TEST helper_markup_strip_a (const char *in, const char *exp, const char *urls) +{ + // out_urls is a return parameter and the content should be ignored + char *out_urls = (char *)0x04; //Chosen by a fair dice roll + char *out = g_strdup(in); + char *msg = g_strconcat("url: ", in, NULL); + + markup_strip_a(&out, &out_urls); + + ASSERT_STR_EQm(msg, exp, out); + + if (urls) { + ASSERT_STR_EQm(msg, urls, out_urls); + } else { + ASSERT_EQm(msg, urls, out_urls); + } + + g_free(out_urls); + g_free(out); + g_free(msg); + + PASS(); +} + +TEST test_markup_strip_a(void) +{ + RUN_TESTp(helper_markup_strip_a, "valid link", "valid link", "[valid] https://url.com"); + RUN_TESTp(helper_markup_strip_a, "valid link", "valid link", "[valid] "); + RUN_TESTp(helper_markup_strip_a, "valid link", "valid link", NULL); + RUN_TESTp(helper_markup_strip_a, "valid link", "valid link", "[valid link] https://url.com"); + + RUN_TESTp(helper_markup_strip_a, " link", " link", NULL); + RUN_TESTp(helper_markup_strip_a, " link", " link", NULL); + + PASS(); +} + +TEST helper_markup_strip_img (const char *in, const char *exp, const char *urls) +{ + // out_urls is a return parameter and the content should be ignored + char *out_urls = (char *)0x04; //Chosen by a fair dice roll + char *out = g_strdup(in); + char *msg = g_strconcat("url: ", in, NULL); + + markup_strip_img(&out, &out_urls); + + ASSERT_STR_EQm(msg, exp, out); + + if (urls) { + ASSERT_STR_EQm(msg, urls, out_urls); + } else { + ASSERT_EQm(msg, urls, out_urls); + } + + g_free(out_urls); + g_free(out); + g_free(msg); + + PASS(); +} + +TEST test_markup_strip_img(void) +{ + RUN_TESTp(helper_markup_strip_img, "v img", "v [image] img", NULL); + RUN_TESTp(helper_markup_strip_img, "v \"valid\" img", "v valid img", NULL); + RUN_TESTp(helper_markup_strip_img, "v img", "v [image] img", "[image] url.com"); + + RUN_TESTp(helper_markup_strip_img, "v \"valid\" img", "v valid img", "[valid] url.com"); + RUN_TESTp(helper_markup_strip_img, "v \"valid\" img", "v valid img", "[valid] url.com"); + RUN_TESTp(helper_markup_strip_img, "v \"valid\" img", "v valid img", "[valid] url.com"); + + RUN_TESTp(helper_markup_strip_img, "i \"invalid img", "i [image] img", "[image] https://url.com"); + RUN_TESTp(helper_markup_strip_img, "i \"broken\" img", "i broken img", NULL); + RUN_TESTp(helper_markup_strip_img, "i \"invalid img", "i [image] img", NULL); + + RUN_TESTp(helper_markup_strip_img, "i \"broken\" img", "i broken img", NULL); + RUN_TESTp(helper_markup_strip_img, "i \"invalid img", "i [image] img", "[image] url.com"); + RUN_TESTp(helper_markup_strip_img, "i \"invalid img", "i [image] img", NULL); + + RUN_TESTp(helper_markup_strip_img, "i \"invalid\" Date: Thu, 23 Nov 2017 14:30:02 +0100 Subject: [PATCH 5/8] Strip [linktext] before opening URL [linktext] can contain arbitary data and also a possible URL, which would get passed to the browser, making the actual URL invalid. --- src/menu.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/menu.c b/src/menu.c index b45cdb5..a90ec20 100644 --- a/src/menu.c +++ b/src/menu.c @@ -95,10 +95,14 @@ char *extract_urls(const char *to_match) */ void open_browser(const char *in) { - // remove prefix and test url - char *url = extract_urls(in); - if (!url) - return; + char *url = NULL; + + // If any, remove leading [ linktext ] from URL + const char *end = strstr(in, "] "); + if (*in == '[' && end) + url = g_strdup(end + 2); + else + url = g_strdup(in); int browser_pid1 = fork(); From b97a49c09ba1d5a6033a29f5e84d54bc4e6c46c8 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sat, 4 Nov 2017 18:58:55 +0100 Subject: [PATCH 6/8] Fix memory leak colors given via hints The hints given via DBus are not constants. Therefore the color fields have to get freed during notification cleanup. As it's not possible to disinguish, which field is constant, we have to duplicate the settings on assignment. --- src/notification.c | 9 ++++++--- src/notification.h | 2 +- src/rules.c | 12 ++++++++---- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/notification.c b/src/notification.c index 7ae2254..5250375 100644 --- a/src/notification.c +++ b/src/notification.c @@ -214,6 +214,9 @@ void notification_free(notification *n) g_free(n->category); g_free(n->text_to_render); g_free(n->urls); + g_free(n->colors[ColFG]); + g_free(n->colors[ColBG]); + g_free(n->colors[ColFrame]); actions_free(n->actions); rawimage_free(n->raw_icon); @@ -312,11 +315,11 @@ void notification_init(notification *n) /* Color hints */ if (!n->colors[ColFG]) - n->colors[ColFG] = xctx.colors[ColFG][n->urgency]; + n->colors[ColFG] = g_strdup(xctx.colors[ColFG][n->urgency]); if (!n->colors[ColBG]) - n->colors[ColBG] = xctx.colors[ColBG][n->urgency]; + n->colors[ColBG] = g_strdup(xctx.colors[ColBG][n->urgency]); if (!n->colors[ColFrame]) - n->colors[ColFrame] = xctx.colors[ColFrame][n->urgency]; + n->colors[ColFrame] = g_strdup(xctx.colors[ColFrame][n->urgency]); /* Sanitize misc hints */ if (n->progress < 0 || n->progress > 100) diff --git a/src/notification.h b/src/notification.h index c83077a..5b05e8a 100644 --- a/src/notification.h +++ b/src/notification.h @@ -55,8 +55,8 @@ typedef struct _notification { enum markup_mode markup; const char *format; - const char *colors[3]; const char *script; + char *colors[3]; /* Hints */ bool transient; /* timeout albeit user is idle */ diff --git a/src/rules.c b/src/rules.c index da64f9b..7be1abb 100644 --- a/src/rules.c +++ b/src/rules.c @@ -28,10 +28,14 @@ void rule_apply(rule_t *r, notification *n) rawimage_free(n->raw_icon); n->raw_icon = NULL; } - if (r->fg) - n->colors[ColFG] = r->fg; - if (r->bg) - n->colors[ColBG] = r->bg; + if (r->fg) { + g_free(n->colors[ColFG]); + n->colors[ColFG] = g_strdup(r->fg); + } + if (r->bg) { + g_free(n->colors[ColBG]); + n->colors[ColBG] = g_strdup(r->bg); + } if (r->format) n->format = r->format; if (r->script) From 3face4ae72fef7e38b8750c4d3a7f0612032ed84 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Sun, 3 Dec 2017 14:55:03 +0100 Subject: [PATCH 7/8] Split notification assembly into separate method Handle replaces_id now via n->id --- src/dbus.c | 17 ++++++++++++----- src/dunst.c | 3 ++- src/queues.c | 5 ++--- src/queues.h | 6 +++--- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/dbus.c b/src/dbus.c index 9af7c2e..bbf5a12 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -124,10 +124,7 @@ static void on_get_capabilities(GDBusConnection *connection, g_dbus_connection_flush(connection, NULL, NULL, NULL); } -static void on_notify(GDBusConnection *connection, - const gchar *sender, - GVariant *parameters, - GDBusMethodInvocation *invocation) +static notification *dbus_message_to_notification(const gchar *sender, GVariant *parameters) { gchar *appname = NULL; @@ -265,6 +262,7 @@ static void on_notify(GDBusConnection *connection, notification *n = notification_create(); + n->id = replaces_id; n->appname = appname; n->summary = summary; n->body = body; @@ -287,7 +285,16 @@ static void on_notify(GDBusConnection *connection, n->colors[ColBG] = bgcolor; notification_init(n); - int id = queues_notification_insert(n, replaces_id); + return n; +} + +static void on_notify(GDBusConnection *connection, + const gchar *sender, + GVariant *parameters, + GDBusMethodInvocation *invocation) +{ + notification *n = dbus_message_to_notification(sender, parameters); + int id = queues_notification_insert(n); GVariant *reply = g_variant_new("(u)", id); g_dbus_method_invocation_return_value(invocation, reply); diff --git a/src/dunst.c b/src/dunst.c index 5d55467..c8e15ba 100644 --- a/src/dunst.c +++ b/src/dunst.c @@ -151,6 +151,7 @@ int dunst_main(int argc, char *argv[]) if (settings.startup_notification) { notification *n = notification_create(); + n->id = 0; n->appname = g_strdup("dunst"); n->summary = g_strdup("startup"); n->body = g_strdup("dunst is up and running"); @@ -159,7 +160,7 @@ int dunst_main(int argc, char *argv[]) n->markup = MARKUP_NO; n->urgency = URG_LOW; notification_init(n); - queues_notification_insert(n, 0); + queues_notification_insert(n); // we do not call wakeup now, wake_up does not work here yet } diff --git a/src/queues.c b/src/queues.c index fa9da51..924b646 100644 --- a/src/queues.c +++ b/src/queues.c @@ -51,7 +51,7 @@ unsigned int queues_length_history() return history->length; } -int queues_notification_insert(notification *n, int replaces_id) +int queues_notification_insert(notification *n) { /* do not display the message, if the message is empty */ @@ -72,12 +72,11 @@ int queues_notification_insert(notification *n, int replaces_id) return 0; } - if (replaces_id == 0) { + if (n->id == 0) { n->id = ++next_notification_id; if (!settings.stack_duplicates || !queues_stack_duplicate(n)) g_queue_insert_sorted(waiting, n, notification_cmp_data, NULL); } else { - n->id = replaces_id; if (!queues_notification_replace_id(n)) g_queue_insert_sorted(waiting, n, notification_cmp_data, NULL); } diff --git a/src/queues.h b/src/queues.h index f280cd5..dab452b 100644 --- a/src/queues.h +++ b/src/queues.h @@ -34,13 +34,13 @@ unsigned int queues_length_history(); * Insert a fully initialized notification into queues * Respects stack_duplicates, and notification replacement * - * If replaces_id != 0, n replaces notification with id replaces_id - * If replaces_id == 0, n gets occupies a new position + * If n->id != 0, n replaces notification with id n->id + * If n->id == 0, n gets a new id assigned * * Returns the assigned notification id * If returned id == 0, the message was dismissed */ -int queues_notification_insert(notification *n, int replaces_id); +int queues_notification_insert(notification *n); /* * Replace the notification which matches the id field of From aae73d888eb52071848185f19c78cc180c33acb2 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 4 Dec 2017 20:31:52 +0100 Subject: [PATCH 8/8] Show multiple URLs pretty with print --- src/notification.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/notification.c b/src/notification.c index 5250375..3eec08b 100644 --- a/src/notification.c +++ b/src/notification.c @@ -49,10 +49,12 @@ void notification_print(notification *n) printf("\tframe: %s\n", n->colors[ColFrame]); printf("\tid: %d\n", n->id); if (n->urls) { + char *urls = string_replace_all("\n", "\t\t\n", g_strdup(n->urls)); printf("\turls:\n"); printf("\t{\n"); - printf("\t\t%s\n", n->urls); + printf("\t\t%s\n", urls); printf("\t}\n"); + g_free(urls); } if (n->actions) {