SUMMARY After recent update[1] of scrcpy, I get these warnings when e.g. launching systemsettings: > kf.config.core: "KConfigIni: In file /usr/share/applications/scrcpy-console.desktop, line 8: " "Invalid escape sequence \"\\\"\"." > kf.config.core: "KConfigIni: In file /usr/share/applications/scrcpy.desktop, line 8: " "Invalid escape sequence \"\\\"\"." Or with a patch[2] that improves readability: > kf.config.core: KConfigIni: In file /usr/share/applications/scrcpy.desktop, line 8: Invalid escape sequence: «\"» scrcpy.desktop has the following Exec line: > Exec=/bin/sh -c "\"\\$SHELL\" -i -c scrcpy" It seems that escape sequences are fine, and it's our parser that is broken. According to the documentation at FDO [3], Exec= line has some special rules about escape sequences. STEPS TO REPRODUCE 1. Parse a file that contains the line above with KConfig, KDesktopFile or KService. OBSERVED RESULT Warnings, poorly escaped resulting string. EXPECTED RESULT Adherence to standards, no warnings. SOFTWARE/OS VERSIONS Operating System: Arch Linux KDE Frameworks Version: 5.240.0 (kf6/master) Qt Version: 6.5.1 ADDITIONAL INFORMATION [1]: https://github.com/Genymobile/scrcpy/commit/d2b7315ba693e11bd9b9a0421a75b7df8a31c5cc [2]: https://invent.kde.org/frameworks/kconfig/-/merge_requests/203 [3]: https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables In kservice.cpp, the `init()` method fetches "Exec" entry using regular `desktopGroup.readEntry("Exec", QString());` call, without any specialized processing. It seems wrong. KDesktopFile has many `read$Thing` methods, but Exec is not one of them. Looking at the added complexity of special escaping rules, it probably should have that as well. And before you ask, no — KIO::DesktopExecParser is NOT a solution. It takes an instance of KService in constructor, which is already messed up by that time.
Git commit 9b8f7d9ac2cbc7c6048c981a040540b7c6e35666 by ivan tkachenko. Committed on 29/06/2023 at 09:25. Pushed by ratijas into branch 'master'. KConfigIniBackend: Make diagnostics about escape sequences more readable Instead of this: kf.config.core: "KConfigIni: In file scrcpy.desktop, line 8: " "Invalid escape sequence \"\\\"\"." We'll be getting this: kf.config.core: KConfigIni: In file scrcpy.desktop, line 8: Invalid escape sequence: «\"» Which is still our bug to fix, but at least now it's clear which one. M +2 -2 src/core/kconfigini.cpp https://invent.kde.org/frameworks/kconfig/-/commit/9b8f7d9ac2cbc7c6048c981a040540b7c6e35666
Removing automated "qt6" keyword. This is not a regression. It might even be worth backporting to KF5 if it won't end up being an ABI break.
desktop-file-validate says this is valid
I don't believe there is anything to really fix here on the parsing side and the warning is correct as is, although I do find the specification a bit hard to interpret or even ambiguous at times concerning the escaping rules. Still, I'm reasonably sure the current implementation matches the actual intent and this seems to be a case where the validation logic isn't fully implemented on desktop-file-utils's side, see their TODO-comment in validate.c (https://gitlab.freedesktop.org/xdg/desktop-file-utils/-/blob/master/src/validate.c?ref_type=heads#L55): > /*TODO: > * + Lecagy-Mixed Encoding (annexe D) > * + The escape sequences \s, \n, \t, \r, and \\ are supported for values of > * type string and localestring, meaning ASCII space, newline, tab, carriage > * return, and backslash, respectively. > */ According to the current version of desktop-file-validate all these are valid (but only the first one really is to my understanding): > Exec=/bin/sh -c "\\"\\$SHELL\\" -i -c scrcpy" > Exec=/bin/sh -c "\"\\$SHELL\" -i -c scrcpy" > Exec=/bin/sh -c "\"\$SHELL\" -i -c scrcpy" but as the error message also indicates when the "$" isn't quoted at all, backslashes for escaping need to be escaped themselves: > Exec=/bin/sh -c "\\"$SHELL\\" -i -c scrcpy" > error: value "/bin/sh -c "\\"$SHELL\\" -i -c 'scrcpy --pause-on-exit=if-error'"" for key "Exec" in group "Desktop Entry" contains a non-escaped character '$' in a quote, but it should be escaped with two backslashes ("\\$") This corresponds to the note in the spec for the Exec key: > 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 ("\\$"). which will also include '\\"' and "\\`". The current version of scrcpy's desktop file works again because without trying to quote the $SHELL variable itself the current escaping ended up correctly escaped with two backslashes: > Exec=/bin/sh -c "\\$SHELL -i -c 'scrcpy --pause-on-exit=if-error'" It would probably be best to instead improve the message further (thanks for the readability patch, that's already much better!) and hint at the need for the double escaping like desktop-file-validate's message above does.
Parsing changed a bit here with the commits that fixed Bug 465290, but beyond that I agree that there doesn't seem to be a bug to fix here.