[pacman-dev] [PATCH v3 11/11][RFC] allow arguments in hook Exec fields

[Andrew Gregory]

Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
---

Using wordexp to split Exec fields makes splitting them, including
proper handling of quoted arguments, extremely easy, but it includes
some features and caveats that I'm not sure we want for this.  From
wordexp(3):

   Since  the  expansion is the same as the expansion by the shell
   (see sh(1)) of the parameters to a command, the string s must not
   contain characters that would be illegal in shell command
   parameters.  In particular, there must not be any unescaped newline
   or |, &, ;, <, >, (, ), {,  }  characters outside a command
   substitution or parameter substitution context.

   If the argument s contains a word that starts with an unquoted
   comment character #, then it is unspecified whether that word and
   all following words are ignored, or the # is treated as
   a non-comment character.

   The expansion done consists of the following stages: tilde
   expansion (replacing ~user by user's home directory),  variable
   substitution  (replacing $FOO  by  the  value of the environment
   variable FOO), command substitution (replacing $(command) or
   `command` by the output of command), arithmetic expansion, field
   splitting, wildcard expansion, quote removal.

   The result of expansion of special parameters ($@, $*, $#, $?, $-,
   $$, $!, $0) is unspecified.

   Field splitting is done using the environment variable $IFS.  If it
   is not set, the field separators are space, tab and newline.

Does anybody have any objection to using wordexp for this?

 lib/libalpm/hook.c                            | 14 ++++++++++----
 test/pacman/tests/TESTS                       |  1 +
 test/pacman/tests/hook-exec-with-arguments.py | 22 ++++++++++++++++++++++
 3 files changed, 33 insertions(+), 4 deletions(-)
 create mode 100644 test/pacman/tests/hook-exec-with-arguments.py

diff --git a/lib/libalpm/hook.c b/lib/libalpm/hook.c
index fe4b204..7485135 100644
--- a/lib/libalpm/hook.c
+++ b/lib/libalpm/hook.c
@@ -20,6 +20,7 @@
 #include <dirent.h>
 #include <errno.h>
 #include <string.h>
+#include <wordexp.h>
 
 #include "handle.h"
 #include "hook.h"
@@ -49,7 +50,7 @@ struct _alpm_hook_t {
 	char *name;
 	alpm_list_t *triggers;
 	alpm_list_t *depends;
-	char *cmd;
+	wordexp_t *cmd;
 	enum _alpm_hook_when_t when;
 	int abort_on_fail;
 };
@@ -71,6 +72,7 @@ static void _alpm_hook_free(struct _alpm_hook_t *hook)
 {
 	if(hook) {
 		free(hook->name);
+		wordfree(hook->cmd);
 		free(hook->cmd);
 		alpm_list_free_inner(hook->triggers, (alpm_list_fn_free) _alpm_trigger_free);
 		alpm_list_free(hook->triggers);
@@ -208,7 +210,12 @@ static int _alpm_hook_parse_cb(const char *file, int line,
 		} else if(strcmp(key, "AbortOnFail") == 0) {
 			hook->abort_on_fail = 1;
 		} else if(strcmp(key, "Exec") == 0) {
-			STRDUP(hook->cmd, value, return 1);
+			wordexp_t *p = calloc(sizeof(wordexp_t), 1);
+			if(wordexp(value, p, WRDE_NOCMD | WRDE_UNDEF) == 0) {
+				hook->cmd = p;
+			} else {
+				error(_("hook %s line %d: invalid Exec %s\n"), file, line, value);
+			}
 		} else {
 			error(_("hook %s line %d: invalid option %s\n"), file, line, value);
 		}
@@ -378,7 +385,6 @@ static alpm_list_t *find_hook(alpm_list_t *haystack, const void *needle)
 static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook)
 {
 	alpm_list_t *i, *pkgs = _alpm_db_get_pkgcache(handle->db_local);
-	char *const argv[] = { hook->cmd, NULL };
 
 	for(i = hook->depends; i; i = i->next) {
 		if(!alpm_find_satisfier(pkgs, i->data)) {
@@ -388,7 +394,7 @@ static int _alpm_hook_run_hook(alpm_handle_t *handle, struct _alpm_hook_t *hook)
 		}
 	}
 
-	return _alpm_run_chroot(handle, hook->cmd, argv);
+	return _alpm_run_chroot(handle, hook->cmd->we_wordv[0], hook->cmd->we_wordv);
 }
 
 int _alpm_hook_run(alpm_handle_t *handle, enum _alpm_hook_when_t when)
diff --git a/test/pacman/tests/TESTS b/test/pacman/tests/TESTS
index 8ad1b9c..afd2e69 100644
--- a/test/pacman/tests/TESTS
+++ b/test/pacman/tests/TESTS
@@ -51,6 +51,7 @@ TESTS += test/pacman/tests/fileconflict030.py
 TESTS += test/pacman/tests/fileconflict031.py
 TESTS += test/pacman/tests/fileconflict032.py
 TESTS += test/pacman/tests/hook-abortonfail.py
+TESTS += test/pacman/tests/hook-exec-with-arguments.py
 TESTS += test/pacman/tests/hook-file-change-packages.py
 TESTS += test/pacman/tests/hook-file-remove-trigger-match.py
 TESTS += test/pacman/tests/hook-file-upgrade-nomatch.py
diff --git a/test/pacman/tests/hook-exec-with-arguments.py b/test/pacman/tests/hook-exec-with-arguments.py
new file mode 100644
index 0000000..d3df87b
--- /dev/null
+++ b/test/pacman/tests/hook-exec-with-arguments.py
@@ -0,0 +1,22 @@
+self.description = "Hook with arguments"
+
+self.add_hook("hook",
+        """
+        [Trigger]
+        Type = Package
+        Operation = Install
+        Target = foo
+
+        [Action]
+        When = PreTransaction
+        Exec = bin/sh -c ': > hook-output'
+        """);
+
+sp = pmpkg("foo")
+self.addpkg2db("sync", sp)
+
+self.args = "-S foo"
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("PKG_EXIST=foo")
+self.addrule("FILE_EXIST=hook-output")
-- 
2.6.1