From ac098c4a02d17654fee7f9c1aa1119dd9f875ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 9 Aug 2018 13:34:38 +0300 Subject: [PATCH] Check validity of size types The get_suffixed_size function is now exposed in the internal config header and it also checks for the validity of the size types. Took the new function into use and added the appropriate error messages. --- include/maxscale/config.h | 4 +- server/core/config.cc | 92 +++++++++++++++++++++++++--------- server/core/internal/config.hh | 28 ++++++++--- 3 files changed, 92 insertions(+), 32 deletions(-) diff --git a/include/maxscale/config.h b/include/maxscale/config.h index 0b5b01c13..b9dada22f 100644 --- a/include/maxscale/config.h +++ b/include/maxscale/config.h @@ -275,8 +275,8 @@ typedef struct bool substitute_variables; /**< Should environment variables be substituted */ char* local_address; /**< Local address to use when connecting */ time_t users_refresh_time; /**< How often the users can be refreshed */ - uint32_t writeq_high_water; /**< High water mark of dcb write queue */ - uint32_t writeq_low_water; /**< Low water mark of dcb write queue */ + uint64_t writeq_high_water; /**< High water mark of dcb write queue */ + uint64_t writeq_low_water; /**< Low water mark of dcb write queue */ } MXS_CONFIG; /** diff --git a/server/core/config.cc b/server/core/config.cc index 90e7da8d8..69a2dcf27 100644 --- a/server/core/config.cc +++ b/server/core/config.cc @@ -213,7 +213,6 @@ static void remove_first_last_char(char* value); static bool test_regex_string_validity(const char* regex_string, const char* key); static pcre2_code* compile_regex_string(const char* regex_string, bool jit_enabled, uint32_t options, uint32_t* output_ovector_size); -static uint64_t get_suffixed_size(const char* value); int config_get_ifaddr(unsigned char *output); static int config_get_release_string(char* release); @@ -1637,8 +1636,10 @@ int config_get_integer(const MXS_CONFIG_PARAMETER *params, const char *key) uint64_t config_get_size(const MXS_CONFIG_PARAMETER *params, const char *key) { const char *value = config_get_value_string(params, key); - - return get_suffixed_size(value); + uint64_t intval; + ss_debug(bool rval = )get_suffixed_size(value, &intval); + ss_dassert(rval); + return intval; } const char* config_get_string(const MXS_CONFIG_PARAMETER *params, const char *key) @@ -2048,7 +2049,11 @@ handle_global_item(const char *name, const char *value) } else if (strcmp(name, CN_THREAD_STACK_SIZE) == 0) { - gateway.thread_stack_size = get_suffixed_size(value); + if (!get_suffixed_size(value, &gateway.thread_stack_size)) + { + MXS_WARNING("Invalid value for '%s': %s.", CN_THREAD_STACK_SIZE, value); + return 0; + } } else if (strcmp(name, CN_NON_BLOCKING_POLLS) == 0) { @@ -2133,7 +2138,15 @@ handle_global_item(const char *name, const char *value) } else if (strcmp(name, CN_QUERY_CLASSIFIER_CACHE_SIZE) == 0) { - decltype(gateway.qc_cache_properties.max_size) max_size = get_suffixed_size(value); + uint64_t int_value; + + if (!get_suffixed_size(value, &int_value)) + { + MXS_ERROR("Invalid value for %s: %s", CN_QUERY_CLASSIFIER_CACHE_SIZE, value); + return 0; + } + + decltype(gateway.qc_cache_properties.max_size) max_size = int_value; if (max_size >= 0) { @@ -2141,7 +2154,7 @@ handle_global_item(const char *name, const char *value) } else { - MXS_ERROR("Invalid value for %s: %s", CN_QUERY_CLASSIFIER_CACHE_SIZE, value); + MXS_ERROR("Value too large for %s: %s", CN_QUERY_CLASSIFIER_CACHE_SIZE, value); return 0; } } @@ -2328,25 +2341,37 @@ handle_global_item(const char *name, const char *value) } else if (strcmp(name, CN_WRITEQ_HIGH_WATER) == 0) { - gateway.writeq_high_water = get_suffixed_size(value); + if (!get_suffixed_size(value, &gateway.writeq_high_water)) + { + MXS_ERROR("Invalid value for %s: %s", CN_WRITEQ_HIGH_WATER, value); + return 0; + } + if (gateway.writeq_high_water < MIN_WRITEQ_HIGH_WATER) { - MXS_WARNING("The specified writeq high water mark %d, is smaller than the minimum allowed size %d. Changing to minimum.", + MXS_WARNING("The specified writeq high water mark %lu, is smaller " + "than the minimum allowed size %lu. Changing to minimum.", gateway.writeq_high_water, MIN_WRITEQ_HIGH_WATER); gateway.writeq_high_water = MIN_WRITEQ_HIGH_WATER; } - MXS_NOTICE("Writeq high water mark set to: %d", gateway.writeq_high_water); + MXS_NOTICE("Writeq high water mark set to: %lu", gateway.writeq_high_water); } else if (strcmp(name, CN_WRITEQ_LOW_WATER) == 0) { - gateway.writeq_low_water = get_suffixed_size(value); + if (!get_suffixed_size(value, &gateway.writeq_high_water)) + { + MXS_ERROR("Invalid value for %s: %s", CN_WRITEQ_HIGH_WATER, value); + return 0; + } + if (gateway.writeq_low_water < MIN_WRITEQ_LOW_WATER) { - MXS_WARNING("The specified writeq low water mark %d, is smaller than the minimum allowed size %d. Changing to minimum.", + MXS_WARNING("The specified writeq low water mark %lu, is smaller " + "than the minimum allowed size %lu. Changing to minimum.", gateway.writeq_low_water, MIN_WRITEQ_LOW_WATER); gateway.writeq_low_water = MIN_WRITEQ_LOW_WATER; } - MXS_NOTICE("Writeq low water mark set to: %d", gateway.writeq_low_water); + MXS_NOTICE("Writeq low water mark set to: %lu", gateway.writeq_low_water); } else { @@ -4417,17 +4442,15 @@ static bool test_regex_string_validity(const char* regex_string, const char* key return rval; } -/** - * Converts a string into the corresponding value, interpreting - * IEC or SI prefixes used as suffixes appropriately. - * - * @param value A numerical string, possibly suffixed by a IEC - * binary prefix or SI prefix. - * - * @return The corresponding size. - */ -static uint64_t get_suffixed_size(const char* value) +bool get_suffixed_size(const char* value, uint64_t* dest) { + if (!isdigit(*value)) + { + // This will also catch negative values + return false; + } + + bool rval = false; char *end; uint64_t size = strtoll(value, &end, 10); @@ -4485,7 +4508,30 @@ static uint64_t get_suffixed_size(const char* value) break; } - return size; + const std::set first{ 'T', 't', 'G', 'g', 'M', 'm', 'K', 'k' }; + const std::set second{ 'I', 'i' }; + + if (end[0] == '\0') + { + rval = true; + } + else if (end[1] == '\0') + { + // First character must be valid + rval = first.count(end[0]); + } + else if (end[2] == '\0') + { + // Both characters have to be valid + rval = first.count(end[0]) && second.count(end[1]); + } + + if (dest) + { + *dest = size; + } + + return rval; } bool config_parse_disk_space_threshold(MxsDiskSpaceThreshold* pDisk_space_threshold, diff --git a/server/core/internal/config.hh b/server/core/internal/config.hh index 271967e64..e1934166d 100644 --- a/server/core/internal/config.hh +++ b/server/core/internal/config.hh @@ -21,13 +21,13 @@ #include #include -#define DEFAULT_NBPOLLS 3 /**< Default number of non block polls before we block */ -#define DEFAULT_POLLSLEEP 1000 /**< Default poll wait time (milliseconds) */ -#define DEFAULT_NTHREADS 1 /**< Default number of polling threads */ -#define DEFAULT_QUERY_RETRIES 1 /**< Number of retries for interrupted queries */ -#define DEFAULT_QUERY_RETRY_TIMEOUT 5 /**< Timeout for query retries */ -#define MIN_WRITEQ_HIGH_WATER 4096 /**< Min high water mark of dcb write queue */ -#define MIN_WRITEQ_LOW_WATER 512 /**< Min low water mark of dcb write queue */ +#define DEFAULT_NBPOLLS 3 /**< Default number of non block polls before we block */ +#define DEFAULT_POLLSLEEP 1000 /**< Default poll wait time (milliseconds) */ +#define DEFAULT_NTHREADS 1 /**< Default number of polling threads */ +#define DEFAULT_QUERY_RETRIES 1 /**< Number of retries for interrupted queries */ +#define DEFAULT_QUERY_RETRY_TIMEOUT 5 /**< Timeout for query retries */ +#define MIN_WRITEQ_HIGH_WATER 4096UL /**< Min high water mark of dcb write queue */ +#define MIN_WRITEQ_LOW_WATER 512UL /**< Min low water mark of dcb write queue */ /** * Maximum length for configuration parameter value. @@ -199,3 +199,17 @@ bool config_global_serialize(); bool export_config_file(const char* filename); bool is_normal_server_parameter(const char *param); + +/** + * Converts a string into the corresponding value, interpreting + * IEC or SI prefixes used as suffixes appropriately. + * + * @param value A numerical string, possibly suffixed by a IEC binary prefix or + * SI prefix. + * @param dest Pointer where the result is stored. If set to NULL, only the + * validity of value is checked. + * + * @return True on success, false on invalid input in which case contents of + * `dest` are left in an undefined state + */ +bool get_suffixed_size(const char* value, uint64_t* dest);