From 845aba3ddcfaf7bcef283002b7b5bbf2075d4539 Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Fri, 23 Nov 2012 10:33:12 +0530 Subject: glusterd: fail vol set when value = empty string/string with all whitespaces PROBLEM: 'volume set' operation accepts empty strings and strings containing all whitespaces for value. The result - subsequent attempts to start glusterd fail. FIX: Added relevant checks in xlator_option_validate_* functions in options.c and in conversion functions in common-utils.c to invalidate wrong option values. Change-Id: I33232c6b42ab4fcce85c2d0e0b0da145fc9232c3 BUG: 859927 Signed-off-by: Krutika Dhananjay Reviewed-on: http://review.gluster.org/4033 Reviewed-by: Pranith Kumar Karampuri Tested-by: Gluster Build System Reviewed-by: Anand Avati --- libglusterfs/src/common-utils.c | 20 ++++++++ libglusterfs/src/options.c | 111 ++++++++++++++++++++++++++-------------- tests/bugs/bug-859927.t | 70 +++++++++++++++++++++++++ 3 files changed, 163 insertions(+), 38 deletions(-) create mode 100755 tests/bugs/bug-859927.t diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c index 40a687953..c327d4fe4 100644 --- a/libglusterfs/src/common-utils.c +++ b/libglusterfs/src/common-utils.c @@ -666,6 +666,8 @@ gf_string2time (const char *str, uint32_t *n) old_errno = errno; errno = 0; value = strtol (str, &tail, 0); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -709,6 +711,8 @@ gf_string2percent (const char *str, double *n) old_errno = errno; errno = 0; value = strtod (str, &tail); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -742,6 +746,8 @@ _gf_string2long (const char *str, long *n, int base) old_errno = errno; errno = 0; value = strtol (str, &tail, base); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -782,6 +788,8 @@ _gf_string2ulong (const char *str, unsigned long *n, int base) old_errno = errno; errno = 0; value = strtoul (str, &tail, base); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -822,6 +830,8 @@ _gf_string2uint (const char *str, unsigned int *n, int base) old_errno = errno; errno = 0; value = strtoul (str, &tail, base); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -853,6 +863,8 @@ _gf_string2double (const char *str, double *n) old_errno = errno; errno = 0; value = strtod (str, &tail); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -884,6 +896,8 @@ _gf_string2longlong (const char *str, long long *n, int base) old_errno = errno; errno = 0; value = strtoll (str, &tail, base); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -924,6 +938,8 @@ _gf_string2ulonglong (const char *str, unsigned long long *n, int base) old_errno = errno; errno = 0; value = strtoull (str, &tail, base); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -1288,6 +1304,8 @@ gf_string2bytesize (const char *str, uint64_t *n) old_errno = errno; errno = 0; value = strtod (str, &tail); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; @@ -1344,6 +1362,8 @@ gf_string2percent_or_bytesize (const char *str, old_errno = errno; errno = 0; value = strtoull (str, &tail, 10); + if (str == tail) + errno = EINVAL; if (errno == ERANGE || errno == EINVAL) return -1; diff --git a/libglusterfs/src/options.c b/libglusterfs/src/options.c index f8ebb94c6..5d436bab3 100644 --- a/libglusterfs/src/options.c +++ b/libglusterfs/src/options.c @@ -228,6 +228,46 @@ out: return ret; } +void +set_error_str (char *errstr, size_t len, volume_option_t *opt, const char *key, + const char *value) +{ + int i = 0; + char given_array[4096] = {0,}; + + for (i = 0; (i < ZR_OPTION_MAX_ARRAY_SIZE) && opt->value[i];) { + strcat (given_array, opt->value[i]); + if (((++i) < ZR_OPTION_MAX_ARRAY_SIZE) && + (opt->value[i])) + strcat (given_array, ", "); + else + strcat (given_array, "."); + } + snprintf (errstr, len, "option %s %s: '%s' is not valid " + "(possible options are %s)", key, value, value, given_array); + return; +} + +int +is_all_whitespaces (const char *value) +{ + int i = 0; + size_t len = 0; + + if (value == NULL) + return -1; + + len = strlen (value); + + for (i = 0; i < len; i++) { + if (value[i] == ' ') + continue; + else + return 0; + } + + return 1; +} static int xlator_option_validate_str (xlator_t *xl, const char *key, const char *value, @@ -235,8 +275,7 @@ xlator_option_validate_str (xlator_t *xl, const char *key, const char *value, { int ret = -1; int i = 0; - char errstr[256]; - char given_array[4096] = {0,}; + char errstr[4096] = {0,}; /* Check if the '*str' is valid */ if (GF_OPTION_LIST_EMPTY(opt)) { @@ -244,6 +283,9 @@ xlator_option_validate_str (xlator_t *xl, const char *key, const char *value, goto out; } + if (is_all_whitespaces (value) == 1) + goto out; + for (i = 0; (i < ZR_OPTION_MAX_ARRAY_SIZE) && opt->value[i]; i++) { #ifdef GF_DARWIN_HOST_OS if (fnmatch (opt->value[i], value, 0) == 0) { @@ -258,8 +300,8 @@ xlator_option_validate_str (xlator_t *xl, const char *key, const char *value, #endif } - if (((i < ZR_OPTION_MAX_ARRAY_SIZE) && (!opt->value[i])) || - (i == ZR_OPTION_MAX_ARRAY_SIZE)) { + if ((i == ZR_OPTION_MAX_ARRAY_SIZE) || (!opt->value[i])) + goto out; /* enter here only if * 1. reached end of opt->value array and haven't * validated input @@ -269,26 +311,15 @@ xlator_option_validate_str (xlator_t *xl, const char *key, const char *value, * matched all possible input values. */ - for (i = 0; (i < ZR_OPTION_MAX_ARRAY_SIZE) && opt->value[i];) { - strcat (given_array, opt->value[i]); - if (((++i) < ZR_OPTION_MAX_ARRAY_SIZE) && - (opt->value[i])) - strcat (given_array, ", "); - else - strcat (given_array, "."); - } - snprintf (errstr, 256, - "option %s %s: '%s' is not valid " - "(possible options are %s)", - key, value, value, given_array); - gf_log (xl->name, GF_LOG_ERROR, "%s", errstr); - goto out; - } - ret = 0; + out: - if (ret && op_errstr) - *op_errstr = gf_strdup (errstr); + if (ret) { + set_error_str (errstr, sizeof (errstr), opt, key, value); + gf_log (xl->name, GF_LOG_ERROR, "%s", errstr); + if (op_errstr) + *op_errstr = gf_strdup (errstr); + } return ret; } @@ -513,32 +544,31 @@ xlator_option_validate_addr_list (xlator_t *xl, const char *key, char *dup_val = NULL; char *addr_tok = NULL; char *save_ptr = NULL; - char errstr[256]; + char errstr[4096] = {0,}; dup_val = gf_strdup (value); - if (!dup_val) { - ret = -1; - snprintf (errstr, 256, "internal error, out of memory."); + if (!dup_val) goto out; - } addr_tok = strtok_r (dup_val, ",", &save_ptr); + if (addr_tok == NULL) + goto out; while (addr_tok) { - if (!valid_internet_address (addr_tok, _gf_true)) { - snprintf (errstr, 256, - "option %s %s: '%s' is not a valid " - "internet-address-list", - key, value, value); - gf_log (xl->name, GF_LOG_ERROR, "%s", errstr); - ret = -1; + if (!valid_internet_address (addr_tok, _gf_true)) goto out; - } + addr_tok = strtok_r (NULL, ",", &save_ptr); } ret = 0; - out: - if (op_errstr && ret) - *op_errstr = gf_strdup (errstr); + +out: + if (ret) { + snprintf (errstr, sizeof (errstr), "option %s %s: '%s' is not " + "a valid internet-address-list", key, value, value); + gf_log (xl->name, GF_LOG_ERROR, "%s", errstr); + if (op_errstr) + *op_errstr = gf_strdup (errstr); + } GF_FREE (dup_val); return ret; @@ -595,6 +625,10 @@ validate_list_elements (const char *string, volume_option_t *opt, goto out; str_ptr = strtok_r (dup_string, ",", &str_sav); + if (str_ptr == NULL) { + ret = -1; + goto out; + } while (str_ptr) { key = strtok_r (str_ptr, ":", &substr_sav); @@ -620,6 +654,7 @@ validate_list_elements (const char *string, volume_option_t *opt, str_ptr = strtok_r (NULL, ",", &str_sav); substr_sav = NULL; } + out: GF_FREE (dup_string); gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret); diff --git a/tests/bugs/bug-859927.t b/tests/bugs/bug-859927.t new file mode 100755 index 000000000..ed74d3eb8 --- /dev/null +++ b/tests/bugs/bug-859927.t @@ -0,0 +1,70 @@ +#!/bin/bash + +. $(dirname $0)/../include.rc +. $(dirname $0)/../volume.rc +cleanup; + +glusterd; + +TEST $CLI volume create $V0 replica 2 stripe 2 $H0:$B0/${V0}{1,2,3,4,5,6,7,8}; + +TEST ! $CLI volume set $V0 statedump-path "" +TEST ! $CLI volume set $V0 statedump-path " " +TEST $CLI volume set $V0 statedump-path "/home/" +EXPECT "/home/" volume_option $V0 server.statedump-path + +TEST ! $CLI volume set $V0 background-self-heal-count "" +TEST ! $CLI volume set $V0 background-self-heal-count " " +TEST $CLI volume set $V0 background-self-heal-count 10 +EXPECT "10" volume_option $V0 cluster.background-self-heal-count + +TEST ! $CLI volume set $V0 cache-size "" +TEST ! $CLI volume set $V0 cache-size " " +TEST $CLI volume set $V0 cache-size 512MB +EXPECT "512MB" volume_option $V0 performance.cache-size + +TEST ! $CLI volume set $V0 self-heal-daemon "" +TEST ! $CLI volume set $V0 self-heal-daemon " " +TEST $CLI volume set $V0 self-heal-daemon on +EXPECT "on" volume_option $V0 cluster.self-heal-daemon + +TEST ! $CLI volume set $V0 read-subvolume "" +TEST ! $CLI volume set $V0 read-subvolume " " +TEST $CLI volume set $V0 read-subvolume $V0-client-0 +EXPECT "$V0-client-0" volume_option $V0 cluster.read-subvolume + +TEST ! $CLI volume set $V0 data-self-heal-algorithm "" +TEST ! $CLI volume set $V0 data-self-heal-algorithm " " +TEST ! $CLI volume set $V0 data-self-heal-algorithm on +TEST $CLI volume set $V0 data-self-heal-algorithm full +EXPECT "full" volume_option $V0 cluster.data-self-heal-algorithm + +TEST ! $CLI volume set $V0 min-free-inodes "" +TEST ! $CLI volume set $V0 min-free-inodes " " +TEST $CLI volume set $V0 min-free-inodes 60% +EXPECT "60%" volume_option $V0 cluster.min-free-inodes + +TEST ! $CLI volume set $V0 min-free-disk "" +TEST ! $CLI volume set $V0 min-free-disk " " +TEST $CLI volume set $V0 min-free-disk 60% +EXPECT "60%" volume_option $V0 cluster.min-free-disk + +TEST $CLI volume set $V0 min-free-disk 120 +EXPECT "120" volume_option $V0 cluster.min-free-disk + +TEST ! $CLI volume set $V0 frame-timeout "" +TEST ! $CLI volume set $V0 frame-timeout " " +TEST $CLI volume set $V0 frame-timeout 0 +EXPECT "0" volume_option $V0 network.frame-timeout + +TEST ! $CLI volume set $V0 auth.allow "" +TEST ! $CLI volume set $V0 auth.allow " " +TEST $CLI volume set $V0 auth.allow 192.168.122.1 +EXPECT "192.168.122.1" volume_option $V0 auth.allow + +TEST ! $CLI volume set $V0 stripe-block-size "" +TEST ! $CLI volume set $V0 stripe-block-size " " +TEST $CLI volume set $V0 stripe-block-size 512MB +EXPECT "512MB" volume_option $V0 cluster.stripe-block-size + +cleanup; -- cgit