From 8d41cc4cae580fc506fd65fabc24b56d4e50da1d Mon Sep 17 00:00:00 2001 From: lifeng68 Date: Wed, 20 May 2020 00:14:34 -0400 Subject: [PATCH] env: ignore env with only key,like -e AAA Signed-off-by: lifeng68 --- CI/test_cases/basic_cases/run.bash | 13 +++ src/cmd/isula/base/create.c | 68 ++++----------- src/cmd/isula/stream/exec.c | 86 ++++++++++++++----- src/cutils/utils.c | 46 ++++++++++ src/cutils/utils.h | 2 + src/libisula.c | 31 +++---- src/services/execution/spec/verify.c | 31 +++++++ src/websocket/service/exec_serve.cc | 2 +- .../execution/spec/selinux_label_mock_llt.cc | 2 +- 9 files changed, 186 insertions(+), 95 deletions(-) diff --git a/CI/test_cases/basic_cases/run.bash b/CI/test_cases/basic_cases/run.bash index 17387b0d2..b8adfbf96 100755 --- a/CI/test_cases/basic_cases/run.bash +++ b/CI/test_cases/basic_cases/run.bash @@ -48,6 +48,19 @@ function do_test_t() isula rm $containername fn_check_eq "$?" "0" "rm failed" + echo AA > /tmp/test_run_env + + isula run --name $containername -itd -e AA=BB -e AA --env-file /tmp/test_run_env busybox + fn_check_eq "$?" "0" "run failed" + testcontainer $containername running + + isula stop -t 0 $containername + fn_check_eq "$?" "0" "stop failed" + testcontainer $containername exited + + isula rm $containername + fn_check_eq "$?" "0" "rm failed" + return $TC_RET_T } diff --git a/src/cmd/isula/base/create.c b/src/cmd/isula/base/create.c index a1a44a1a6..bd751331f 100644 --- a/src/cmd/isula/base/create.c +++ b/src/cmd/isula/base/create.c @@ -229,18 +229,27 @@ static int request_pack_custom_env(struct client_arguments *args, isula_containe { int ret = 0; char *pe = NULL; + char *new_env = NULL; if (args->custom_conf.env != NULL) { size_t i; for (i = 0; i < util_array_len((const char **)(args->custom_conf.env)); i++) { - if (util_array_append(&conf->env, args->custom_conf.env[i]) != 0) { - COMMAND_ERROR("Failed to append custom config env list"); + if (util_validate_env(args->custom_conf.env[i], &new_env) != 0) { + COMMAND_ERROR("Invalid environment %s", args->custom_conf.env[i]); + ret = -1; + goto out; + } + if (new_env == NULL) { + continue; + } + if (util_array_append(&conf->env, new_env) != 0) { + COMMAND_ERROR("Failed to append custom config env list %s", new_env); ret = -1; goto out; } + free(new_env); + new_env = NULL; } - util_free_array(args->custom_conf.env); - args->custom_conf.env = conf->env; /* make sure args->custom_conf.env point to valid memory. */ conf->env_len = util_array_len((const char **)(conf->env)); } @@ -253,9 +262,7 @@ static int request_pack_custom_env(struct client_arguments *args, isula_containe goto out; } } - args->custom_conf.env = conf->env; /* make sure args->custom_conf.env point to valid memory. */ conf->env_len = util_array_len((const char **)(conf->env)); - conf->accel = args->custom_conf.accel; conf->accel_len = util_array_len((const char **)(args->custom_conf.accel)); if (util_env_set_isulad_enable_plugins(&conf->env, &conf->env_len, ISULAD_ISULA_ADAPTER)) { @@ -267,52 +274,7 @@ static int request_pack_custom_env(struct client_arguments *args, isula_containe out: free(pe); - return ret; -} - -static int validate_env(const char *env, char **dst) -{ - int ret = 0; - char *value = NULL; - char **arr = util_string_split_multi(env, '='); - if (arr == NULL) { - ERROR("Failed to split env string"); - return -1; - } - if (strlen(arr[0]) == 0) { - ERROR("Invalid environment variable: %s", env); - ret = -1; - goto out; - } - - if (util_array_len((const char **)arr) > 1) { - *dst = util_strdup_s(env); - goto out; - } - - value = getenv(env); - if (value == NULL) { - *dst = util_strdup_s(env); - goto out; - } else { - int sret; - size_t len = strlen(env) + 1 + strlen(value) + 1; - *dst = (char *)util_common_calloc_s(len); - if (*dst == NULL) { - ERROR("Out of memory"); - ret = -1; - goto out; - } - sret = snprintf(*dst, len, "%s=%s", env, value); - if (sret < 0 || (size_t)sret >= len) { - ERROR("Failed to compose env string"); - ret = -1; - goto out; - } - } - -out: - util_free_array(arr); + free(new_env); return ret; } @@ -343,7 +305,7 @@ static int read_env_from_file(const char *path, size_t file_size, isula_containe continue; } buf[len - 1] = '\0'; - if (validate_env(buf, &new_env) != 0) { + if (util_validate_env(buf, &new_env) != 0) { ret = -1; goto out; } diff --git a/src/cmd/isula/stream/exec.c b/src/cmd/isula/stream/exec.c index 5494caf65..65aa4f29c 100644 --- a/src/cmd/isula/stream/exec.c +++ b/src/cmd/isula/stream/exec.c @@ -38,11 +38,64 @@ sem_t g_command_waitexit_sem; struct client_arguments g_cmd_exec_args = {}; +static int fill_exec_request(const struct client_arguments *args, const struct command_fifo_config *fifos, + struct isula_exec_request *request) +{ + int ret = 0; + size_t i = 0; + char *new_env = NULL; + + request->name = util_strdup_s(args->name); + request->suffix = util_strdup_s(args->exec_suffix); + request->tty = args->custom_conf.tty; + request->open_stdin = args->custom_conf.open_stdin; + request->attach_stdin = args->custom_conf.attach_stdin; + request->attach_stdout = args->custom_conf.attach_stdout; + request->attach_stderr = args->custom_conf.attach_stderr; + if (fifos != NULL) { + request->stdin = util_strdup_s(fifos->stdin_name); + request->stdout = util_strdup_s(fifos->stdout_name); + request->stderr = util_strdup_s(fifos->stderr_name); + } + + request->user = util_strdup_s(args->custom_conf.user); + + if (dup_array_of_strings((const char **)args->argv, args->argc, &(request->argv), (size_t *) & (request->argc)) != 0) { + ERROR("Failed to dup args"); + ret = -1; + goto out; + } + + /* environment variables */ + for (i = 0; i < util_array_len((const char **)(args->extra_env)); i++) { + if (util_validate_env(args->extra_env[i], &new_env) != 0) { + ERROR("Invalid environment %s", args->extra_env[i]); + ret = -1; + goto out; + } + if (new_env == NULL) { + continue; + } + if (util_array_append(&request->env, new_env) != 0) { + ERROR("Failed to append custom config env list %s", new_env); + ret = -1; + goto out; + } + request->env_len++; + free(new_env); + new_env = NULL; + } + +out: + free(new_env); + return ret; +} + static int client_exec(const struct client_arguments *args, const struct command_fifo_config *fifos, uint32_t *exit_code) { isula_connect_ops *ops = NULL; - struct isula_exec_request request = { 0 }; + struct isula_exec_request *request = NULL; struct isula_exec_response *response = NULL; client_connect_config_t config = { 0 }; int ret = 0; @@ -53,26 +106,18 @@ static int client_exec(const struct client_arguments *args, const struct command return ECOMMON; } - request.name = args->name; - request.suffix = args->exec_suffix; - request.tty = args->custom_conf.tty; - request.open_stdin = args->custom_conf.open_stdin; - request.attach_stdin = args->custom_conf.attach_stdin; - request.attach_stdout = args->custom_conf.attach_stdout; - request.attach_stderr = args->custom_conf.attach_stderr; - if (fifos != NULL) { - request.stdin = fifos->stdin_name; - request.stdout = fifos->stdout_name; - request.stderr = fifos->stderr_name; + request = util_common_calloc_s(sizeof(struct isula_exec_request)); + if (request == NULL) { + ERROR("Out of memory"); + ret = ECOMMON; + goto out; } - request.user = args->custom_conf.user; - request.argc = args->argc; - request.argv = (char **)args->argv; - - /* environment variables */ - request.env_len = util_array_len((const char **)(args->extra_env)); - request.env = args->extra_env; + if (fill_exec_request(args, fifos, request) != 0) { + ERROR("Failed to fill exec request"); + ret = ECOMMON; + goto out; + } ops = get_connect_client_ops(); if (ops == NULL || !ops->container.exec) { @@ -82,7 +127,7 @@ static int client_exec(const struct client_arguments *args, const struct command } config = get_connect_config(args); - ret = ops->container.exec(&request, response, &config); + ret = ops->container.exec(request, response, &config); if (ret) { client_print_error(response->cc, response->server_errono, response->errmsg); ret = ECOMMON; @@ -94,6 +139,7 @@ out: } isula_exec_response_free(response); + isula_exec_request_free(request); return ret; } diff --git a/src/cutils/utils.c b/src/cutils/utils.c index d31484e44..b5565cd0c 100644 --- a/src/cutils/utils.c +++ b/src/cutils/utils.c @@ -1509,3 +1509,49 @@ void add_array_kv(char **array, size_t total, size_t *pos, const char *k, const add_array_elem(array, total, pos, v); } +int util_validate_env(const char *env, char **dst) +{ + int ret = 0; + char *value = NULL; + + char **arr = util_string_split_multi(env, '='); + if (arr == NULL) { + ERROR("Failed to split env string"); + return -1; + } + if (strlen(arr[0]) == 0) { + ERROR("Invalid environment variable: %s", env); + ret = -1; + goto out; + } + + if (util_array_len((const char **)arr) > 1) { + *dst = util_strdup_s(env); + goto out; + } + + value = getenv(env); + if (value == NULL) { + *dst = NULL; + goto out; + } else { + int sret; + size_t len = strlen(env) + 1 + strlen(value) + 1; + *dst = (char *)util_common_calloc_s(len); + if (*dst == NULL) { + ERROR("Out of memory"); + ret = -1; + goto out; + } + sret = snprintf(*dst, len, "%s=%s", env, value); + if (sret < 0 || (size_t)sret >= len) { + ERROR("Failed to compose env string"); + ret = -1; + goto out; + } + } + +out: + util_free_array(arr); + return ret; +} diff --git a/src/cutils/utils.h b/src/cutils/utils.h index ff6c6919a..580ece52f 100644 --- a/src/cutils/utils.h +++ b/src/cutils/utils.h @@ -431,6 +431,8 @@ void add_array_kv(char **array, size_t total, size_t *pos, const char *k, const typedef int (*mount_info_call_back_t)(const char *, const char *); bool util_deal_with_mount_info(mount_info_call_back_t cb, const char *); +int util_validate_env(const char *env, char **dst); + #ifdef __cplusplus } #endif diff --git a/src/libisula.c b/src/libisula.c index 7ed8ae90c..f9fec676d 100644 --- a/src/libisula.c +++ b/src/libisula.c @@ -664,26 +664,17 @@ void isula_exec_request_free(struct isula_exec_request *request) free(request->stderr); request->stderr = NULL; - if (request->argc && request->argv != NULL) { - int i; - for (i = 0; i < request->argc; i++) { - free(request->argv[i]); - request->argv[i] = NULL; - } - free(request->argv); - request->argv = NULL; - request->argc = 0; - } - if (request->env_len && request->env != NULL) { - size_t j; - for (j = 0; j < request->env_len; j++) { - free(request->env[j]); - request->env[j] = NULL; - } - free(request->env); - request->env = NULL; - request->env_len = 0; - } + free(request->user); + request->user = NULL; + + util_free_array_by_len(request->argv, request->argc); + request->argv = NULL; + request->argc = 0; + + util_free_array_by_len(request->env, request->env_len); + request->env = NULL; + request->env_len = 0; + free(request); } diff --git a/src/services/execution/spec/verify.c b/src/services/execution/spec/verify.c index a06649213..3367fda84 100644 --- a/src/services/execution/spec/verify.c +++ b/src/services/execution/spec/verify.c @@ -1482,6 +1482,29 @@ out: return ret; } +/* verify oci linux */ +static int verify_process_env(const defs_process *process) +{ + int ret = 0; + size_t i = 0; + char *new_env = NULL; + + for (i = 0; i < process->env_len; i++) { + if (util_validate_env(process->env[i], &new_env) != 0) { + ERROR("Invalid environment %s", process->env[i]); + isulad_set_error_message("Invalid environment %s", process->env[i]); + ret = -1; + goto out; + } + free(new_env); + new_env = NULL; + } + +out: + free(new_env); + return ret; +} + static int verify_container_linux(const oci_runtime_spec *container, const sysinfo_t *sysinfo) { int ret = 0; @@ -1498,6 +1521,14 @@ static int verify_container_linux(const oci_runtime_spec *container, const sysin } } + /* verify oci spec process settings */ + if (container->process != NULL) { + ret = verify_process_env(container->process); + if (ret != 0) { + goto out; + } + } + out: return ret; } diff --git a/src/websocket/service/exec_serve.cc b/src/websocket/service/exec_serve.cc index d3528a98b..66a083532 100644 --- a/src/websocket/service/exec_serve.cc +++ b/src/websocket/service/exec_serve.cc @@ -53,7 +53,7 @@ int ExecServe::Execute(struct lws *wsi, const std::string &token, StderrstringWriter.write_func = WsWriteStderrToClient; int ret = cb->container.exec(container_req, &container_res, container_req->attach_stdin ? read_pipe_fd : -1, - container_req->attach_stdout ? &StdoutstringWriter: nullptr, + container_req->attach_stdout ? &StdoutstringWriter : nullptr, container_req->attach_stderr ? &StderrstringWriter : nullptr); if (ret != 0) { std::string message; diff --git a/test/services/execution/spec/selinux_label_mock_llt.cc b/test/services/execution/spec/selinux_label_mock_llt.cc index 94f84173e..eecdc4c8d 100644 --- a/test/services/execution/spec/selinux_label_mock_llt.cc +++ b/test/services/execution/spec/selinux_label_mock_llt.cc @@ -56,7 +56,7 @@ TEST_F(SELinuxGetEnableUnitTest, test_selinux_get_enable_normal) const uint32_t selinuxfsMagic = 0xf97cff8c; struct statfs sfbuf { .f_type = selinuxfsMagic, - .f_flags = 0 + .f_flags = 0 }; EXPECT_CALL(m_syscall, Statfs(_, _)) -- Gitee