Ticket #73: do not intercept 'cd' in compound commands#5121
Conversation
github-actions
Bot
added
needs triage
|
|
||
| for (p = cmd; *p != '\0'; p++) | ||
| { | ||
| if (*p == ';' && (p == cmd || *(p - 1) != '\\')) |
There was a problem hiding this comment.
this is unreliable. you need to parse forward.
| { | ||
| if (*p == ';' && (p == cmd || *(p - 1) != '\\')) | ||
| return TRUE; | ||
| if (p[0] == '&' && p[1] == '&') |
There was a problem hiding this comment.
why do you exclude single amps and pipes?
| */ | ||
|
|
||
| gboolean | ||
| has_shell_metacharacters (const char *cmd) |
There was a problem hiding this comment.
i think it would be better to use glib's shell_parse_argv() with NULL output arguments.
Skip internal cd handling when the command line contains unescaped ';', '&&', or '||', letting the shell execute it instead. Signed-off-by: Marek Libra <marek.libra@gmail.com>
mareklibra
force-pushed
the
issue.73.cdSemicolon
branch
from
c5350f7 to
db89c89
Compare
| return TRUE; | ||
| } | ||
|
|
||
| for (p = cmd; *p != '\0'; p++) |
There was a problem hiding this comment.
huh, that's not what i meant. the idea is that you can entirely replace this code with the above call. the remaining comments were only for the case that the idea turns out infeasible.
after a second thought, the idea needs to be revised, of course: you actually need to use the output. that means that you need to weave it somewhat more deeply into enter() below instead of "bolting it on".
There was a problem hiding this comment.
It seems like the GLib doesn't treat ; as a command separator, causing errors for commands like cd ..;ls, cd /tmp;ls, cd ..&&ls, cd & or cd a&b.
On the other hand, a command like cd .. ; ls would work well with g_shell_parse_argv().
For that reason, a forward-parsing has_unquoted_metacharacters() pre-check is still there.
I have added more test cases.
There was a problem hiding this comment.
It seems like the GLib doesn't treat
;as a command separator,
huh, that looks like an outright bug.
maybe they tried fixing over-interpreting # inside words, and caused collateral damage.
There was a problem hiding this comment.
I found this comment: https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gshell.c#L356
It seems like they claim it as a feature, not a bug.
I believe the right behavior for the MC is to be closest to the shell as possible.
@ossilator What do you think about reverting the last change and doing the parsing on our own?
There was a problem hiding this comment.
yeah, it seems that this function is too far removed from actual shells. i thought it would be closer to https://github.com/KDE/kcoreaddons/blob/master/src/lib/util/kshell_unix.cpp#L46 in AbortOnMeta mode. do only our own forward-parsing.
mareklibra
force-pushed
the
issue.73.cdSemicolon
branch
from
0002ffe to
1836b8b
Compare
Signed-off-by: Marek Libra <marek.libra@gmail.com>
mareklibra
force-pushed
the
issue.73.cdSemicolon
branch
from
1836b8b to
17d31ab
Compare
2 participants
Proposed changes
Skip internal cd handling when the command line contains unescaped ';', '&&', or '||', letting the shell execute it instead.
Checklist
👉 Our coding style can be found here: https://midnight-commander.org/coding-style/ 👈
git commit --amend -smake indent && make check)