search

enkore / j4-dmenu-desktop

A fast desktop menu
GNU General Public License v3.0
682 stars 69 forks source link

i3 ipc: path should be escaped properly #174

Open baltitenger opened 3 months ago

baltitenger commented 3 months ago

The command fails to launch if the Path property of the desktop file contains shell special characters.

Example: common with wine applications as path is something like .../dosdevices/c:/Program Files (x86)/... The resulting error is sh: -c: line 1: syntax error near unexpected token `('

baltitenger commented 3 months ago

nevermind, quoting is completely broken with i3ipc at least

meator commented 3 months ago

Thanks for the report!

I believe that it should be functional on the develop branch. Would you mind testing out the develop branch or the r3.1 pre-release https://github.com/enkore/j4-dmenu-desktop/releases/tag/r3.1-rc2?

I was actually planning to make a release today, which should definitely fix this problem.

baltitenger commented 3 months ago

Hmm, I was building from develop. However, based on what this is printing, the whole escaping seems to be off, every backslash is doubled:

sh: -c: line 1: syntax error near unexpected token `('
sh: -c: line 1: `'/bin/sh' '-c' '--' 'cd '\\''/home/baltazar/.wine/dosdevices/c:/Program Files (x86)/Line6/POD HD Pro X Edit'\\'' && exec '\\''env'\\'' '\\''WINEPREFIX=/home/baltazar/.wine'\\'' '\\''wine'\\'' '\\''C:\\\\ProgramData\\\\Microsoft\\\\Windows\\\\Start\\'\\'' '\\''Menu\\\\Programs\\\\Line\\'\\'' '\\''6\\\\POD\\'\\'' '\\''HD\\'\\'' '\\''Pro\\'\\'' '\\''X\\'\\'' '\\''Edit\\\\POD\\'\\'' '\\''HD\\'\\'' '\\''Pro\\'\\'' '\\''X\\'\\'' '\\''Edit.lnk'\\''

Note that I'm on sway, though I assumed it was supposed to be fully compatible in this regard.

baltitenger commented 3 months ago

Also, placeholders like %U are always left in the command line, even though no urls are supposed to be passed to the application.

meator commented 3 months ago

Also, placeholders like %U are always left in the command line, even though no urls are supposed to be passed to the application.

Thanks for reporting! I have fixed this in 2d9743513577b847e66eaaed2fc56ee7e7d6320e. I3 mode did not expand field codes at all.

About your first issue, I am not able to reproduce your problem on i3. I will do more testing on Sway.

meator commented 3 months ago

This is kinda hard to diagnose. I am also convinced that this is a bug in Sway, because its IPC should be fully compatible with i3's one.

baltitenger commented 3 months ago

Yeah I thought so. I guess this should be reported to sway then?

meator commented 3 months ago

d7e594a3a9ec07bfad62f0ad71fedd736a537962 should hopefully fix the issue. Could you please test it @baltitenger?

I've had a pretty interesting discussion at #sway on Libera chat. I still believe that this is a bug that could be reported. The problem is much more complicated that I initially thought, I've had to spin up a VM with i3 and Sway installed to diagnose it. I still do not fully trust j4dd's i3/Sway integration, but I believe that I've improved it now.

baltitenger commented 3 months ago

Now the path works fine but fails on the actual command. Wine commandlines are a real pain to escape :P Here's what it's doing now:

sway[41053]: 01:01:38.273 [INFO] [sway/commands.c:260] Handling command 'exec cd '/home/baltazar/.wine/dosdevices/c:/Program Files (x86)/Line6/POD HD Pro X Edit' && 'env' 'WINEPREFIX=/home/baltazar/.wine' 'wine' 'C:\\ProgramData\\Microsoft\\Windows\\Start\' 'Menu\\Programs\\Line\' '6\\POD\' 'HD\' 'Pro\' 'X\' 'Edit\\POD\' 'HD\' 'Pro\' 'X\' 'Edit.lnk''
wine[52146]: wine: failed to open "C:\\\\ProgramData\\\\Microsoft\\\\Windows\\\\Start\\": c0000135

Now it's spaces not being escaped for the shell apparently.

meator commented 3 months ago

Could you post here the entire desktop file?

baltitenger commented 3 months ago

here you go:

[Desktop Entry]
Name=POD HD Pro X Edit
Exec=env WINEPREFIX="/home/baltazar/.wine" wine C:\\\\ProgramData\\\\Microsoft\\\\Windows\\\\Start\\ Menu\\\\Programs\\\\Line\\ 6\\\\POD\\ HD\\ Pro\\ X\\ Edit\\\\POD\\ HD\\ Pro\\ X\\ Edit.lnk
Type=Application
StartupNotify=true
Path=/home/baltazar/.wine/dosdevices/c:/Program Files (x86)/Line6/POD HD Pro X Edit
Icon=452C_POD HD Pro X Edit.0
StartupWMClass=pod hd pro x edit.exe
meator commented 3 months ago

I think that your desktop file is faulty. The standard mandates that reserved characters such as space must be quoted with double quotes.

I think that a standard compliant Exec could look like this:

Exec=env WINEPREFIX=/home/baltazar/.wine wine "C:\\\\ProgramData\\\\Microsoft\\\\Windows\\\\Start Menu\\\\Programs\\\\Line 6\\\\POD HD Pro X Edit\\\\POD HD Pro X Edit.lnk"

This desktop file might have worked with older versions. I have since then reworked command line assembly of j4-dmenu-desktop. There is some ambiguity in the standard on whether a shell or direct execution should be used. This is confusing, I have even messaged the XDG mailing list and we agreed that the standard should be updated to clarify this.

The new implementation is more strict and compliant than the old one, so desktop files which weren't compliant might stop working after an update to r3.1. This wouldn't need to happen if the standard was unambiguous.

baltitenger commented 3 months ago

Oh well. This was generated by wine so I guess they should be told as well...

meator commented 3 months ago

Is everything else working as intended? If you have time to spare, could you please try out a desktop file which has Terminal=true in it (like htop)? Handling of terminal desktop files is more complex.

baltitenger commented 3 months ago

Everything else seems fine, tested terminal too.

meator commented 3 months ago

Ok, thanks!

I’d still like to release r3.1 today. While this issue might require more time, I hope I've fixed it by now. There are also a lot of bug fixes that have accumulated in the development branch, which I'd like to release.

meator commented 1 month ago

I have verified that desktop files generated by Wine do not work in j4-dmenu-desktop. This is unfortunate. Desktop files generated by Wine are not conforming to the Desktop entry specification, so this isn't exactly j4-dmenu-desktop's fault, but this won't really help users who are struck by this change.

I have created an issue about it: #181