config: tokenize key bindings' pipe command when loading the configuration

This allows us to detect syntax errors early on, and is also more
efficient since we don't have to re-tokenize the command line every
time the binding is executed.
This commit is contained in:
Daniel Eklöf 2020-07-30 18:53:51 +02:00
parent 6a186cb356
commit 7767041c2c
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
5 changed files with 50 additions and 65 deletions

View file

@ -35,6 +35,13 @@
* Renamed man page for `footrc` from **foot**(5) to **footrc**(5). * Renamed man page for `footrc` from **foot**(5) to **footrc**(5).
### Fixed
* Command lines for **pipe-visible** and **pipe-scrollback** are now
tokenized (i.e. syntax checked) when the configuration is loaded,
instead of every time the key binding is executed.
### Security ### Security
### Contributors ### Contributors

View file

@ -20,6 +20,7 @@
#define LOG_ENABLE_DBG 0 #define LOG_ENABLE_DBG 0
#include "log.h" #include "log.h"
#include "input.h" #include "input.h"
#include "tokenize.h"
#include "util.h" #include "util.h"
#include "wayland.h" #include "wayland.h"
@ -624,7 +625,8 @@ parse_section_key_bindings(
const char *key, const char *value, struct config *conf, const char *key, const char *value, struct config *conf,
const char *path, unsigned lineno) const char *path, unsigned lineno)
{ {
const char *pipe_cmd = NULL; char *pipe_cmd = NULL;
char **pipe_argv = NULL;
size_t pipe_len = 0; size_t pipe_len = 0;
if (value[0] == '[') { if (value[0] == '[') {
@ -634,10 +636,18 @@ parse_section_key_bindings(
return false; return false;
} }
pipe_cmd = &value[1];
pipe_len = pipe_cmd_end - pipe_cmd; pipe_len = pipe_cmd_end - pipe_cmd;
pipe_cmd = strndup(&value[1], pipe_len);
if (!tokenize_cmdline(pipe_cmd, &pipe_argv)) {
LOG_ERR("%s:%d: syntax error in command line", path, lineno);
free(pipe_cmd);
return false;
}
value = pipe_cmd_end + 1; value = pipe_cmd_end + 1;
while (isspace(*value))
value++;
} }
for (enum bind_action_normal action = 0; for (enum bind_action_normal action = 0;
@ -654,31 +664,37 @@ parse_section_key_bindings(
tll_foreach(conf->bindings.key, it) { tll_foreach(conf->bindings.key, it) {
if (it->item.action == action) { if (it->item.action == action) {
free(it->item.key); free(it->item.key);
free(it->item.pipe_cmd); free(it->item.pipe.cmd);
free(it->item.pipe.argv);
tll_remove(conf->bindings.key, it); tll_remove(conf->bindings.key, it);
} }
} }
free(pipe_argv);
free(pipe_cmd);
return true; return true;
} }
if (!verify_key_combo(conf, action, binding_action_map, value, path, lineno)) { if (!verify_key_combo(conf, action, binding_action_map, value, path, lineno)) {
free(pipe_argv);
free(pipe_cmd);
return false; return false;
} }
bool already_added = false; bool already_added = false;
tll_foreach(conf->bindings.key, it) { tll_foreach(conf->bindings.key, it) {
if (it->item.action == action && if (it->item.action == action &&
((it->item.pipe_cmd == NULL && pipe_cmd == NULL) || ((it->item.pipe.argv == NULL && pipe_argv == NULL) ||
(it->item.pipe_cmd != NULL && pipe_cmd != NULL && (it->item.pipe.argv != NULL && pipe_argv != NULL &&
strncmp(it->item.pipe_cmd, pipe_cmd, pipe_len) == 0))) argv_compare(it->item.pipe.argv, pipe_argv) == 0)))
{ {
free(it->item.key); free(it->item.key);
free(it->item.pipe_cmd); free(it->item.pipe.cmd);
free(it->item.pipe.argv);
it->item.key = strdup(value); it->item.key = strdup(value);
it->item.pipe_cmd = pipe_cmd != NULL it->item.pipe.cmd = pipe_cmd;
? strndup(pipe_cmd, pipe_len) : NULL; it->item.pipe.argv = pipe_argv;
already_added = true; already_added = true;
break; break;
} }
@ -688,7 +704,10 @@ parse_section_key_bindings(
struct config_key_binding_normal binding = { struct config_key_binding_normal binding = {
.action = action, .action = action,
.key = strdup(value), .key = strdup(value),
.pipe_cmd = pipe_cmd != NULL ? strndup(pipe_cmd, pipe_len) : NULL, .pipe = {
.cmd = pipe_cmd,
.argv = pipe_argv,
},
}; };
tll_push_back(conf->bindings.key, binding); tll_push_back(conf->bindings.key, binding);
} }
@ -705,24 +724,6 @@ parse_section_search_bindings(
const char *key, const char *value, struct config *conf, const char *key, const char *value, struct config *conf,
const char *path, unsigned lineno) const char *path, unsigned lineno)
{ {
#if 0
const char *pipe_cmd = NULL;
size_t pipe_len = 0;
if (value[0] == '[') {
const char *pipe_cmd_end = strrchr(value, ']');
if (pipe_cmd_end == NULL) {
LOG_ERR("%s:%d: unclosed '['", path, lineno);
return false;
}
pipe_cmd = &value[1];
pipe_len = pipe_cmd_end - pipe_cmd;
value = pipe_cmd_end + 1;
}
#endif
for (enum bind_action_search action = 0; for (enum bind_action_search action = 0;
action < BIND_ACTION_SEARCH_COUNT; action < BIND_ACTION_SEARCH_COUNT;
action++) action++)
@ -737,7 +738,6 @@ parse_section_search_bindings(
tll_foreach(conf->bindings.search, it) { tll_foreach(conf->bindings.search, it) {
if (it->item.action == action) { if (it->item.action == action) {
free(it->item.key); free(it->item.key);
// free(it->item.pipe_cmd);
tll_remove(conf->bindings.search, it); tll_remove(conf->bindings.search, it);
} }
} }
@ -750,24 +750,11 @@ parse_section_search_bindings(
bool already_added = false; bool already_added = false;
tll_foreach(conf->bindings.search, it) { tll_foreach(conf->bindings.search, it) {
if (it->item.action == action if (it->item.action == action) {
#if 0
&&
((it->item.pipe_cmd == NULL && pipe_cmd == NULL) ||
(it->item.pipe_cmd != NULL && pipe_cmd != NULL &&
strncmp(it->item.pipe_cmd, pipe_cmd, pipe_len) == 0))
#endif
)
{
free(it->item.key); free(it->item.key);
// free(it->item.pipe_cmd);
it->item.key = strdup(value); it->item.key = strdup(value);
#if 0
it->item.pipe_cmd = pipe_cmd != NULL
? strndup(pipe_cmd, pipe_len) : NULL;
#endif
already_added = true; already_added = true;
break; break;
} }
@ -777,7 +764,6 @@ parse_section_search_bindings(
struct config_key_binding_search binding = { struct config_key_binding_search binding = {
.action = action, .action = action,
.key = strdup(value), .key = strdup(value),
// .pipe_cmd = pipe_cmd != NULL ? strndup(pipe_cmd, pipe_len) : NULL,
}; };
tll_push_back(conf->bindings.search, binding); tll_push_back(conf->bindings.search, binding);
} }
@ -1289,7 +1275,8 @@ config_free(struct config conf)
tll_foreach(conf.bindings.key, it) { tll_foreach(conf.bindings.key, it) {
free(it->item.key); free(it->item.key);
free(it->item.pipe_cmd); free(it->item.pipe.cmd);
free(it->item.pipe.argv);
} }
tll_foreach(conf.bindings.search, it) tll_foreach(conf.bindings.search, it)
free(it->item.key); free(it->item.key);

View file

@ -17,7 +17,10 @@ struct config_font {
struct config_key_binding_normal { struct config_key_binding_normal {
enum bind_action_normal action; enum bind_action_normal action;
char *key; char *key;
char *pipe_cmd; struct {
char *cmd;
char **argv;
} pipe;
}; };
struct config_key_binding_search { struct config_key_binding_search {

24
input.c
View file

@ -76,7 +76,7 @@ pipe_closed:
static void static void
execute_binding(struct seat *seat, struct terminal *term, execute_binding(struct seat *seat, struct terminal *term,
enum bind_action_normal action, const char *pipe_cmd, enum bind_action_normal action, char *const *pipe_argv,
uint32_t serial) uint32_t serial)
{ {
switch (action) { switch (action) {
@ -146,7 +146,7 @@ execute_binding(struct seat *seat, struct terminal *term,
case BIND_ACTION_PIPE_SCROLLBACK: case BIND_ACTION_PIPE_SCROLLBACK:
case BIND_ACTION_PIPE_VIEW: { case BIND_ACTION_PIPE_VIEW: {
if (pipe_cmd == NULL) if (pipe_argv == NULL)
break; break;
struct pipe_context *ctx = NULL; struct pipe_context *ctx = NULL;
@ -158,12 +158,6 @@ execute_binding(struct seat *seat, struct terminal *term,
char *text = NULL; char *text = NULL;
size_t len = 0; size_t len = 0;
char *cmd = strdup(pipe_cmd);
char **argv = NULL;
if (!tokenize_cmdline(cmd, &argv))
goto pipe_err;
if (pipe(pipe_fd) < 0) { if (pipe(pipe_fd) < 0) {
LOG_ERRNO("failed to create pipe"); LOG_ERRNO("failed to create pipe");
goto pipe_err; goto pipe_err;
@ -207,13 +201,9 @@ execute_binding(struct seat *seat, struct terminal *term,
} }
} }
if (!spawn(term->reaper, NULL, argv, pipe_fd[0], stdout_fd, stderr_fd)) if (!spawn(term->reaper, NULL, pipe_argv, pipe_fd[0], stdout_fd, stderr_fd))
goto pipe_err; goto pipe_err;
/* Not needed anymore */
free(argv); argv = NULL;
free(cmd); cmd = NULL;
/* Close read end */ /* Close read end */
close(pipe_fd[0]); close(pipe_fd[0]);
@ -239,8 +229,6 @@ execute_binding(struct seat *seat, struct terminal *term,
if (pipe_fd[1] >= 0) if (pipe_fd[1] >= 0)
close(pipe_fd[1]); close(pipe_fd[1]);
free(text); free(text);
free(argv);
free(cmd);
free(ctx); free(ctx);
break; break;
} }
@ -426,7 +414,7 @@ keyboard_keymap(void *data, struct wl_keyboard *wl_keyboard,
((struct key_binding_normal){ ((struct key_binding_normal){
.bind = it2->item, .bind = it2->item,
.action = it->item.action, .action = it->item.action,
.pipe_cmd = it->item.pipe_cmd})); .pipe_argv = it->item.pipe.argv}));
} }
tll_free(bindings); tll_free(bindings);
} }
@ -743,14 +731,14 @@ keyboard_key(void *data, struct wl_keyboard *wl_keyboard, uint32_t serial,
/* Match symbol */ /* Match symbol */
if (it->item.bind.sym == sym) { if (it->item.bind.sym == sym) {
execute_binding(seat, term, it->item.action, it->item.pipe_cmd, serial); execute_binding(seat, term, it->item.action, it->item.pipe_argv, serial);
goto maybe_repeat; goto maybe_repeat;
} }
/* Match raw key code */ /* Match raw key code */
tll_foreach(it->item.bind.key_codes, code) { tll_foreach(it->item.bind.key_codes, code) {
if (code->item == key) { if (code->item == key) {
execute_binding(seat, term, it->item.action, it->item.pipe_cmd, serial); execute_binding(seat, term, it->item.action, it->item.pipe_argv, serial);
goto maybe_repeat; goto maybe_repeat;
} }
} }

View file

@ -47,7 +47,7 @@ enum bind_action_normal {
struct key_binding_normal { struct key_binding_normal {
struct key_binding bind; struct key_binding bind;
enum bind_action_normal action; enum bind_action_normal action;
const char *pipe_cmd; char **pipe_argv;
}; };
struct mouse_binding { struct mouse_binding {