Fix i3-dmenu-desktop quoting (#5162)

Commit 70f23caa9a18afc146f696fdf7d2481e5f7f0101 introduced new issues.

Instead of distinguishing " and \, as that commit attempted,
let’s instead keep the level of escaping by escaping each backslash,
just like each double quote.

I tested this with:

    # recommended way to quote $ and " in quoted arguments, not ambiguous
    Exec=/tmp/logargs "hello \\$PWD \\"and\\" more"

    # permitted way to quote $ and " in quoted arguments, but ambiguous
    Exec=/tmp/logargs "hello \$PWD \"and\" more"

    # permitted way to quote arguments, slightly unusual to quote first arg
    Exec="/tmp/logargs" hey

    # a complicated shell expression, not ambiguous
    Exec=sh -c "if [ -n \\"\\$*\\" ]; then exec /tmp/logargs --alternate-editor= --display=\\"\\$DISPLAY\\" \\"\\$@\\"; else exec /tmp/logargs --alternate-editor= --create-frame; fi" placeholder %F

related to https://github.com/i3/i3/issues/4697 (electrum, original)
related to https://github.com/i3/i3/issues/5152 (phpstorm, breakage)
related to https://github.com/i3/i3/issues/5156 (emacsclient, breakage)

2 files changed, 141 insertions, 4 deletions

diff --git a/i3-dmenu-desktop b/i3-dmenu-desktop
index e43d95aa..bb693350 100755
--- a/i3-dmenu-desktop
+++ b/i3-dmenu-desktop
@@ -413,7 +413,7 @@ my $exec = $app->{Exec};
 my $location = $app->{_Location};
 
 # Quote as described by “The Exec key”:
-# https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s06.html
+# https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s07.html
 sub quote {
     my ($str) = @_;
     $str =~ s/("|`|\$|\\)/\\$1/g;
@@ -425,6 +425,17 @@ $choice = quote($choice);
 $location = quote($location);
 $name = quote($name);
 
+# https://standards.freedesktop.org/desktop-entry-spec/latest/ar01s07.html:
+#
+# Note that the general escape rule for values of type string states that the
+# backslash character can be escaped as ("\\") as well and that this escape rule
+# is applied before the quoting rule. As such, to unambiguously represent a
+# literal backslash character in a quoted argument in a desktop entry file
+# requires the use of four successive backslash characters ("\\\\"). Likewise, a
+# literal dollar sign in a quoted argument in a desktop entry file is
+# unambiguously represented with ("\\$").
+$exec =~ s/\\\\/\\/g;
+
 # Remove deprecated field codes, as the spec dictates.
 $exec =~ s/%[dDnNvm]//g;
 
@@ -481,9 +492,10 @@ EOT
     # starts with a double quote ("), everything is parsed as-is until the next
     # double quote which is NOT preceded by a backslash (\).
     #
-    # Therefore, we escape all double quotes (") by replacing them with \"
-    $exec =~ s/\\"/\\\\\\"/g;
-    $exec =~ s/([^\\])"/$1\\"/g;
+    # Therefore, we escape all double quotes (") by replacing them with \".
+    # To not change the meaning of any double quote, backslashes need to be
+    # escaped as well.
+    $exec =~ s/(["\\])/\\$1/g;
 
     if (exists($app->{StartupNotify}) && !$app->{StartupNotify}) {
         $nosn = '--no-startup-id';
diff --git a/testcases/t/318-i3-dmenu-desktop.t b/testcases/t/318-i3-dmenu-desktop.t
new file mode 100644
index 00000000..75d983ef
--- /dev/null
+++ b/testcases/t/318-i3-dmenu-desktop.t
@@ -0,0 +1,125 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • https://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • https://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • https://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Verifies that i3-dmenu-desktop correctly parses Exec= lines in .desktop files
+# and sends the command to i3 for execution.
+# Ticket: #5152, #5156
+# Bug still in: 4.21-17-g389d555d
+use i3test;
+use i3test::Util qw(slurp);
+use File::Temp qw(tempfile tempdir);
+use POSIX qw(mkfifo);
+use JSON::XS qw(decode_json);
+
+my $desktopdir = tempdir(CLEANUP => 1);
+
+$ENV{XDG_DATA_DIRS} = "$desktopdir";
+
+mkdir("$desktopdir/applications");
+
+# Create an i3-msg executable that dumps command line flags to a FIFO
+my $tmpdir = tempdir(CLEANUP => 1);
+
+$ENV{PATH} = "$tmpdir:" . $ENV{PATH};
+
+mkfifo("$tmpdir/fifo", 0600) or BAIL_OUT "Could not create FIFO: $!";
+
+open(my $i3msg_dump, '>', "$tmpdir/i3-msg");
+say $i3msg_dump <<EOT;
+#!/usr/bin/env perl
+use strict;
+use warnings;
+use JSON::XS qw(encode_json);
+open(my \$f, '>', "$tmpdir/fifo");
+say \$f encode_json(\\\@ARGV);
+close(\$f);
+EOT
+close($i3msg_dump);
+chmod 0755, "$tmpdir/i3-msg";
+
+my $testcnt = 0;
+sub verify_exec {
+    my ($execline, $want_arg) = @_;
+
+    $testcnt++;
+
+    open(my $desktop, '>', "$desktopdir/applications/desktop$testcnt.desktop");
+    say $desktop <<EOT;
+[Desktop Entry]
+Name=i3-testsuite-$testcnt
+Type=Application
+Exec=$execline
+EOT
+    close($desktop);
+
+    # complete-run.pl arranges for $PATH to be set up such that the
+    # i3-dmenu-desktop version we execute is the one from the build directory.
+    my $exit = system("i3-dmenu-desktop --dmenu 'echo i3-testsuite-$testcnt' &");
+    if ($exit != 0) {
+	die "failed to run i3-dmenu-desktop";
+    }
+
+    chomp($want_arg);  # trim trailing newline
+    my $got_args = decode_json(slurp("$tmpdir/fifo"));
+    is_deeply($got_args, [ $want_arg ], 'i3-dmenu-desktop executed command as expected');
+}
+
+# recommended number of backslashes by the spec, not ambiguous
+my $exec_1 = <<'EOS';
+echo "hello \\$PWD \\"and\\" more"
+EOS
+my $want_1 = <<'EOS';
+exec  "echo \"hello \\$PWD \\\"and\\\" more\""
+EOS
+verify_exec($exec_1, $want_1);
+
+# permitted, but ambiguous
+my $exec_2 = <<'EOS';
+echo "hello \$PWD \"and\" more"
+EOS
+my $want_2 = <<'EOS';
+exec  "echo \"hello \\$PWD \\\"and\\\" more\""
+EOS
+verify_exec($exec_2, $want_2);
+
+# electrum
+my $exec_3 = <<'EOS';
+sh -c "PATH=\"\\$HOME/.local/bin:\\$PATH\"; electrum %u"
+EOS
+my $want_3 = <<'EOS';
+exec  "sh -c \"PATH=\\\"\\$HOME/.local/bin:\\$PATH\\\"; electrum \""
+EOS
+verify_exec($exec_3, $want_3);
+
+# complicated emacsclient command
+my $exec_4 = <<'EOS';
+sh -c "if [ -n \\"\\$*\\" ]; then exec emacsclient --alternate-editor= --display=\\"\\$DISPLAY\\" \\"\\$@\\"; else exec emacsclient --alternate-editor= --create-frame; fi" placeholder %F
+EOS
+my $want_4 = <<'EOS';
+exec  "sh -c \"if [ -n \\\"\\$*\\\" ]; then exec emacsclient --alternate-editor= --display=\\\"\\$DISPLAY\\\" \\\"\\$@\\\"; else exec emacsclient --alternate-editor= --create-frame; fi\" placeholder "
+EOS
+verify_exec($exec_4, $want_4);
+
+# permitted, but unusual to quote the first arg
+my $exec_5 = <<'EOS';
+"electrum" arg
+EOS
+my $want_5 = <<'EOS';
+exec  "\"electrum\" arg"
+EOS
+verify_exec($exec_5, $want_5);
+
+done_testing;