You are not logged in.

#1 2016-06-01 17:44:08

nokangaroo
Member
Registered: 2013-09-08
Posts: 25

Patches for mate- and xfce4-terminal for backslash-escaping uris

Proposed patches for mate-terminal and xfce4-terminal for backslash-escaping uris

The use of g_shell_parse_argv in line 548 of xfce4-terminal-0.6.3/terminal/terminal-widget.c seems unsafe to me; it fails to detect filenames like "test&file". On the other hand mate-terminal is way too timid , quoting everything all the time (and annoying; some programs like the VLC lua commandline can't handle quotes). Quoting is for wimps :-)

I posted an issue on github regarding mate-terminal, but so far there has been no response. I post the patch here (and the corresponding patch for xfce4-terminal) for feedback. The patched files compile without errors (there are compiler warnings for mate-terminal which seem to be unrelated).

The patches replace the wholesale quoting with selective backslash escaping of blacklisted characters (like OS X Terminal.app). The first half of the blacklist is from man bash; the rest is from my own tests with downloaded html pages (whose file names contain unicode bullet points, pipe symbols, quotes and other nastiness. A juicy example:

"Understanding Bash fork() bomb ~ :(){ :|:& };:.html"

Don't try that one with the unpatched xfce4-terminal). I'd prefer a glib function to the handmade solution, but there doesn't seem to be any. Use patch -p0 in PKGBUILD (I assume I am posting to experienced users, so no disclaimer).

It seems that not all shell special characters need to be escaped (this is about paths dragged to the terminal window, not shell commands). If someone finds a character missing from the blacklist please post.



Here is the patch for mate-terminal (inspired by xfce4-terminal; didn't work with GFile. The space after the drag is handled by the following function in the same file). I noticed that I could not feed the escaped path directly to

uris[i]

It seems that unicode characters are split in two in the for loop and need to be reassembled by g_filename_display_name. (As the programs use char * to begin with I feel justified in doing all this).

--- src/terminal-util.c    2015-10-06 17:58:46.000000000 +0200
+++ src/terminal-util.c    2016-05-29 17:25:16.082584990 +0200
@@ -287,30 +287,42 @@
 void
 terminal_util_transform_uris_to_quoted_fuse_paths (char **uris)
 {
-    guint i;
-
     if (!uris)
         return;
 
+    size_t i, j;
+    const char blacklist[] = "|&;()<>!\'\"`* ";
+    char *target;
+    char *filename;
+    char *ptr;
+    size_t n = 0;
     for (i = 0; uris[i]; ++i)
     {
-        GFile *file;
-        char *path;
-
-        file = g_file_new_for_uri (uris[i]);
-
-        if ((path = g_file_get_path (file)))
+        filename = g_filename_from_uri (uris[i], NULL, NULL);
+        if (G_LIKELY (filename != NULL))
         {
-            char *quoted;
-
-            quoted = g_shell_quote (path);
+            n = strlen (filename);
+            /* worst-case scenario (every char gets quoted) + '\0' */
+            target = g_malloc (n * 2 + 1);
+            ptr = target;
+            for(j = 0; j < n; j++)
+            {
+                if (strchr(blacklist, (char) filename[j]) == NULL)
+                {
+                    *(ptr++) = filename[j];
+                }
+                else
+                {
+                    *(ptr++) = '\\';
+                    *(ptr++) = filename[j];
+                }
+            }
+            *ptr = '\0';
             g_free (uris[i]);
-            g_free (path);
-
-            uris[i] = quoted;
+            uris[i] = g_filename_display_name (target);
+            g_free (target);
         }
-
-        g_object_unref (file);
+        g_free (filename);
     }
 }
 


The patch for xfce4-terminal (which also adds a space after the dragged path):

--- terminal/terminal-widget.c    2013-12-26 22:31:10.000000000 +0100
+++ terminal/terminal-widget.c    2016-05-29 17:21:38.679026088 +0200
@@ -460,7 +460,6 @@
   gchar        **uris;
   gchar         *filename;
   gchar         *text;
-  gint           argc;
   gint           n;
   GtkWidget     *screen;
 
@@ -533,7 +532,10 @@
           text = g_strndup ((const gchar *) selection_data->data, selection_data->length);
           uris = g_uri_list_extract_uris (text);
           g_free (text);
-
+          const char blacklist[] = "|&;()<>!\'\"`* ";
+          char *target;
+          char *ptr;
+          size_t i, j;
           /* translate all file:-URIs to quoted file names */
           for (n = 0; uris[n] != NULL; ++n)
             {
@@ -541,28 +543,30 @@
               filename = g_filename_from_uri (uris[n], NULL, NULL);
               if (G_LIKELY (filename != NULL))
                 {
-                  /* release the file:-URI */
-                  g_free (uris[n]);
-
-                  /* check if we need to quote the file name (for the shell) */
-                  if (!g_shell_parse_argv (filename, &argc, NULL, NULL) || argc != 1)
-                    {
-                      /* we need to quote the filename */
-                      uris[n] = g_shell_quote (filename);
-                      g_free (filename);
-                    }
-                  else
+                  i = strlen (filename);
+                  /* worst-case scenario + space + '\0' */
+                  target = g_malloc (i * 2 + 2);
+                  ptr = target;
+                  for(j = 0; j < i; j++)
                     {
-                      /* no need to quote, shell will handle properly */
-                      uris[n] = filename;
+                      if (strchr(blacklist, (char) filename[j]) == NULL)
+                        {
+                          *(ptr++) = filename[j];
+                        }
+                      else
+                        {
+                          *(ptr++) = '\\';
+                          *(ptr++) = filename[j];
+                        }
                     }
+                  *(ptr++) = ' ';
+                  *ptr = '\0';
+                  vte_terminal_feed_child (VTE_TERMINAL (widget), target, strlen (target));
+                  g_free (filename);
+                  g_free (target);
                 }
             }
-
-          text = g_strjoinv (" ", uris);
-          vte_terminal_feed_child (VTE_TERMINAL (widget), text, strlen (text));
           g_strfreev (uris);
-          g_free (text);
         }
       break;
 

Edit:
Booting into my backup of OS X Snow Leopard and doing

strings /Applications/Utilities/Terminal.app/Contents/MacOS/Terminal

I found this:

\!|&;<>()$'` 
*?[]#~=%"

So this would be the complete list that Terminal.app uses; but some of these are harmless (at least in Linux). The backslash could be added to the blacklist, but I never found it in actual filenames.

Last edited by nokangaroo (2016-06-05 20:44:51)

Offline

Board footer

Powered by FluxBB