From ab9bf55892c8c05302ef7d8b012924609050c67f Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 28 Aug 2017 11:23:21 +0200 Subject: [PATCH] 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: */