From f2a52378029f4d6068e874873c30f935e93a5216 Mon Sep 17 00:00:00 2001 From: Ryan Hileman Date: Wed, 18 Sep 2013 17:19:30 -0700 Subject: [PATCH] Improve dangerous/undefined PID expansion behavior 1. Use Bash-like expansion for empty searches (when you just use a '%' by itself). '%' will now *only* match the last valid backgrounded process. If there are no such processes, an expansion error will be generated. '%' by itself would previously match either *all* backgrounded processes, or failing that, all processes owned by your user. If you ever tried to run `kill -9 %`, it would either kill all backgrounded processes or *all* of your processes. I'm not sure why anyone would ever want that to be a single keystroke away. You could almost typo it. As a result, `fg %`, `bg %`, `kill %`, etc will all operate on the last process touched by job control. 2. Don't run 'by-name' matches when the search term is numeric. This prevents you from running a command like `kill %1` and accidentally killing a process named something like "1Command". Overloaded behavior can be dangerous, and we probably shouldn't play fast and loose with expansion characters that generate process IDs. --- expand.cpp | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/expand.cpp b/expand.cpp index 4408a3e4b..45d292491 100644 --- a/expand.cpp +++ b/expand.cpp @@ -585,7 +585,30 @@ static int find_process(const wchar_t *proc, ASSERT_IS_MAIN_THREAD(); const job_t *j; - if (iswnumeric(proc) || (wcslen(proc)==0)) + // do the empty param check first, because an empty string passes our 'numeric' check + if (wcslen(proc)==0) + { + /* + This is an empty job expansion: '%' + It expands to the last job backgrounded. + */ + job_iterator_t jobs; + while ((j = jobs.next())) + { + if (!j->command_is_empty()) + { + append_completion(out, to_string(j->pgid)); + break; + } + } + /* + You don't *really* want to flip a coin between killing + the last process backgrounded and all processes, do you? + Let's not try other match methods with the solo '%' syntax. + */ + found = 1; + } + else if (iswnumeric(proc)) { /* This is a numeric job string, like '%2' @@ -611,11 +634,9 @@ static int find_process(const wchar_t *proc, 0); } } - } else { - int jid; wchar_t *end; @@ -624,15 +645,17 @@ static int find_process(const wchar_t *proc, if (jid > 0 && !errno && !*end) { j = job_get(jid); - if ((j != 0) && (j->command_wcstr() != 0)) + if ((j != 0) && (j->command_wcstr() != 0) && (!j->command_is_empty())) { - { - append_completion(out, to_string(j->pgid)); - found = 1; - } + append_completion(out, to_string(j->pgid)); } } } + /* + Stop here so you can't match a random process name + when you're just trying to use job control. + */ + found = 1; } if (found) return 1;