From 09f43fa9c089ebfc18401ce84755d3f2000ba033 Mon Sep 17 00:00:00 2001 From: Robert Kanter Date: Wed, 9 Nov 2016 13:34:40 +0100 Subject: [PATCH] YARN-5736. YARN container executor config does not handle white space (miklos.szegedi@cloudera.com via rkanter) --- .../container-executor/impl/configuration.c | 41 ++++++++-- .../container-executor/impl/configuration.h | 9 +++ .../native/container-executor/impl/main.c | 5 +- .../test/test-container-executor.c | 77 ++++++++++++++++++- 4 files changed, 123 insertions(+), 9 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c index 69ceaf6d4c0..8da7d24339f 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.c @@ -31,6 +31,7 @@ #include #include #include +#include #define MAX_SIZE 10 @@ -126,6 +127,37 @@ int check_configuration_permissions(const char* file_name) { return 0; } +/** + * Trim whitespace from beginning and end. +*/ +char* trim(char* input) +{ + char *val_begin; + char *val_end; + char *ret; + + if (input == NULL) { + return NULL; + } + + val_begin = input; + val_end = input + strlen(input); + + while (val_begin < val_end && isspace(*val_begin)) + val_begin++; + while (val_end > val_begin && isspace(*(val_end - 1))) + val_end--; + + ret = (char *) malloc( + sizeof(char) * (val_end - val_begin + 1)); + if (ret == NULL) { + fprintf(ERRORFILE, "Allocation error\n"); + exit(OUT_OF_MEMORY); + } + + strncpy(ret, val_begin, val_end - val_begin); + return ret; +} void read_config(const char* file_name, struct configuration *cfg) { FILE *conf_file; @@ -202,9 +234,8 @@ void read_config(const char* file_name, struct configuration *cfg) { #endif memset(cfg->confdetails[cfg->size], 0, sizeof(struct confentry)); - cfg->confdetails[cfg->size]->key = (char *) malloc( - sizeof(char) * (strlen(equaltok)+1)); - strcpy((char *)cfg->confdetails[cfg->size]->key, equaltok); + cfg->confdetails[cfg->size]->key = trim(equaltok); + equaltok = strtok_r(NULL, "=", &temp_equaltok); if (equaltok == NULL) { fprintf(LOGFILE, "configuration tokenization failed \n"); @@ -222,9 +253,7 @@ void read_config(const char* file_name, struct configuration *cfg) { fprintf(LOGFILE, "read_config : Adding conf value : %s \n", equaltok); #endif - cfg->confdetails[cfg->size]->value = (char *) malloc( - sizeof(char) * (strlen(equaltok)+1)); - strcpy((char *)cfg->confdetails[cfg->size]->value, equaltok); + cfg->confdetails[cfg->size]->value = trim(equaltok); if((cfg->size + 1) % MAX_SIZE == 0) { cfg->confdetails = (struct confentry **) realloc(cfg->confdetails, sizeof(struct confentry **) * (MAX_SIZE + cfg->size)); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h index eced13b0b47..2d14867a0c5 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/configuration.h @@ -105,3 +105,12 @@ int get_kv_key(const char *input, char *out, size_t out_len); * 0 on success */ int get_kv_value(const char *input, char *out, size_t out_len); + +/** + * Trim whitespace from beginning and end. + * + * @param input Input string that needs to be trimmed + * + * @return the trimmed string allocated with malloc. I has to be freed by the caller +*/ +char* trim(char* input); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c index 47bb3b9a243..fdc0496986d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/impl/main.c @@ -143,7 +143,8 @@ static void assert_valid_setup(char *argv0) { int ret; char *executable_file = get_executable(argv0); if (!executable_file) { - fprintf(ERRORFILE,"realpath of executable: %s\n",strerror(errno)); + fprintf(ERRORFILE,"realpath of executable: %s\n", + errno != 0 ? strerror(errno) : "unknown"); flush_and_close_log_files(); exit(-1); } @@ -178,7 +179,7 @@ static void assert_valid_setup(char *argv0) { if (group_info == NULL) { free(executable_file); fprintf(ERRORFILE, "Can't get group information for %s - %s.\n", nm_group, - strerror(errno)); + errno != 0 ? strerror(errno) : "unknown"); flush_and_close_log_files(); exit(INVALID_CONFIG_FILE); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c index f7d4975e649..0c9e496bde6 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/native/container-executor/test/test-container-executor.c @@ -565,7 +565,7 @@ void test_list_as_user() { } // Test with empty dir string - sprintf(buffer, ""); + strcpy(buffer, ""); int ret = list_as_user(buffer); if (ret == 0) { @@ -1051,6 +1051,77 @@ void test_dir_permissions() { free(container_dir); } +/** + * This test is used to verify that trim() works correctly + */ +void test_trim_function() { + char* trimmed = NULL; + + printf("\nTesting trim function\n"); + + // Check NULL input + if (trim(NULL) != NULL) { + printf("FAIL: trim(NULL) should be NULL\n"); + exit(1); + } + + // Check empty input + trimmed = trim(""); + if (strcmp(trimmed, "") != 0) { + printf("FAIL: trim(\"\") should be \"\"\n"); + exit(1); + } + free(trimmed); + + // Check single space input + trimmed = trim(" "); + if (strcmp(trimmed, "") != 0) { + printf("FAIL: trim(\" \") should be \"\"\n"); + exit(1); + } + free(trimmed); + + // Check multi space input + trimmed = trim(" "); + if (strcmp(trimmed, "") != 0) { + printf("FAIL: trim(\" \") should be \"\"\n"); + exit(1); + } + free(trimmed); + + // Check both side trim input + trimmed = trim(" foo "); + if (strcmp(trimmed, "foo") != 0) { + printf("FAIL: trim(\" foo \") should be \"foo\"\n"); + exit(1); + } + free(trimmed); + + // Check left side trim input + trimmed = trim("foo "); + if (strcmp(trimmed, "foo") != 0) { + printf("FAIL: trim(\"foo \") should be \"foo\"\n"); + exit(1); + } + free(trimmed); + + // Check right side trim input + trimmed = trim(" foo"); + if (strcmp(trimmed, "foo") != 0) { + printf("FAIL: trim(\" foo\") should be \"foo\"\n"); + exit(1); + } + free(trimmed); + + // Check no trim input + trimmed = trim("foo"); + if (strcmp(trimmed, "foo") != 0) { + printf("FAIL: trim(\"foo\") should be \"foo\"\n"); + exit(1); + } + free(trimmed); +} + // This test is expected to be executed either by a regular // user or by root. If executed by a regular user it doesn't // test all the functions that would depend on changing the @@ -1218,9 +1289,13 @@ int main(int argc, char **argv) { #endif run("rm -fr " TEST_ROOT); + + test_trim_function(); + printf("\nFinished tests\n"); free(current_username); free_executor_configurations(); + return 0; }