It is very unclear at this point what a valid use case for this feature
would possibly be. The old documentation only mentions $(hostname) as
an example, which can be done with $HOSTNAME instead.
Note that $(...) is still supported in Exec lines of desktop files,
this does not require [$e] anyway (and actually works better without it,
otherwise the $ signs need to be doubled to obey kconfig $e escaping rules...).
ctest passes; various testcases with $(...) in desktop files,
directory files, and config files, no longer execute commands.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
| docs/options.md | ||
|---|---|---|
| 69–70 | "and shell commands" to be dropped here no? | |
LGTM. Regarding the test, if we want to get this change in asap due to the security focus I can submit a follow up patch re-adding it.
| autotests/kconfigtest.cpp | ||
|---|---|---|
| 530 | Instead of removing this test, can it instead be switched to verify the command execution does not occur? | |
| docs/options.md | ||
| 76–77 | Grammar suggestion: Note that the application will replace $USER with its expanded values after saving. | |
+1
I got a valid usecase "Please don't fix this. I use a recursive symlink and a shell script to raise my machine load. The extra heat it produces keeps children warm in the winter." ;)
| autotests/kconfigtest.cpp | ||
|---|---|---|
| 530 | Hehe, that's what I did initially, and the value being read was (hostname) without the $ because of the way [$e] works. A bit surprising, but in line with the fact that $/ $? $@ etc would also remove the $ (because the code just sees an empty env var name), and if someone wanted to keep the $ they would have to write $$. So I concluded invalid testcase, nobody would write this anymore. But OK, it's a test about old files that might have this. I'll re-add the test. | |
Does anyone think this can be easily backported to previous versions of KDE in upstream distros such as Kubuntu, etc.?
I backported this down to KConfig 5.20 and KDELibs 4.14.18, differences were trivial to resolve.