From ab9bf55892c8c05302ef7d8b012924609050c67f Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 28 Aug 2017 11:23:21 +0200 Subject: [PATCH 1/3] Prevent replacement of format strings in message To replace all occuring format strings inside the msg, replace_all had been used previously. This leads to buggy behavior, if a format string occurs in the replaced text, as the format string will get replaced again under certain conditions. Introducing a pointer, which skips the already replaced parts, will prevent doubly replacing format strings from content. Fixes #322 --- CHANGELOG.md | 1 + src/notification.c | 129 +++++++++++++++++++++++++++++++++----------- src/notification.h | 2 +- src/utils.h | 3 ++ test/notification.c | 20 ++++--- 5 files changed, 115 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2264f8d..8e8efbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed - `new_icon` rule being ignored on notifications that had a raw icon +- Do not replace format strings, which are in notification content ## Changed - transient hints are now handled diff --git a/src/notification.c b/src/notification.c index 621f7d7..f6ac0f6 100644 --- a/src/notification.c +++ b/src/notification.c @@ -200,17 +200,33 @@ void notification_free(notification *n) } /* - * Replace all occurrences of "needle" with a quoted "replacement", - * according to the markup settings. + * Replace the two chars where **needle points + * with a quoted "replacement", according to the markup settings. + * + * The needle is a double pointer and gets updated upon return + * to point to the first char, which occurs after replacement. + * */ -char *notification_replace_format(const char *needle, const char *replacement, - char *haystack, enum markup_mode markup_mode) { - char *tmp; +char *notification_replace_single_field(char *haystack, char **needle, + const char *replacement, enum markup_mode markup_mode) { + + assert(*needle[0] == '%'); + // needle has to point into haystack (but not on the last char) + assert(*needle >= haystack); + assert(*needle - haystack < strlen(haystack) - 1); + char *ret; - tmp = markup_transform(g_strdup(replacement), markup_mode); - ret = string_replace_all(needle, tmp, haystack); - g_free(tmp); + int pos = *needle - haystack; + + char *input = markup_transform(g_strdup(replacement), markup_mode); + ret = string_replace_at(haystack, pos, 2, input); + + // point the needle to the next char + // which was originally in haystack + *needle = ret + (*needle - haystack) + strlen(input); + + g_free(input); return ret; } @@ -307,31 +323,82 @@ int notification_init(notification *n, int id) n->urls = notification_extract_markup_urls(&(n->body)); n->msg = string_replace_all("\\n", "\n", g_strdup(n->format)); - n->msg = notification_replace_format("%a", n->appname, n->msg, - MARKUP_NO); - n->msg = notification_replace_format("%s", n->summary, n->msg, - n->markup); - n->msg = notification_replace_format("%b", n->body, n->msg, - n->markup); - if (n->icon) { - char *tmp = g_strdup(n->icon); - n->msg = notification_replace_format("%I", basename(tmp), - n->msg, MARKUP_NO); - n->msg = notification_replace_format("%i", n->icon, - n->msg, MARKUP_NO); - g_free(tmp); - } + /* replace all formatter */ + for(char *substr = strchr(n->msg, '%'); + substr; + substr = strchr(substr, '%')){ - if (n->progress) { - char pg[10]; - sprintf(pg, "[%3d%%]", n->progress - 1); - n->msg = string_replace_all("%p", pg, n->msg); - sprintf(pg, "%d", n->progress - 1); - n->msg = string_replace_all("%n", pg, n->msg); - } else { - n->msg = string_replace_all("%p", "", n->msg); - n->msg = string_replace_all("%n", "", n->msg); + char pg[16]; + + switch(substr[1]){ + case 'a': + n->msg = notification_replace_single_field( + n->msg, + &substr, + n->appname, + MARKUP_NO); + break; + case 's': + n->msg = notification_replace_single_field( + n->msg, + &substr, + n->summary, + n->markup); + break; + case 'b': + n->msg = notification_replace_single_field( + n->msg, + &substr, + n->body, + n->markup); + break; + case 'I': + n->msg = notification_replace_single_field( + n->msg, + &substr, + n->icon ? basename(n->icon) : "", + MARKUP_NO); + break; + case 'i': + n->msg = notification_replace_single_field( + n->msg, + &substr, + n->icon ? n->icon : "", + MARKUP_NO); + break; + case 'p': + if (n->progress) + sprintf(pg, "[%3d%%]", n->progress - 1); + + n->msg = notification_replace_single_field( + n->msg, + &substr, + n->progress ? pg : "", + MARKUP_NO); + break; + case 'n': + if (n->progress) + sprintf(pg, "%d", n->progress - 1); + + n->msg = notification_replace_single_field( + n->msg, + &substr, + n->progress ? pg : "", + MARKUP_NO); + break; + case '\0': + fprintf(stderr, "WARNING: format_string has trailing %% character." + "To escape it use %%%%."); + break; + default: + fprintf(stderr, "WARNING: format_string %%%c" + " is unknown\n", substr[1]); + // shift substr pointer forward, + // as we can't interpret the format string + substr++; + break; + } } n->msg = g_strchomp(n->msg); diff --git a/src/notification.h b/src/notification.h index b653575..dadfc3b 100644 --- a/src/notification.h +++ b/src/notification.h @@ -73,7 +73,7 @@ int notification_is_duplicate(const notification *a, const notification *b); void notification_run_script(notification *n); int notification_close(notification *n, int reason); void notification_print(notification *n); -char *notification_replace_format(const char *needle, const char *replacement, char *haystack, enum markup_mode markup); +char *notification_replace_single_field(char *haystack, char **needle, const char *replacement, enum markup_mode markup_mode); void notification_update_text_to_render(notification *n); int notification_get_ttl(notification *n); int notification_get_age(notification *n); diff --git a/src/utils.h b/src/utils.h index 07bc5aa..9b46a1b 100644 --- a/src/utils.h +++ b/src/utils.h @@ -9,6 +9,9 @@ char *string_replace_char(char needle, char replacement, char *haystack); char *string_replace_all(const char *needle, const char *replacement, char *haystack); +/* replace characters with at position of the string */ +char *string_replace_at(char *buf, int pos, int len, const char *repl); + /* replace needle with replacement in haystack */ char *string_replace(const char *needle, const char *replacement, char *haystack); diff --git a/test/notification.c b/test/notification.c index 8f513c1..d077959 100644 --- a/test/notification.c +++ b/test/notification.c @@ -70,21 +70,25 @@ TEST test_notification_is_duplicate(void *notifications) PASS(); } -TEST test_notification_replace_format(void) +TEST test_notification_replace_single_field(void) { char *str = g_malloc(128 * sizeof(char)); - - strcpy(str, "Testing format replacement"); - ASSERT_STR_EQ("Testing text replacement", (str = notification_replace_format("format", "text", str, MARKUP_FULL))); + char *substr = NULL; strcpy(str, "Markup %a preserved"); - ASSERT_STR_EQ("Markup and & is preserved", (str = notification_replace_format("%a", "and & is", str, MARKUP_FULL))); + substr = strchr(str, '%'); + ASSERT_STR_EQ("Markup and & is preserved", (str = notification_replace_single_field(str, &substr, "and & is", MARKUP_FULL))); + ASSERT_EQ(26, substr - str); strcpy(str, "Markup %a escaped"); - ASSERT_STR_EQ("Markup and & <i>is</i> escaped", (str = notification_replace_format("%a", "and & is", str, MARKUP_NO))); + substr = strchr(str, '%'); + ASSERT_STR_EQ("Markup and & <i>is</i> escaped", (str = notification_replace_single_field(str, &substr, "and & is", MARKUP_NO))); + ASSERT_EQ(38, substr - str); strcpy(str, "Markup %a"); - ASSERT_STR_EQ("Markup is removed and & escaped", (str = notification_replace_format("%a", "is removed and & escaped", str, MARKUP_STRIP))); + substr = strchr(str, '%'); + ASSERT_STR_EQ("Markup is removed and & escaped", (str = notification_replace_single_field(str, &substr, "is removed and & escaped", MARKUP_STRIP))); + ASSERT_EQ(35, substr - str); g_free(str); PASS(); @@ -112,7 +116,7 @@ SUITE(suite_notification) g_free(a); g_free(b); - RUN_TEST(test_notification_replace_format); + RUN_TEST(test_notification_replace_single_field); } /* vim: set tabstop=8 shiftwidth=8 expandtab textwidth=0: */ From 4b7f656f6e994df5e38c4511c66fed68ab9f5f91 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Wed, 6 Sep 2017 21:38:43 +0200 Subject: [PATCH 2/3] Interpret '%%' as literal '%' in format --- docs/dunst.pod | 2 ++ dunstrc | 1 + src/notification.c | 7 +++++++ 3 files changed, 10 insertions(+) diff --git a/docs/dunst.pod b/docs/dunst.pod index d334ea0..5d3c677 100644 --- a/docs/dunst.pod +++ b/docs/dunst.pod @@ -324,6 +324,8 @@ equivalent notification attribute. =item B<%n> progress value without any extra characters +=item B<%%> Literal % + =back If any of these exists in the format but hasn't been specified in the diff --git a/dunstrc b/dunstrc index 3b2857f..be0dd0a 100644 --- a/dunstrc +++ b/dunstrc @@ -123,6 +123,7 @@ # %I iconname (without its path) # %p progress value if set ([ 0%] to [100%]) or nothing # %n progress value if set without any extra characters + # %% Literal % # Markup is allowed format = "%s\n%b" diff --git a/src/notification.c b/src/notification.c index f6ac0f6..e480180 100644 --- a/src/notification.c +++ b/src/notification.c @@ -387,6 +387,13 @@ int notification_init(notification *n, int id) n->progress ? pg : "", MARKUP_NO); break; + case '%': + n->msg = notification_replace_single_field( + n->msg, + &substr, + "%", + MARKUP_NO); + break; case '\0': fprintf(stderr, "WARNING: format_string has trailing %% character." "To escape it use %%%%."); From e16117ca30a15f19085349703f03d12b59875642 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Wed, 6 Sep 2017 21:41:26 +0200 Subject: [PATCH 3/3] Use double pointers for haystack --- src/notification.c | 96 ++++++++++++++++++++++----------------------- src/notification.h | 2 +- test/notification.c | 9 +++-- 3 files changed, 53 insertions(+), 54 deletions(-) diff --git a/src/notification.c b/src/notification.c index e480180..68bf2eb 100644 --- a/src/notification.c +++ b/src/notification.c @@ -207,28 +207,24 @@ void notification_free(notification *n) * to point to the first char, which occurs after replacement. * */ -char *notification_replace_single_field(char *haystack, char **needle, +void notification_replace_single_field(char **haystack, char **needle, const char *replacement, enum markup_mode markup_mode) { assert(*needle[0] == '%'); // needle has to point into haystack (but not on the last char) - assert(*needle >= haystack); - assert(*needle - haystack < strlen(haystack) - 1); + assert(*needle >= *haystack); + assert(*needle - *haystack < strlen(*haystack) - 1); - char *ret; - - int pos = *needle - haystack; + int pos = *needle - *haystack; char *input = markup_transform(g_strdup(replacement), markup_mode); - ret = string_replace_at(haystack, pos, 2, input); + *haystack = string_replace_at(*haystack, pos, 2, input); // point the needle to the next char // which was originally in haystack - *needle = ret + (*needle - haystack) + strlen(input); + *needle = *haystack + pos + strlen(input); g_free(input); - - return ret; } char *notification_extract_markup_urls(char **str_ptr) { @@ -333,66 +329,66 @@ int notification_init(notification *n, int id) switch(substr[1]){ case 'a': - n->msg = notification_replace_single_field( - n->msg, - &substr, - n->appname, - MARKUP_NO); + notification_replace_single_field( + &n->msg, + &substr, + n->appname, + MARKUP_NO); break; case 's': - n->msg = notification_replace_single_field( - n->msg, - &substr, - n->summary, - n->markup); + notification_replace_single_field( + &n->msg, + &substr, + n->summary, + n->markup); break; case 'b': - n->msg = notification_replace_single_field( - n->msg, - &substr, - n->body, - n->markup); + notification_replace_single_field( + &n->msg, + &substr, + n->body, + n->markup); break; case 'I': - n->msg = notification_replace_single_field( - n->msg, - &substr, - n->icon ? basename(n->icon) : "", - MARKUP_NO); + notification_replace_single_field( + &n->msg, + &substr, + n->icon ? basename(n->icon) : "", + MARKUP_NO); break; case 'i': - n->msg = notification_replace_single_field( - n->msg, - &substr, - n->icon ? n->icon : "", - MARKUP_NO); + notification_replace_single_field( + &n->msg, + &substr, + n->icon ? n->icon : "", + MARKUP_NO); break; case 'p': if (n->progress) sprintf(pg, "[%3d%%]", n->progress - 1); - n->msg = notification_replace_single_field( - n->msg, - &substr, - n->progress ? pg : "", - MARKUP_NO); + notification_replace_single_field( + &n->msg, + &substr, + n->progress ? pg : "", + MARKUP_NO); break; case 'n': if (n->progress) sprintf(pg, "%d", n->progress - 1); - n->msg = notification_replace_single_field( - n->msg, - &substr, - n->progress ? pg : "", - MARKUP_NO); + notification_replace_single_field( + &n->msg, + &substr, + n->progress ? pg : "", + MARKUP_NO); break; case '%': - n->msg = notification_replace_single_field( - n->msg, - &substr, - "%", - MARKUP_NO); + notification_replace_single_field( + &n->msg, + &substr, + "%", + MARKUP_NO); break; case '\0': fprintf(stderr, "WARNING: format_string has trailing %% character." diff --git a/src/notification.h b/src/notification.h index dadfc3b..c0efef4 100644 --- a/src/notification.h +++ b/src/notification.h @@ -73,7 +73,7 @@ int notification_is_duplicate(const notification *a, const notification *b); void notification_run_script(notification *n); int notification_close(notification *n, int reason); void notification_print(notification *n); -char *notification_replace_single_field(char *haystack, char **needle, const char *replacement, enum markup_mode markup_mode); +void notification_replace_single_field(char **haystack, char **needle, const char *replacement, enum markup_mode markup_mode); void notification_update_text_to_render(notification *n); int notification_get_ttl(notification *n); int notification_get_age(notification *n); diff --git a/test/notification.c b/test/notification.c index d077959..e5812dd 100644 --- a/test/notification.c +++ b/test/notification.c @@ -77,17 +77,20 @@ TEST test_notification_replace_single_field(void) strcpy(str, "Markup %a preserved"); substr = strchr(str, '%'); - ASSERT_STR_EQ("Markup and & is preserved", (str = notification_replace_single_field(str, &substr, "and & is", MARKUP_FULL))); + notification_replace_single_field(&str, &substr, "and & is", MARKUP_FULL); + ASSERT_STR_EQ("Markup and & is preserved", str); ASSERT_EQ(26, substr - str); strcpy(str, "Markup %a escaped"); substr = strchr(str, '%'); - ASSERT_STR_EQ("Markup and & <i>is</i> escaped", (str = notification_replace_single_field(str, &substr, "and & is", MARKUP_NO))); + notification_replace_single_field(&str, &substr, "and & is", MARKUP_NO); + ASSERT_STR_EQ("Markup and & <i>is</i> escaped", str); ASSERT_EQ(38, substr - str); strcpy(str, "Markup %a"); substr = strchr(str, '%'); - ASSERT_STR_EQ("Markup is removed and & escaped", (str = notification_replace_single_field(str, &substr, "is removed and & escaped", MARKUP_STRIP))); + notification_replace_single_field(&str, &substr, "is removed and & escaped", MARKUP_STRIP); + ASSERT_STR_EQ("Markup is removed and & escaped", str); ASSERT_EQ(35, substr - str); g_free(str);