[BUG] Starting programs with arguments from run history fails due to incorrect quoting

Ferdi265 commented 2 months ago

Rofi version (rofi -v or git commit in case of build issue)

1.7.5+wayland3-2

Configuration

(shown for completeness, config is /dev/null) https://gist.github.com/Ferdi265/632b99b10f16440b3cdf43f8282b58cb

Theme

(shown for completeness, config is /dev/null) https://gist.github.com/Ferdi265/632b99b10f16440b3cdf43f8282b58cb

Timing report

No response

Launch command

rofi -config /dev/null -show run

Step to reproduce

run powerprofilesctl set performance (note that the 'set', 'performance' arguments are extra arguments not part of the suggested commands)
observe that it works
run powerprofilesctl set performance again (note that it now finds this command in the history)
observe that it doesn't work due to incorrect quoting

Expected behavior

Running a command from history should behave the exact same was as running a command the first time.

Actual behavior

Rofi applies quoting to the command from history, resulting in rofi trying to find a command named 'powerprofilesctl set performance' (including quotes), which fails.

Additional information

The following patch fixes the immediate problem by avoiding shell quoting of the command arguments:

--- a/source/modes/run.c    2024-03-05 17:15:44.000000000 +0100
+++ b/source/modes/run.c    2024-03-10 16:15:04.117103489 +0100
@@ -449,11 +449,11 @@
                                    &path);
       if (retv == MODE_EXIT) {
         if (path == NULL) {
-          char *arg = g_shell_quote(rmpd->cmd_list[rmpd->selected_line].entry);
+          char *arg = rmpd->cmd_list[rmpd->selected_line].entry;
           exec_cmd(arg, run_in_term, rmpd->cmd_list[rmpd->selected_line].entry);
           g_free(arg);
         } else {
-          char *earg = g_shell_quote(rmpd->cmd_list[rmpd->selected_line].entry);
+          char *earg = rmpd->cmd_list[rmpd->selected_line].entry;
           char *epath = g_shell_quote(path);
           char *arg = g_strdup_printf("%s %s", earg, epath);
           exec_cmd(arg, run_in_term, arg);
@@ -468,7 +468,7 @@
   }

   if ((mretv & MENU_OK) && rmpd->cmd_list[selected_line].entry != NULL) {
-    char *earg = g_shell_quote(rmpd->cmd_list[selected_line].entry);
+    char *earg = rmpd->cmd_list[selected_line].entry;
     if (!exec_cmd(earg, run_in_term, rmpd->cmd_list[selected_line].entry)) {
       retv = RELOAD_DIALOG;
     }

This patch works, and I've been using it on my system for weeks now, but I haven't worked on it a lot and there might be a better solution.

I'd be happy to work out a solution to this in more detail if required and can open a PR for this.

Note that this issue is specific to this fork, appeared rather recently, and doesn't occur with upstream rofi releases.

Using wayland display server protocol

[X] Yes, I use rofi with wayland support

I've checked if the issue exists in the latest stable release

[X] Yes, I have checked the problem exists in the latest stable version

alebastr commented 2 months ago

Should be fixed by https://github.com/davatorium/rofi/commit/04f16052a98414b20bf83677cfef3b306d6659f6 and https://github.com/davatorium/rofi/commit/1063b6ec05238341180de4aa704f9e59ab6c9da0 so another merge from upstream might be necessary.

Ferdi265 commented 2 months ago

I can confirm that these upstream commits fix the issue when applied on top of the latest release.

lbonn commented 2 months ago

Ok I've updated the wayland branch, it should be fixed now.

lbonn / rofi