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

@@ -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

src/notification.c

@@ -200,17 +200,33 @@ void notification_free(notification *n)
 }
 
 /*
- * Replace all occurrences of "needle" with a quoted "replacement",
+ * Replace the two chars where **needle points
- * according to the markup settings.
+ * 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 *notification_replace_single_field(char *haystack, char **needle,
-                                  char *haystack, enum markup_mode markup_mode) {
+                const char *replacement, enum markup_mode markup_mode) {
-        char *tmp;
+
+        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);
+        int pos = *needle - haystack;
-        ret = string_replace_all(needle, tmp, haystack);
+
-        g_free(tmp);
+        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,
+
+        /* replace all formatter */
+        for(char *substr = strchr(n->msg, '%');
+                  substr;
+                  substr = strchr(substr, '%')){
+
+                char pg[16];
+
+                switch(substr[1]){
+                        case 'a':
+                                n->msg = notification_replace_single_field(
+                                                n->msg,
+                                                &substr,
+                                                n->appname,
                                                 MARKUP_NO);
-        n->msg = notification_replace_format("%s", n->summary, n->msg,
+                                break;
+                        case 's':
+                                n->msg = notification_replace_single_field(
+                                                n->msg,
+                                                &substr,
+                                                n->summary,
                                                 n->markup);
-        n->msg = notification_replace_format("%b", n->body, n->msg,
+                                break;
+                        case 'b':
+                                n->msg = notification_replace_single_field(
+                                                n->msg,
+                                                &substr,
+                                                n->body,
                                                 n->markup);
-
+                                break;
-        if (n->icon) {
+                        case 'I':
-                char *tmp = g_strdup(n->icon);
+                                n->msg = notification_replace_single_field(
-                n->msg = notification_replace_format("%I", basename(tmp),
+                                                n->msg,
-                        n->msg, MARKUP_NO);
+                                                &substr,
-                n->msg = notification_replace_format("%i", n->icon,
+                                                n->icon ? basename(n->icon) : "",
-                        n->msg, MARKUP_NO);
+                                                MARKUP_NO);
-                g_free(tmp);
+                                break;
-        }
+                        case 'i':
-
+                                n->msg = notification_replace_single_field(
-        if (n->progress) {
+                                                n->msg,
-                char pg[10];
+                                                &substr,
+                                                n->icon ? n->icon : "",
+                                                MARKUP_NO);
+                                break;
+                        case 'p':
+                                if (n->progress)
                                         sprintf(pg, "[%3d%%]", n->progress - 1);
-                n->msg = string_replace_all("%p", pg, n->msg);
+
+                                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 = string_replace_all("%n", pg, n->msg);
+
-        } else {
+                                n->msg = notification_replace_single_field(
-                n->msg = string_replace_all("%p", "", n->msg);
+                                                n->msg,
-                n->msg = string_replace_all("%n", "", 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);

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);

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 <len> characters with <repl> at position <pos> of the string <buf> */
+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);

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));
-
+        char *substr = NULL;
-        strcpy(str, "Testing format replacement");
-        ASSERT_STR_EQ("Testing text replacement", (str = notification_replace_format("format", "text", str, MARKUP_FULL)));
 
         strcpy(str, "Markup %a preserved");
-        ASSERT_STR_EQ("Markup and &amp; <i>is</i> preserved", (str = notification_replace_format("%a", "and &amp; <i>is</i>", str, MARKUP_FULL)));
+        substr = strchr(str, '%');
+        ASSERT_STR_EQ("Markup and &amp; <i>is</i> preserved", (str = notification_replace_single_field(str, &substr, "and &amp; <i>is</i>", MARKUP_FULL)));
+        ASSERT_EQ(26, substr - str);
 
         strcpy(str, "Markup %a escaped");
-        ASSERT_STR_EQ("Markup and &amp; &lt;i&gt;is&lt;/i&gt; escaped", (str = notification_replace_format("%a", "and & <i>is</i>", str, MARKUP_NO)));
+        substr = strchr(str, '%');
+        ASSERT_STR_EQ("Markup and &amp; &lt;i&gt;is&lt;/i&gt; escaped", (str = notification_replace_single_field(str, &substr, "and & <i>is</i>", MARKUP_NO)));
+        ASSERT_EQ(38, substr - str);
 
         strcpy(str, "Markup %a");
-        ASSERT_STR_EQ("Markup is removed and &amp; escaped", (str = notification_replace_format("%a", "<i>is removed</i> and & escaped", str, MARKUP_STRIP)));
+        substr = strchr(str, '%');
+        ASSERT_STR_EQ("Markup is removed and &amp; escaped", (str = notification_replace_single_field(str, &substr, "<i>is removed</i> 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: */