fixed pl_format_title code duplication in shellexec plugin

5 files changed, 73 insertions, 114 deletions

diff --git a/deadbeef.h b/deadbeef.h
index e2eb8846..59d62488 100644
--- a/deadbeef.h
+++ b/deadbeef.h
@@ -405,6 +405,8 @@ typedef struct {
        more to come
     */
     int (*pl_format_title) (DB_playItem_t *it, int idx, char *s, int size, int id, const char *fmt);
+    // _escaped version wraps all conversions with '' and replaces every ' in conversions with \'
+    int (*pl_format_title_escaped) (DB_playItem_t *it, int idx, char *s, int size, int id, const char *fmt);
     void (*pl_format_time) (float t, char *dur, int size);
     void (*pl_format_item_display_name) (DB_playItem_t *it, char *str, int len);
 //    void (*pl_set_next) (DB_playItem_t *it, DB_playItem_t *next, int iter);
diff --git a/playlist.c b/playlist.c
index 40d1a671..802605a3 100644
--- a/playlist.c
+++ b/playlist.c
@@ -2436,13 +2436,14 @@ pl_format_elapsed (const char *ret, char *elapsed, int size) {
     return elapsed;
 }
 
-int
-pl_format_title (playItem_t *it, int idx, char *s, int size, int id, const char *fmt) {
+// this function allows to escape special chars substituted for conversions
+// @escape_chars: list of escapable characters terminated with 0, or NULL if none
+static int
+pl_format_title_int (const char *escape_chars, playItem_t *it, int idx, char *s, int size, int id, const char *fmt) {
     char dur[50];
     char elp[50];
     char fno[50];
     char tags[200];
-    char artistalbum[1024];
     const char *duration = NULL;
     const char *elapsed = NULL;
 
@@ -2480,7 +2481,7 @@ pl_format_title (playItem_t *it, int idx, char *s, int size, int id, const char
         return 0;
     }
     int n = size-1;
-    while (*fmt && n) {
+    while (*fmt && n > 0) {
         if (*fmt != '%') {
             *s++ = *fmt;
             n--;
@@ -2557,6 +2558,9 @@ pl_format_title (playItem_t *it, int idx, char *s, int size, int id, const char
                     meta++;
                 }
             }
+            else if (*fmt == 'F') {
+                meta = it->fname;
+            }
             else if (*fmt == 'T') {
                 char *t = tags;
                 char *e = tags + sizeof (tags);
@@ -2607,16 +2611,53 @@ pl_format_title (playItem_t *it, int idx, char *s, int size, int id, const char
 
             if (meta) {
                 const char *value = meta;
-                while (n > 0 && *value) {
-                    *s++ = *value++;
+                if (escape_chars) {
+                    // need space for at least 2 single-quotes
+                    if (n < 2) {
+                        goto error;
+                    }
+                    *s++ = '\'';
+                    n--;
+                    while (n > 2 && *value) {
+                        const char *e = escape_chars;
+                        for (; *e; e++) {
+                            if (*value == *e) {
+                                if (n < 2) {
+                                    // doesn't fit into output buffer, return
+                                    // empty string and error code
+                                    *ss = 0;
+                                    return -1;
+                                }
+                                *s++ = '\\';
+                                n--;
+                                *s++ = *value++;
+                                n--;
+                            }
+                            else {
+                                *s++ = *value++;
+                            }
+                        }
+                    }
+                    if (n < 1) {
+                        fprintf (stderr, "pl_format_title_int: got unpredicted state while formatting escaped string. please report a bug.\n");
+                        *ss = 0; // should never happen
+                        return -1; 
+                    }
+                    *s++ = '\'';
                     n--;
                 }
+                else {
+                    while (n > 0 && *value) {
+                        *s++ = *value++;
+                        n--;
+                    }
+                }
             }
         }
         fmt++;
     }
+error:
     *s = 0;
-
     UNLOCK;
 
     // replace all \n with ;
@@ -2630,6 +2671,16 @@ pl_format_title (playItem_t *it, int idx, char *s, int size, int id, const char
     return size - n - 1;
 }
 
+int
+pl_format_title (playItem_t *it, int idx, char *s, int size, int id, const char *fmt) {
+    return pl_format_title_int (NULL, it, idx, s, size, id, fmt);
+}
+
+int
+pl_format_title_escaped (playItem_t *it, int idx, char *s, int size, int id, const char *fmt) {
+    return pl_format_title_int ("'", it, idx, s, size, id, fmt);
+}
+
 static int pl_sort_is_duration;
 static int pl_sort_is_track;
 static int pl_sort_ascending;
diff --git a/playlist.h b/playlist.h
index c04fe11e..e0abee64 100644
--- a/playlist.h
+++ b/playlist.h
@@ -255,6 +255,9 @@ pl_set_item_flags (playItem_t *it, uint32_t flags);
 int
 pl_format_title (playItem_t *it, int idx, char *s, int size, int id, const char *fmt);
 
+int
+pl_format_title_escaped (playItem_t *it, int idx, char *s, int size, int id, const char *fmt);
+
 void
 pl_format_time (float t, char *dur, int size);
 
diff --git a/plugins.c b/plugins.c
index 07c38a7e..368768b9 100644
--- a/plugins.c
+++ b/plugins.c
@@ -160,6 +160,7 @@ static DB_functions_t deadbeef_api = {
     .pl_get_next = (DB_playItem_t *(*) (DB_playItem_t *, int))pl_get_next,
     .pl_get_prev = (DB_playItem_t *(*) (DB_playItem_t *, int))pl_get_prev,
     .pl_format_title = (int (*) (DB_playItem_t *it, int idx, char *s, int size, int id, const char *fmt))pl_format_title,
+    .pl_format_title_escaped = (int (*) (DB_playItem_t *it, int idx, char *s, int size, int id, const char *fmt))pl_format_title_escaped,
     .pl_format_time = pl_format_time,
     .pl_move_items = (void (*) (int iter, int plt_from, DB_playItem_t *drop_before, uint32_t *indexes, int count))pl_move_items,
     .pl_copy_items = (void (*) (int iter, int plt_from, DB_playItem_t *before, uint32_t *indices, int cnt))pl_copy_items,
diff --git a/plugins/shellexec/shellexec.c b/plugins/shellexec/shellexec.c
index 7e65c52a..84a77bad 100644
--- a/plugins/shellexec/shellexec.c
+++ b/plugins/shellexec/shellexec.c
@@ -40,6 +40,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <limits.h>
 
 #include "../../deadbeef.h"
 
@@ -83,116 +84,17 @@ trim (char* s)
 }
 
 static int
-shell_quote (const char *source, char *dest)
-{
-    const char *sp;
-    char *dp;
-
-    for (sp = source, dp = dest; *sp; sp++, dp++)
-    {
-        if (*sp == '\'')
-        {
-            strcpy (dp, "'\\''");
-            dp += 3;
-        }
-        else
-            *dp = *sp;
-    }
-    *dp = 0;
-}
-
-
-/*
-    format_shell_command function
-    Similarly to db_format_title formats track's string
-    and quotes all fields to make them safe for shell
-    
-    %a, %t, %b, %B, %C, %n, %N, %y, %g, %c, %r are handled. Use double-% to
-    skip field.
-
-
-    Example:
-
-    Format string:
-    echo %a - %t %%a - %%t
-
-    Output:
-    echo 'Blind Faith' - 'Can'\''t Find My Way Home' %a - %t
-*/
-static char*
-format_shell_command (DB_playItem_t *it, const char *format)
-{
-    char *p;
-    const char *trailing = format;
-    const char *field;
-    char *res, *res_p;
-
-    res = res_p = malloc (65536); //FIXME: possible heap corruption
-
-    for (;;)
-    {
-        p = strchr (trailing, '%');
-        if (!p) break;
-
-        switch (*(p+1))
-        {
-            case 'a': field = "artist"; break;
-            case 't': field = "title"; break;
-            case 'b': field = "album"; break;
-            case 'B': field = "band"; break;
-            case 'C': field = "composer"; break;
-            case 'n': field = "track"; break;
-            case 'N': field = "numtracks"; break;
-            case 'y': field = "year"; break;
-            case 'g': field = "genre"; break;
-            case 'c': field = "comment"; break;
-            case 'r': field = "copyright"; break;
-            case 'f': break;
-            default: field = NULL;
-        }
-
-        if (field == NULL)
-        {
-            int l = ((*(p+1) == '%') ? 1 : 0);
-            trace ("field is null; p: %s; p+1: %s; l: %d; trailing: %s; res_p: %s\n", p, p+1, l, trailing, res_p);
-            strncpy (res_p, trailing, p-trailing+l);
-            res_p += (p-trailing+l);
-            trailing = p+l+1;
-            trace ("res: %s; trailing: %s\n", res, res_p, trailing);
-            continue;
-        }
-        else
-        {
-            const char *meta;
-            if (*(p+1) == 'f')
-                meta = it->fname;
-            else
-                meta = deadbeef->pl_find_meta (it, field);
-
-            char quoted [strlen (meta) * 4 + 1]; //Worst case is when all chars are single quote
-            shell_quote (meta, quoted);
-            
-            strncpy (res_p, trailing, p-trailing);
-            res_p += (p-trailing);
-            *res_p++ = '\'';
-            strcpy (res_p, quoted);
-            res_p += strlen (quoted);
-            *res_p++ = '\'';
-            trailing = p+2;
-        }
-    }
-    strcpy (res_p, trailing);
-    strcat (res_p, "&");
-    return res;
-}
-
-static int
 shx_callback (Shx_action_t *action, DB_playItem_t *it)
 {
-    char *cmd = format_shell_command (it, action->shcommand);
-    printf ("%s\n", cmd);
+    char cmd[_POSIX_ARG_MAX];
+    int res = deadbeef->pl_format_title_escaped (it, -1, cmd, sizeof (cmd) - 2, -1, action->shcommand);
+    if (res < 0) {
+        trace ("shellexec: failed to format string for execution (too long?)\n");
+        return -1;
+    }
+    strcat (cmd, "&");
+    trace ("%s\n", cmd);
     system (cmd);
-    free (cmd);
     return 0;
 }