From 392d0227a6ea6afd1a62d1ab89fc2e5aa562b85d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Nov 2023 10:08:59 +0100 Subject: [PATCH 01/14] Move environment variables related functions/definitions into env.{c,h} without any further changes Signed-off-by: DL6ER --- src/config/CMakeLists.txt | 2 + src/config/env.c | 271 ++++++++++++++++++++++++++++++++++++++ src/config/env.h | 23 ++++ src/config/toml_helper.c | 257 ------------------------------------ src/config/toml_helper.h | 1 - src/config/toml_reader.c | 2 + 6 files changed, 298 insertions(+), 258 deletions(-) create mode 100644 src/config/env.c create mode 100644 src/config/env.h diff --git a/src/config/CMakeLists.txt b/src/config/CMakeLists.txt index 99c08375d..55572bc7c 100644 --- a/src/config/CMakeLists.txt +++ b/src/config/CMakeLists.txt @@ -15,6 +15,8 @@ set(sources config.h dnsmasq_config.c dnsmasq_config.h + env.c + env.h inotify.c inotify.h legacy_reader.c diff --git a/src/config/env.c b/src/config/env.c new file mode 100644 index 000000000..6ec7a007c --- /dev/null +++ b/src/config/env.c @@ -0,0 +1,271 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Environment-related routines +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#include "env.h" +#include "log.h" +#include "config/config.h" +// get_refresh_hostnames_str() +#include "datastructure.h" +//set_and_check_password() +#include "config/password.h" + +bool readEnvValue(struct conf_item *conf_item, struct config *newconf) +{ + // Allocate memory for config key + prefix (sizeof includes the trailing '\0') + const size_t envkey_size = strlen(conf_item->k) + sizeof(FTLCONF_PREFIX); + char *envkey = calloc(envkey_size, sizeof(char)); + + // Build env key to look for + strcpy(envkey, FTLCONF_PREFIX); + strcat(envkey, conf_item->k); + + // Replace all "." by "_" as this is the convention used in v5.x and earlier + for(unsigned int i = 0; i < envkey_size - 1; i++) + if(envkey[i] == '.') + envkey[i] = '_'; + + // First check if a environmental variable with the given key exists + const char *envvar = getenv(envkey); + + // Return early if this environment variable does not exist + if(envvar == NULL) + { + free(envkey); + return false; + } + + log_debug(DEBUG_CONFIG, "ENV %s = \"%s\"", envkey, envvar); + + switch(conf_item->t) + { + case CONF_BOOL: + { + if(strcasecmp(envvar, "true") == 0 || strcasecmp(envvar, "yes") == 0) + conf_item->v.b = true; + else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) + conf_item->v.b = false; + else + log_warn("ENV %s is not of type bool", envkey); + break; + } + case CONF_ALL_DEBUG_BOOL: + { + if(strcasecmp(envvar, "true") == 0 || strcasecmp(envvar, "yes") == 0) + set_all_debug(newconf, true); + else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) + set_all_debug(newconf, false); + else + log_warn("ENV %s is not of type bool", envkey); + break; + } + case CONF_INT: + { + int val = 0; + if(sscanf(envvar, "%i", &val) == 1) + conf_item->v.i = val; + else + log_warn("ENV %s is not of type integer", envkey); + break; + } + case CONF_UINT: + { + unsigned int val = 0; + if(sscanf(envvar, "%u", &val) == 1) + conf_item->v.ui = val; + else + log_warn("ENV %s is not of type unsigned integer", envkey); + break; + } + case CONF_UINT16: + { + unsigned int val = 0; + if(sscanf(envvar, "%u", &val) == 1 && val <= UINT16_MAX) + conf_item->v.ui = val; + else + log_warn("ENV %s is not of type unsigned integer (16 bit)", envkey); + break; + } + case CONF_LONG: + { + long val = 0; + if(sscanf(envvar, "%li", &val) == 1) + conf_item->v.l = val; + else + log_warn("ENV %s is not of type long", envkey); + break; + } + case CONF_ULONG: + { + unsigned long val = 0; + if(sscanf(envvar, "%lu", &val) == 1) + conf_item->v.ul = val; + else + log_warn("ENV %s is not of type unsigned long", envkey); + break; + } + case CONF_DOUBLE: + { + double val = 0; + if(sscanf(envvar, "%lf", &val) == 1) + conf_item->v.d = val; + else + log_warn("ENV %s is not of type double", envkey); + break; + } + case CONF_STRING: + case CONF_STRING_ALLOCATED: + { + if(conf_item->t == CONF_STRING_ALLOCATED) + free(conf_item->v.s); + conf_item->v.s = strdup(envvar); + conf_item->t = CONF_STRING_ALLOCATED; + break; + } + case CONF_ENUM_PTR_TYPE: + { + const int ptr_type = get_ptr_type_val(envvar); + if(ptr_type != -1) + conf_item->v.ptr_type = ptr_type; + else + log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + break; + } + case CONF_ENUM_BUSY_TYPE: + { + const int busy_reply = get_busy_reply_val(envvar); + if(busy_reply != -1) + conf_item->v.busy_reply = busy_reply; + else + log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + break; + } + case CONF_ENUM_BLOCKING_MODE: + { + const int blocking_mode = get_blocking_mode_val(envvar); + if(blocking_mode != -1) + conf_item->v.blocking_mode = blocking_mode; + else + log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + break; + } + case CONF_ENUM_REFRESH_HOSTNAMES: + { + const int refresh_hostnames = get_refresh_hostnames_val(envvar); + if(refresh_hostnames != -1) + conf_item->v.refresh_hostnames = refresh_hostnames; + else + log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + break; + } + case CONF_ENUM_LISTENING_MODE: + { + const int listeningMode = get_listeningMode_val(envvar); + if(listeningMode != -1) + conf_item->v.listeningMode = listeningMode; + else + log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + break; + } + case CONF_ENUM_WEB_THEME: + { + const int web_theme = get_web_theme_val(envvar); + if(web_theme != -1) + conf_item->v.web_theme = web_theme; + else + log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + break; + } + case CONF_ENUM_TEMP_UNIT: + { + const int temp_unit = get_temp_unit_val(envvar); + if(temp_unit != -1) + conf_item->v.temp_unit = temp_unit; + else + log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + break; + } + case CONF_ENUM_PRIVACY_LEVEL: + { + int val = 0; + if(sscanf(envvar, "%i", &val) == 1 && val >= PRIVACY_SHOW_ALL && val <= PRIVACY_MAXIMUM) + conf_item->v.i = val; + else + log_warn("ENV %s is invalid (not of type integer or outside allowed bounds)", envkey); + break; + } + case CONF_STRUCT_IN_ADDR: + { + struct in_addr addr4 = { 0 }; + if(strlen(envvar) == 0) + { + // Special case: empty string -> 0.0.0.0 + conf_item->v.in_addr.s_addr = INADDR_ANY; + } + else if(inet_pton(AF_INET, envvar, &addr4)) + memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); + else + log_warn("ENV %s is invalid (not of type IPv4 address)", envkey); + break; + } + case CONF_STRUCT_IN6_ADDR: + { + struct in6_addr addr6 = { 0 }; + if(strlen(envvar) == 0) + { + // Special case: empty string -> :: + memcpy(&conf_item->v.in6_addr, &in6addr_any, sizeof(in6addr_any)); + } + else if(inet_pton(AF_INET6, envvar, &addr6)) + memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); + else + log_warn("ENV %s is invalid (not of type IPv6 address)", envkey); + break; + } + case CONF_JSON_STRING_ARRAY: + { + // Make a copy of envvar as strtok modified the input string + char *envvar_copy = strdup(envvar); + // Free previously allocated JSON array + cJSON_Delete(conf_item->v.json); + conf_item->v.json = cJSON_CreateArray(); + // Parse envvar array and generate a JSON array (env var + // arrays are ;-delimited) + const char delim[] =";"; + const char *elem = strtok(envvar_copy, delim); + while(elem != NULL) + { + // Only import non-empty entries + if(strlen(elem) > 0) + { + // Add string to our JSON array + cJSON *item = cJSON_CreateString(elem); + cJSON_AddItemToArray(conf_item->v.json, item); + } + + // Search for the next element + elem = strtok(NULL, delim); + } + free(envvar_copy); + break; + } + case CONF_PASSWORD: + { + if(!set_and_check_password(conf_item, envvar)) + { + log_warn("ENV %s is invalid", envkey); + break; + } + } + } + + // Free allocated env var name + free(envkey); + return true; +} diff --git a/src/config/env.h b/src/config/env.h new file mode 100644 index 000000000..29fdf5ddf --- /dev/null +++ b/src/config/env.h @@ -0,0 +1,23 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Environment-related prototypes +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ +#ifndef CONFIG_ENV_H +#define CONFIG_ENV_H + +#include "FTL.h" +// union conf_value +#include "config.h" +// type toml_table_t +#include "tomlc99/toml.h" + +#define FTLCONF_PREFIX "FTLCONF_" + +bool readEnvValue(struct conf_item *conf_item, struct config *newconf); + +#endif //CONFIG_ENV_H diff --git a/src/config/toml_helper.c b/src/config/toml_helper.c index 49c1da520..ab866f487 100644 --- a/src/config/toml_helper.c +++ b/src/config/toml_helper.c @@ -8,7 +8,6 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -#include "FTL.h" #include "toml_helper.h" #include "log.h" #include "config/config.h" @@ -809,259 +808,3 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t } } } - -#define FTLCONF_PREFIX "FTLCONF_" -bool readEnvValue(struct conf_item *conf_item, struct config *newconf) -{ - // Allocate memory for config key + prefix (sizeof includes the trailing '\0') - const size_t envkey_size = strlen(conf_item->k) + sizeof(FTLCONF_PREFIX); - char *envkey = calloc(envkey_size, sizeof(char)); - - // Build env key to look for - strcpy(envkey, FTLCONF_PREFIX); - strcat(envkey, conf_item->k); - - // Replace all "." by "_" as this is the convention used in v5.x and earlier - for(unsigned int i = 0; i < envkey_size - 1; i++) - if(envkey[i] == '.') - envkey[i] = '_'; - - // First check if a environmental variable with the given key exists - const char *envvar = getenv(envkey); - - // Return early if this environment variable does not exist - if(envvar == NULL) - { - log_debug(DEBUG_CONFIG, "ENV %s is not set", envkey); - free(envkey); - return false; - } - - log_debug(DEBUG_CONFIG, "ENV %s = \"%s\"", envkey, envvar); - - switch(conf_item->t) - { - case CONF_BOOL: - { - if(strcasecmp(envvar, "true") == 0 || strcasecmp(envvar, "yes") == 0) - conf_item->v.b = true; - else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) - conf_item->v.b = false; - else - log_warn("ENV %s is not of type bool", envkey); - break; - } - case CONF_ALL_DEBUG_BOOL: - { - if(strcasecmp(envvar, "true") == 0 || strcasecmp(envvar, "yes") == 0) - set_all_debug(newconf, true); - else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) - set_all_debug(newconf, false); - else - log_warn("ENV %s is not of type bool", envkey); - break; - } - case CONF_INT: - { - int val = 0; - if(sscanf(envvar, "%i", &val) == 1) - conf_item->v.i = val; - else - log_warn("ENV %s is not of type integer", envkey); - break; - } - case CONF_UINT: - { - unsigned int val = 0; - if(sscanf(envvar, "%u", &val) == 1) - conf_item->v.ui = val; - else - log_warn("ENV %s is not of type unsigned integer", envkey); - break; - } - case CONF_UINT16: - { - unsigned int val = 0; - if(sscanf(envvar, "%u", &val) == 1 && val <= UINT16_MAX) - conf_item->v.ui = val; - else - log_warn("ENV %s is not of type unsigned integer (16 bit)", envkey); - break; - } - case CONF_LONG: - { - long val = 0; - if(sscanf(envvar, "%li", &val) == 1) - conf_item->v.l = val; - else - log_warn("ENV %s is not of type long", envkey); - break; - } - case CONF_ULONG: - { - unsigned long val = 0; - if(sscanf(envvar, "%lu", &val) == 1) - conf_item->v.ul = val; - else - log_warn("ENV %s is not of type unsigned long", envkey); - break; - } - case CONF_DOUBLE: - { - double val = 0; - if(sscanf(envvar, "%lf", &val) == 1) - conf_item->v.d = val; - else - log_warn("ENV %s is not of type double", envkey); - break; - } - case CONF_STRING: - case CONF_STRING_ALLOCATED: - { - if(conf_item->t == CONF_STRING_ALLOCATED) - free(conf_item->v.s); - conf_item->v.s = strdup(envvar); - conf_item->t = CONF_STRING_ALLOCATED; - break; - } - case CONF_ENUM_PTR_TYPE: - { - const int ptr_type = get_ptr_type_val(envvar); - if(ptr_type != -1) - conf_item->v.ptr_type = ptr_type; - else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); - break; - } - case CONF_ENUM_BUSY_TYPE: - { - const int busy_reply = get_busy_reply_val(envvar); - if(busy_reply != -1) - conf_item->v.busy_reply = busy_reply; - else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); - break; - } - case CONF_ENUM_BLOCKING_MODE: - { - const int blocking_mode = get_blocking_mode_val(envvar); - if(blocking_mode != -1) - conf_item->v.blocking_mode = blocking_mode; - else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); - break; - } - case CONF_ENUM_REFRESH_HOSTNAMES: - { - const int refresh_hostnames = get_refresh_hostnames_val(envvar); - if(refresh_hostnames != -1) - conf_item->v.refresh_hostnames = refresh_hostnames; - else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); - break; - } - case CONF_ENUM_LISTENING_MODE: - { - const int listeningMode = get_listeningMode_val(envvar); - if(listeningMode != -1) - conf_item->v.listeningMode = listeningMode; - else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); - break; - } - case CONF_ENUM_WEB_THEME: - { - const int web_theme = get_web_theme_val(envvar); - if(web_theme != -1) - conf_item->v.web_theme = web_theme; - else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); - break; - } - case CONF_ENUM_TEMP_UNIT: - { - const int temp_unit = get_temp_unit_val(envvar); - if(temp_unit != -1) - conf_item->v.temp_unit = temp_unit; - else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); - break; - } - case CONF_ENUM_PRIVACY_LEVEL: - { - int val = 0; - if(sscanf(envvar, "%i", &val) == 1 && val >= PRIVACY_SHOW_ALL && val <= PRIVACY_MAXIMUM) - conf_item->v.i = val; - else - log_warn("ENV %s is invalid (not of type integer or outside allowed bounds)", envkey); - break; - } - case CONF_STRUCT_IN_ADDR: - { - struct in_addr addr4 = { 0 }; - if(strlen(envvar) == 0) - { - // Special case: empty string -> 0.0.0.0 - conf_item->v.in_addr.s_addr = INADDR_ANY; - } - else if(inet_pton(AF_INET, envvar, &addr4)) - memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); - else - log_warn("ENV %s is invalid (not of type IPv4 address)", envkey); - break; - } - case CONF_STRUCT_IN6_ADDR: - { - struct in6_addr addr6 = { 0 }; - if(strlen(envvar) == 0) - { - // Special case: empty string -> :: - memcpy(&conf_item->v.in6_addr, &in6addr_any, sizeof(in6addr_any)); - } - else if(inet_pton(AF_INET6, envvar, &addr6)) - memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); - else - log_warn("ENV %s is invalid (not of type IPv6 address)", envkey); - break; - } - case CONF_JSON_STRING_ARRAY: - { - // Make a copy of envvar as strtok modified the input string - char *envvar_copy = strdup(envvar); - // Free previously allocated JSON array - cJSON_Delete(conf_item->v.json); - conf_item->v.json = cJSON_CreateArray(); - // Parse envvar array and generate a JSON array (env var - // arrays are ;-delimited) - const char delim[] =";"; - const char *elem = strtok(envvar_copy, delim); - while(elem != NULL) - { - // Only import non-empty entries - if(strlen(elem) > 0) - { - // Add string to our JSON array - cJSON *item = cJSON_CreateString(elem); - cJSON_AddItemToArray(conf_item->v.json, item); - } - - // Search for the next element - elem = strtok(NULL, delim); - } - free(envvar_copy); - break; - } - case CONF_PASSWORD: - { - if(!set_and_check_password(conf_item, envvar)) - { - log_warn("ENV %s is invalid", envkey); - break; - } - } - } - - // Free allocated env var name - free(envkey); - return true; -} diff --git a/src/config/toml_helper.h b/src/config/toml_helper.h index db0d259a0..70f4844c6 100644 --- a/src/config/toml_helper.h +++ b/src/config/toml_helper.h @@ -23,6 +23,5 @@ void print_comment(FILE *fp, const char *str, const char *intro, const unsigned void print_toml_allowed_values(cJSON *allowed_values, FILE *fp, const unsigned int width, const unsigned int indent); void writeTOMLvalue(FILE * fp, const int indent, const enum conf_type t, union conf_value *v); void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *toml, struct config *newconf); -bool readEnvValue(struct conf_item *conf_item, struct config *newconf); #endif //CONFIG_WRITER_H diff --git a/src/config/toml_reader.c b/src/config/toml_reader.c index 7bad6af9a..235f9b89b 100644 --- a/src/config/toml_reader.c +++ b/src/config/toml_reader.c @@ -23,6 +23,8 @@ #include "config/toml_helper.h" // delete_all_sessions() #include "api/api.h" +// readEnvValue() +#include "config/env.h" // Private prototypes static toml_table_t *parseTOML(const unsigned int version); From 58249a76fbe012d4fc90187f8a42c6c164837b45 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Nov 2023 10:09:48 +0100 Subject: [PATCH 02/14] Store a list of FTLCONF_ environment variables Signed-off-by: DL6ER --- src/config/config.c | 7 ++++- src/config/env.c | 74 +++++++++++++++++++++++++++++++++++++++++++-- src/config/env.h | 2 ++ src/daemon.c | 5 +++ 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/src/config/config.c b/src/config/config.c index 58f2c40b6..0c84b5f01 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -29,6 +29,8 @@ #include "api/api.h" // exit_code #include "signals.h" +// getEnvVars() +#include "config/env.h" struct config config = { 0 }; static bool config_initialized = false; @@ -1372,7 +1374,10 @@ bool readFTLconf(struct config *conf, const bool rewrite) // Initialize config with default values initConfig(conf); - // First try to read TOML config file + // First, read the environment + getEnvVars(); + + // Try to read TOML config file // If we cannot parse /etc/pihole.toml (due to missing or invalid syntax), // we try to read the rotated files in /etc/pihole/config_backup starting at // the most recent one and going back in time until we find a valid config diff --git a/src/config/env.c b/src/config/env.c index 6ec7a007c..c4edb9704 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -16,6 +16,73 @@ //set_and_check_password() #include "config/password.h" +struct env_item +{ + bool used; + char *key; + char *value; + struct env_item *next; +}; + +static struct env_item *env_list = NULL; + +void getEnvVars(void) +{ + // Read environment variables only once + if(env_list != NULL) + return; + + // Get all environment variables + for(char **env = environ; *env != NULL; env++) + { + // Check if this is a FTLCONF_ variable + if(strncmp(*env, FTLCONF_PREFIX, sizeof(FTLCONF_PREFIX) - 1) == 0) + { + // Split key and value + char *key = strtok(*env, "="); + char *value = strtok(NULL, "="); + + // Add to list + struct env_item *new_item = calloc(1, sizeof(struct env_item)); + new_item->used = false; + new_item->key = strdup(key); + new_item->value = strdup(value); + new_item->next = env_list; + env_list = new_item; + } + } +} + +static char *getFTLenv(const char *key) +{ + // Iterate over all known FTLCONF environment variables + for(struct env_item *item = env_list; item != NULL; item = item->next) + { + // Check if this is the requested key + if(strcmp(item->key, key) == 0) + { + item->used = true; + return item->value; + } + } + + // Return NULL if the key was not found + return NULL; +} + +void freeEnvVars(void) +{ + // Free all environment variables + while(env_list != NULL) + { + struct env_item *next = env_list->next; + free(env_list->key); + free(env_list->value); + free(env_list); + env_list = next; + } +} + bool readEnvValue(struct conf_item *conf_item, struct config *newconf) { // Allocate memory for config key + prefix (sizeof includes the trailing '\0') @@ -31,8 +98,9 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(envkey[i] == '.') envkey[i] = '_'; - // First check if a environmental variable with the given key exists - const char *envvar = getenv(envkey); + // First check if a environmental variable with the given key exists by + // iterating over the list of FTLCONF_ variables + char *envvar = getFTLenv(envkey); // Return early if this environment variable does not exist if(envvar == NULL) @@ -41,7 +109,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) return false; } - log_debug(DEBUG_CONFIG, "ENV %s = \"%s\"", envkey, envvar); + log_debug(DEBUG_CONFIG, "ENV %s = %s", envkey, envvar); switch(conf_item->t) { diff --git a/src/config/env.h b/src/config/env.h index 29fdf5ddf..51399aca0 100644 --- a/src/config/env.h +++ b/src/config/env.h @@ -18,6 +18,8 @@ #define FTLCONF_PREFIX "FTLCONF_" +void getEnvVars(void); +void freeEnvVars(void); bool readEnvValue(struct conf_item *conf_item, struct config *newconf); #endif //CONFIG_ENV_H diff --git a/src/daemon.c b/src/daemon.c index 60b659265..38e93c0c1 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -37,6 +37,8 @@ #include "api/api.h" // setlocale() #include +// freeEnvVars() +#include "config/env.h" pthread_t threads[THREADS_MAX] = { 0 }; bool resolver_ready = false; @@ -351,6 +353,9 @@ void cleanup(const int ret) // This should be the last action when c destroy_shmem(); + // Free environment variables + freeEnvVars(); + char buffer[42] = { 0 }; format_time(buffer, 0, timer_elapsed_msec(EXIT_TIMER)); log_info("########## FTL terminated after%s (code %i)! ##########", buffer, ret); From d74c4ad3028e6afedf860d46aa79c8a91c88e01b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 31 Oct 2023 16:50:15 +0100 Subject: [PATCH 03/14] Log unused FTLCONF env vars Signed-off-by: DL6ER --- src/config/config.c | 25 +++++++---- src/config/config.h | 1 + src/config/env.c | 94 +++++++++++++++++++++++----------------- src/config/env.h | 1 + src/config/toml_reader.c | 3 ++ 5 files changed, 76 insertions(+), 48 deletions(-) diff --git a/src/config/config.c b/src/config/config.c index 0c84b5f01..f8bb2f223 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -1311,17 +1311,24 @@ void initConfig(struct config *conf) // Parse and split paths conf_item->p = gen_config_path(conf_item->k, '.'); - // Verify all config options are defined above - if(!conf_item->p || !conf_item->k || !conf_item->h) - { - log_err("Config option %u/%u is not set!", i, (unsigned int)CONFIG_ELEMENTS); - continue; - } + // Initialize environment variable name + // Allocate memory for config key + prefix (sizeof includes the trailing '\0') + const size_t envkey_size = strlen(conf_item->k) + sizeof(FTLCONF_PREFIX); + conf_item->e = calloc(envkey_size, sizeof(char)); - // Verify that all config options have a type - if(conf_item->t == 0) + // Build env key to look for + strcpy(conf_item->e, FTLCONF_PREFIX); + strcat(conf_item->e, conf_item->k); + + // Replace all "." by "_" as this is the convention used in v5.x and earlier + for(unsigned int j = 0; j < envkey_size - 1; j++) + if(conf_item->e[j] == '.') + conf_item->e[j] = '_'; + + // Verify all config options are defined above + if(!conf_item->p || !conf_item->k || !conf_item->h || !conf_item->e || conf_item->t == 0) { - log_err("Config option %s has no type!", conf_item->k); + log_err("Config option %u/%u is not fully configured!", i, (unsigned int)CONFIG_ELEMENTS); continue; } diff --git a/src/config/config.h b/src/config/config.h index cdacdbe38..48f08392f 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -98,6 +98,7 @@ enum conf_type { struct conf_item { const char *k; // item Key char **p; // item Path + char *e; // item Environment variable const char *h; // Help text / description cJSON *a; // JSON array or object of Allowed values (where applicable) enum conf_type t; // variable Type diff --git a/src/config/env.c b/src/config/env.c index c4edb9704..bbdae658a 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -15,6 +15,8 @@ #include "datastructure.h" //set_and_check_password() #include "config/password.h" +// cli_tick() +#include "args.h" struct env_item { @@ -53,6 +55,38 @@ void getEnvVars(void) } } +void printFTLenv(void) +{ + // Nothing to print if no env vars are used + if(env_list == NULL) + return; + + // Count number of used and unused env vars + unsigned int used = 0, unused = 0; + for(struct env_item *item = env_list; item != NULL; item = item->next) + { + if(item->used) + used++; + else + unused++; + } + + log_info("%u FTLCONF environment variable%s found (%u used, %u unused)", + used + unused, used + unused == 1 ? "" : "s", used, unused); + + // Iterate over all known FTLCONF environment variables + for(struct env_item *item = env_list; item != NULL; item = item->next) + { + if(item->used) + { + log_info("%s %s", cli_tick(), item->key); + continue; + } + // else: print warning + log_warn("%s %s is unknown", cli_cross(), item->key); + } +} + static char *getFTLenv(const char *key) { // Iterate over all known FTLCONF environment variables @@ -85,31 +119,15 @@ void freeEnvVars(void) bool readEnvValue(struct conf_item *conf_item, struct config *newconf) { - // Allocate memory for config key + prefix (sizeof includes the trailing '\0') - const size_t envkey_size = strlen(conf_item->k) + sizeof(FTLCONF_PREFIX); - char *envkey = calloc(envkey_size, sizeof(char)); - - // Build env key to look for - strcpy(envkey, FTLCONF_PREFIX); - strcat(envkey, conf_item->k); - - // Replace all "." by "_" as this is the convention used in v5.x and earlier - for(unsigned int i = 0; i < envkey_size - 1; i++) - if(envkey[i] == '.') - envkey[i] = '_'; - // First check if a environmental variable with the given key exists by // iterating over the list of FTLCONF_ variables - char *envvar = getFTLenv(envkey); + char *envvar = getFTLenv(conf_item->e); // Return early if this environment variable does not exist if(envvar == NULL) - { - free(envkey); return false; - } - log_debug(DEBUG_CONFIG, "ENV %s = %s", envkey, envvar); + log_debug(DEBUG_CONFIG, "ENV %s = %s", conf_item->e, envvar); switch(conf_item->t) { @@ -120,7 +138,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) conf_item->v.b = false; else - log_warn("ENV %s is not of type bool", envkey); + log_warn("ENV %s is not of type bool", conf_item->e); break; } case CONF_ALL_DEBUG_BOOL: @@ -130,7 +148,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) set_all_debug(newconf, false); else - log_warn("ENV %s is not of type bool", envkey); + log_warn("ENV %s is not of type bool", conf_item->e); break; } case CONF_INT: @@ -139,7 +157,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(sscanf(envvar, "%i", &val) == 1) conf_item->v.i = val; else - log_warn("ENV %s is not of type integer", envkey); + log_warn("ENV %s is not of type integer", conf_item->e); break; } case CONF_UINT: @@ -148,7 +166,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(sscanf(envvar, "%u", &val) == 1) conf_item->v.ui = val; else - log_warn("ENV %s is not of type unsigned integer", envkey); + log_warn("ENV %s is not of type unsigned integer", conf_item->e); break; } case CONF_UINT16: @@ -157,7 +175,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(sscanf(envvar, "%u", &val) == 1 && val <= UINT16_MAX) conf_item->v.ui = val; else - log_warn("ENV %s is not of type unsigned integer (16 bit)", envkey); + log_warn("ENV %s is not of type unsigned integer (16 bit)", conf_item->e); break; } case CONF_LONG: @@ -166,7 +184,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(sscanf(envvar, "%li", &val) == 1) conf_item->v.l = val; else - log_warn("ENV %s is not of type long", envkey); + log_warn("ENV %s is not of type long", conf_item->e); break; } case CONF_ULONG: @@ -175,7 +193,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(sscanf(envvar, "%lu", &val) == 1) conf_item->v.ul = val; else - log_warn("ENV %s is not of type unsigned long", envkey); + log_warn("ENV %s is not of type unsigned long", conf_item->e); break; } case CONF_DOUBLE: @@ -184,7 +202,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(sscanf(envvar, "%lf", &val) == 1) conf_item->v.d = val; else - log_warn("ENV %s is not of type double", envkey); + log_warn("ENV %s is not of type double", conf_item->e); break; } case CONF_STRING: @@ -202,7 +220,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(ptr_type != -1) conf_item->v.ptr_type = ptr_type; else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); break; } case CONF_ENUM_BUSY_TYPE: @@ -211,7 +229,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(busy_reply != -1) conf_item->v.busy_reply = busy_reply; else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); break; } case CONF_ENUM_BLOCKING_MODE: @@ -220,7 +238,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(blocking_mode != -1) conf_item->v.blocking_mode = blocking_mode; else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); break; } case CONF_ENUM_REFRESH_HOSTNAMES: @@ -229,7 +247,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(refresh_hostnames != -1) conf_item->v.refresh_hostnames = refresh_hostnames; else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); break; } case CONF_ENUM_LISTENING_MODE: @@ -238,7 +256,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(listeningMode != -1) conf_item->v.listeningMode = listeningMode; else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); break; } case CONF_ENUM_WEB_THEME: @@ -247,7 +265,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(web_theme != -1) conf_item->v.web_theme = web_theme; else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); break; } case CONF_ENUM_TEMP_UNIT: @@ -256,7 +274,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(temp_unit != -1) conf_item->v.temp_unit = temp_unit; else - log_warn("ENV %s is invalid, allowed options are: %s", envkey, conf_item->h); + log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); break; } case CONF_ENUM_PRIVACY_LEVEL: @@ -265,7 +283,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(sscanf(envvar, "%i", &val) == 1 && val >= PRIVACY_SHOW_ALL && val <= PRIVACY_MAXIMUM) conf_item->v.i = val; else - log_warn("ENV %s is invalid (not of type integer or outside allowed bounds)", envkey); + log_warn("ENV %s is invalid (not of type integer or outside allowed bounds)", conf_item->e); break; } case CONF_STRUCT_IN_ADDR: @@ -279,7 +297,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) else if(inet_pton(AF_INET, envvar, &addr4)) memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); else - log_warn("ENV %s is invalid (not of type IPv4 address)", envkey); + log_warn("ENV %s is invalid (not of type IPv4 address)", conf_item->e); break; } case CONF_STRUCT_IN6_ADDR: @@ -293,7 +311,7 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) else if(inet_pton(AF_INET6, envvar, &addr6)) memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); else - log_warn("ENV %s is invalid (not of type IPv6 address)", envkey); + log_warn("ENV %s is invalid (not of type IPv6 address)", conf_item->e); break; } case CONF_JSON_STRING_ARRAY: @@ -327,13 +345,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) { if(!set_and_check_password(conf_item, envvar)) { - log_warn("ENV %s is invalid", envkey); + log_warn("ENV %s is invalid", conf_item->e); break; } } } - // Free allocated env var name - free(envkey); return true; } diff --git a/src/config/env.h b/src/config/env.h index 51399aca0..0b4c9d80a 100644 --- a/src/config/env.h +++ b/src/config/env.h @@ -20,6 +20,7 @@ void getEnvVars(void); void freeEnvVars(void); +void printFTLenv(void); bool readEnvValue(struct conf_item *conf_item, struct config *newconf); #endif //CONFIG_ENV_H diff --git a/src/config/toml_reader.c b/src/config/toml_reader.c index 235f9b89b..28d956e5c 100644 --- a/src/config/toml_reader.c +++ b/src/config/toml_reader.c @@ -131,6 +131,9 @@ bool readFTLtoml(struct config *oldconf, struct config *newconf, if(verbose) reportDebugFlags(); + // Print FTL environment variables (if used) + printFTLenv(); + // Free memory allocated by the TOML parser and return success toml_free(toml); return true; From d55b1b8d934f750d661219c927c47df362740137 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 31 Oct 2023 19:04:42 +0100 Subject: [PATCH 04/14] Suggest closest env var key when we find unknown keys Signed-off-by: DL6ER --- src/config/CMakeLists.txt | 2 + src/config/env.c | 25 +++++--- src/config/levenshtein.c | 126 ++++++++++++++++++++++++++++++++++++++ src/config/levenshtein.h | 19 ++++++ 4 files changed, 165 insertions(+), 7 deletions(-) create mode 100644 src/config/levenshtein.c create mode 100644 src/config/levenshtein.h diff --git a/src/config/CMakeLists.txt b/src/config/CMakeLists.txt index 55572bc7c..15f91874b 100644 --- a/src/config/CMakeLists.txt +++ b/src/config/CMakeLists.txt @@ -21,6 +21,8 @@ set(sources inotify.h legacy_reader.c legacy_reader.h + levenshtein.c + levenshtein.h password.c password.h toml_writer.c diff --git a/src/config/env.c b/src/config/env.c index bbdae658a..3ee3dcd06 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -17,7 +17,8 @@ #include "config/password.h" // cli_tick() #include "args.h" - +// suggest_closest() +#include "config/levenshtein.h" struct env_item { bool used; @@ -27,6 +28,7 @@ struct env_item }; static struct env_item *env_list = NULL; +static const char *env_keys[sizeof(config) / sizeof(struct conf_item)] = { NULL }; void getEnvVars(void) { @@ -53,6 +55,14 @@ void getEnvVars(void) env_list = new_item; } } + + // Add all config item env keys to the list + for(unsigned int i = 0; i < sizeof(env_keys) / sizeof(*env_keys); i++) + { + struct conf_item *conf_item = get_conf_item(&config, i); + env_keys[i] = conf_item->e; + } + } void printFTLenv(void) @@ -61,18 +71,18 @@ void printFTLenv(void) if(env_list == NULL) return; - // Count number of used and unused env vars - unsigned int used = 0, unused = 0; + // Count number of used and ignored env vars + unsigned int used = 0, ignored = 0; for(struct env_item *item = env_list; item != NULL; item = item->next) { if(item->used) used++; else - unused++; + ignored++; } - log_info("%u FTLCONF environment variable%s found (%u used, %u unused)", - used + unused, used + unused == 1 ? "" : "s", used, unused); + log_info("%u FTLCONF environment variable%s found (%u used, %u ignored)", + used + ignored, used + ignored == 1 ? "" : "s", used, ignored); // Iterate over all known FTLCONF environment variables for(struct env_item *item = env_list; item != NULL; item = item->next) @@ -83,7 +93,8 @@ void printFTLenv(void) continue; } // else: print warning - log_warn("%s %s is unknown", cli_cross(), item->key); + const char *closest_match = suggest_closest(env_keys, sizeof(env_keys) / sizeof(*env_keys), item->key); + log_warn("%s %s is unknown, did you mean %s ?", cli_cross(), item->key, closest_match); } } diff --git a/src/config/levenshtein.c b/src/config/levenshtein.c new file mode 100644 index 000000000..d135638c4 --- /dev/null +++ b/src/config/levenshtein.c @@ -0,0 +1,126 @@ + +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Levenshtein distance routines +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#include "config/levenshtein.h" + +// Returns the minimum of three size_t values +static size_t min3(size_t x, size_t y, size_t z) +{ + const size_t tmp = x < y ? x : y; + return tmp < z ? tmp : z; +} + +// Simply swaps two size_t pointers in memory +static void swap(size_t **a, size_t **b) +{ + size_t *tmp = *a; + *a = *b; + *b = tmp; +} + +// The Levenshtein distance is a string metric for measuring the difference +// between two sequences. Informally, the Levenshtein distance between two words +// is the minimum number of single-character edits (insertions, deletions or +// substitutions) required to change one word into the other. It is named after +// the Soviet mathematician Vladimir Levenshtein, who considered this distance +// in 1965. (Wikipedia) +// +// For example, the Levenshtein distance between "kitten" and "sitting" is 3, +// since the following 3 edits change one into the other, and there is no way to +// do it with fewer than 3 edits: +// kitten -> sitten (substitution of "s" for "k"), +// sitten -> sittin (substitution of "i" for "e"), +// sittin -> sitting (insertion of "g" at the end). +// +// Our implementation follows the algorithm described in Wikipedia but was +// inspired by https://stackoverflow.com/a/71810739/2087442 +static size_t levenshtein_distance(const char *s1, const size_t len1, const char *s2, const size_t len2) +{ + // Allocate two vectors of size len2 + 1 + size_t *v0 = calloc(len2 + 1, sizeof(size_t)); + size_t *v1 = calloc(len2 + 1, sizeof(size_t)); + + // Initialize v0 + // v0[i] = the Levenshtein distance between s1[0..i] and the empty string + // v0[i] = i + for (size_t j = 0; j <= len2; ++j) + v0[j] = j; + + // Calculate v1 + // v1[i] = the Levenshtein distance between s1[0..i] and s2[0..j] + // v1[i] = min(v0[j] + 1, v1[j - 1] + 1, v0[j - 1] + (s1[i] == s2[j] ? 0 : 1)) + for (size_t i = 0; i < len1; ++i) + { + // Initialize v1 + v1[0] = i + 1; + + // Loop over remaining columns + for (size_t j = 0; j < len2; ++j) + { + // Calculate deletion, insertion and substitution costs + const size_t delcost = v0[j + 1] + 1; + const size_t inscost = v1[j] + 1; + const size_t subcost = s1[i] == s2[j] ? v0[j] : v0[j] + 1; + + // Take the minimum of the three costs (see above) + v1[j + 1] = min3(delcost, inscost, subcost); + } + + // Swap addresses to avoid copying data around + swap(&v0, &v1); + } + + // Return the Levenshtein distance between s1 and s2 + size_t dist = v0[len2]; + free(v0); + free(v1); + return dist; +} + +// Returns the the closest matching string +const char *__attribute__((pure)) suggest_closest(const char *strings[], size_t nstrings, const char *string) +{ + size_t mindist = 0; + ssize_t minidx = -1; + + // The Levenshtein distance is at most the length of the longer string + for(size_t i = 0; i < nstrings; ++i) + { + const size_t len = strlen(strings[i]); + if(len > mindist) + mindist = len; + } + + // Loop over all strings and find the closest match + for (size_t i = 0; i < nstrings; ++i) + { + // Calculate the Levenshtein distance between the current string + // (out of nstrings) and the string we are checking against + const char *current = strings[i]; + size_t dist = levenshtein_distance(current, strlen(current), string, strlen(string)); + + // If the distance is smaller than the smallest minimum we found + // so far, update the minimum and the index of the closest match + if (mindist >= dist) + { + mindist = dist; + minidx = i; + } + } + + // Return "---" if no match was found (this can only happen if no + // strings were given) + if(minidx == -1) + return "---"; + + // else: Return the closest match + return strings[minidx]; +} \ No newline at end of file diff --git a/src/config/levenshtein.h b/src/config/levenshtein.h new file mode 100644 index 000000000..16252cf63 --- /dev/null +++ b/src/config/levenshtein.h @@ -0,0 +1,19 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Levenshtein distance prototypes +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ +#ifndef LEVENSHTEIN_H +#define LEVENSHTEIN_H + +#include "FTL.h" +// union conf_value +#include "config.h" + +const char *suggest_closest(const char *strings[], size_t nstrings, const char *string) __attribute__((pure)); + +#endif //LEVENSHTEIN_H From 5ddbdf4607b911322753e908251e806d0c23ebc4 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Nov 2023 10:10:30 +0100 Subject: [PATCH 05/14] Log invalid environment variables Signed-off-by: DL6ER --- src/config/env.c | 161 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 146 insertions(+), 15 deletions(-) diff --git a/src/config/env.c b/src/config/env.c index 3ee3dcd06..55ed1ffd6 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -22,6 +22,7 @@ struct env_item { bool used; + bool valid; char *key; char *value; struct env_item *next; @@ -72,43 +73,47 @@ void printFTLenv(void) return; // Count number of used and ignored env vars - unsigned int used = 0, ignored = 0; + unsigned int used = 0, invalid = 0, ignored = 0; for(struct env_item *item = env_list; item != NULL; item = item->next) { if(item->used) - used++; + if(item->valid) + used++; + else + invalid++; else ignored++; } - log_info("%u FTLCONF environment variable%s found (%u used, %u ignored)", - used + ignored, used + ignored == 1 ? "" : "s", used, ignored); + const unsigned int sum = used + invalid + ignored; + log_info("%u FTLCONF environment variable%s found (%u used, %u invalid, %u ignored)", + sum, sum == 1 ? "" : "s", used, invalid, ignored); // Iterate over all known FTLCONF environment variables for(struct env_item *item = env_list; item != NULL; item = item->next) { if(item->used) { - log_info("%s %s", cli_tick(), item->key); + if(item->valid) + log_info("%s %s is used", cli_tick(), item->key); + else + log_info("%s %s is invalid, using default", cli_cross(), item->key); continue; } // else: print warning const char *closest_match = suggest_closest(env_keys, sizeof(env_keys) / sizeof(*env_keys), item->key); - log_warn("%s %s is unknown, did you mean %s ?", cli_cross(), item->key, closest_match); + log_info("%s %s is unknown and ignored, did you mean %s ?", cli_qst(), item->key, closest_match); } } -static char *getFTLenv(const char *key) +static struct env_item *__attribute__((pure)) getFTLenv(const char *key) { // Iterate over all known FTLCONF environment variables for(struct env_item *item = env_list; item != NULL; item = item->next) { // Check if this is the requested key if(strcmp(item->key, key) == 0) - { - item->used = true; - return item->value; - } + return item; } // Return NULL if the key was not found @@ -132,12 +137,19 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) { // First check if a environmental variable with the given key exists by // iterating over the list of FTLCONF_ variables - char *envvar = getFTLenv(conf_item->e); + struct env_item *item = getFTLenv(conf_item->e); // Return early if this environment variable does not exist - if(envvar == NULL) + if(item == NULL) return false; + + // Mark this environment variable as used + item->used = true; + + // else: We found an environment variable with the given key + const char *envvar = item != NULL ? item->value : NULL; + log_debug(DEBUG_CONFIG, "ENV %s = %s", conf_item->e, envvar); switch(conf_item->t) @@ -145,75 +157,129 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) case CONF_BOOL: { if(strcasecmp(envvar, "true") == 0 || strcasecmp(envvar, "yes") == 0) + { conf_item->v.b = true; + item->valid = true; + } else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) + { conf_item->v.b = false; + item->valid = true; + } else + { log_warn("ENV %s is not of type bool", conf_item->e); + item->valid = false; + } break; } case CONF_ALL_DEBUG_BOOL: { if(strcasecmp(envvar, "true") == 0 || strcasecmp(envvar, "yes") == 0) + { set_all_debug(newconf, true); + item->valid = true; + } else if(strcasecmp(envvar, "false") == 0 || strcasecmp(envvar, "no") == 0) + { set_all_debug(newconf, false); + item->valid = true; + } else + { log_warn("ENV %s is not of type bool", conf_item->e); + item->valid = false; + } break; } case CONF_INT: { int val = 0; if(sscanf(envvar, "%i", &val) == 1) + { conf_item->v.i = val; + item->valid = true; + } else + { log_warn("ENV %s is not of type integer", conf_item->e); + item->valid = false; + } break; } case CONF_UINT: { unsigned int val = 0; if(sscanf(envvar, "%u", &val) == 1) + { conf_item->v.ui = val; + item->valid = true; + } else + { log_warn("ENV %s is not of type unsigned integer", conf_item->e); + item->valid = false; + } break; } case CONF_UINT16: { unsigned int val = 0; if(sscanf(envvar, "%u", &val) == 1 && val <= UINT16_MAX) + { conf_item->v.ui = val; + item->valid = true; + } else + { log_warn("ENV %s is not of type unsigned integer (16 bit)", conf_item->e); + item->valid = false; + } break; } case CONF_LONG: { long val = 0; if(sscanf(envvar, "%li", &val) == 1) + { conf_item->v.l = val; + item->valid = true; + } else + { log_warn("ENV %s is not of type long", conf_item->e); + item->valid = false; + } break; } case CONF_ULONG: { unsigned long val = 0; if(sscanf(envvar, "%lu", &val) == 1) + { conf_item->v.ul = val; + item->valid = true; + } else + { log_warn("ENV %s is not of type unsigned long", conf_item->e); + item->valid = false; + } break; } case CONF_DOUBLE: { double val = 0; if(sscanf(envvar, "%lf", &val) == 1) + { conf_item->v.d = val; + item->valid = true; + } else + { log_warn("ENV %s is not of type double", conf_item->e); + item->valid = false; + } break; } case CONF_STRING: @@ -223,78 +289,127 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) free(conf_item->v.s); conf_item->v.s = strdup(envvar); conf_item->t = CONF_STRING_ALLOCATED; + item->valid = true; break; } case CONF_ENUM_PTR_TYPE: { const int ptr_type = get_ptr_type_val(envvar); if(ptr_type != -1) + { conf_item->v.ptr_type = ptr_type; + item->valid = true; + } else + { log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->valid = false; + } break; } case CONF_ENUM_BUSY_TYPE: { const int busy_reply = get_busy_reply_val(envvar); if(busy_reply != -1) + { conf_item->v.busy_reply = busy_reply; + item->valid = true; + } else + { log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->valid = false; + } break; } case CONF_ENUM_BLOCKING_MODE: { const int blocking_mode = get_blocking_mode_val(envvar); if(blocking_mode != -1) + { conf_item->v.blocking_mode = blocking_mode; + item->valid = true; + } else + { log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->valid = false; + } break; } case CONF_ENUM_REFRESH_HOSTNAMES: { const int refresh_hostnames = get_refresh_hostnames_val(envvar); if(refresh_hostnames != -1) + { conf_item->v.refresh_hostnames = refresh_hostnames; + item->valid = true; + } else + { log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->valid = false; + } break; } case CONF_ENUM_LISTENING_MODE: { const int listeningMode = get_listeningMode_val(envvar); if(listeningMode != -1) + { conf_item->v.listeningMode = listeningMode; + item->valid = true; + } else + { log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->valid = false; + } break; } case CONF_ENUM_WEB_THEME: { const int web_theme = get_web_theme_val(envvar); if(web_theme != -1) + { conf_item->v.web_theme = web_theme; + item->valid = true; + } else + { log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->valid = false; + } break; } case CONF_ENUM_TEMP_UNIT: { const int temp_unit = get_temp_unit_val(envvar); if(temp_unit != -1) + { conf_item->v.temp_unit = temp_unit; + item->valid = true; + } else + { log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->valid = false; + } break; } case CONF_ENUM_PRIVACY_LEVEL: { int val = 0; if(sscanf(envvar, "%i", &val) == 1 && val >= PRIVACY_SHOW_ALL && val <= PRIVACY_MAXIMUM) + { conf_item->v.i = val; + item->valid = true; + } else + { log_warn("ENV %s is invalid (not of type integer or outside allowed bounds)", conf_item->e); + item->valid = false; + } break; } case CONF_STRUCT_IN_ADDR: @@ -306,9 +421,15 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) conf_item->v.in_addr.s_addr = INADDR_ANY; } else if(inet_pton(AF_INET, envvar, &addr4)) + { memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); + item->valid = true; + } else + { log_warn("ENV %s is invalid (not of type IPv4 address)", conf_item->e); + item->valid = false; + } break; } case CONF_STRUCT_IN6_ADDR: @@ -320,9 +441,15 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) memcpy(&conf_item->v.in6_addr, &in6addr_any, sizeof(in6addr_any)); } else if(inet_pton(AF_INET6, envvar, &addr6)) + { memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); + item->valid = true; + } else + { log_warn("ENV %s is invalid (not of type IPv6 address)", conf_item->e); + item->valid = false; + } break; } case CONF_JSON_STRING_ARRAY: @@ -342,14 +469,15 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(strlen(elem) > 0) { // Add string to our JSON array - cJSON *item = cJSON_CreateString(elem); - cJSON_AddItemToArray(conf_item->v.json, item); + cJSON *citem = cJSON_CreateString(elem); + cJSON_AddItemToArray(conf_item->v.json, citem); } // Search for the next element elem = strtok(NULL, delim); } free(envvar_copy); + item->valid = true; break; } case CONF_PASSWORD: @@ -357,8 +485,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) if(!set_and_check_password(conf_item, envvar)) { log_warn("ENV %s is invalid", conf_item->e); + item->valid = false; break; } + item->valid = true; + break; } } From 2932ca34d5269e4f5dcc5ee06a3e463c928d749c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 31 Oct 2023 19:31:53 +0100 Subject: [PATCH 06/14] Add env vars tests Signed-off-by: DL6ER --- test/run.sh | 2 ++ test/test_suite.bats | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/test/run.sh b/test/run.sh index 88e8f7f2b..5f34e9d60 100755 --- a/test/run.sh +++ b/test/run.sh @@ -70,6 +70,8 @@ umask 0022 # Set exemplary config value by environment variable export FTLCONF_misc_nice="-11" +export FTLCONF_dns_upstrrr="-11" +export FTLCONF_debug_api="not_a_bool" # Start FTL if ! su pihole -s /bin/sh -c /home/pihole/pihole-FTL; then diff --git a/test/test_suite.bats b/test/test_suite.bats index 650f9a48c..f7cca9258 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1297,6 +1297,30 @@ [[ ${lines[1]} == " nice = -11 ### CHANGED, default = -10" ]] } +@test "Correct number of environmental variables is logged" { + run bash -c 'grep -q "3 FTLCONF environment variables found (1 used, 1 invalid, 1 ignored)" /var/log/pihole/FTL.log' + printf "%s\n" "${lines[@]}" + [[ $status == 0 ]] +} + +@test "Correct environmental variable is logged" { + run bash -c 'grep -q "FTLCONF_misc_nice is used" /var/log/pihole/FTL.log' + printf "%s\n" "${lines[@]}" + [[ $status == 0 ]] +} + +@test "Invalid environmental variable is logged" { + run bash -c 'grep -q "FTLCONF_debug_api is invalid, using default" /var/log/pihole/FTL.log' + printf "%s\n" "${lines[@]}" + [[ $status == 0 ]] +} + +@test "Unknown environmental variable is logged, a useful alternative is suggested" { + run bash -c 'grep -q "FTLCONF_dns_upstrrr is unknown and ignored, did you mean FTLCONF_dns_upstreams" /var/log/pihole/FTL.log' + printf "%s\n" "${lines[@]}" + [[ $status == 0 ]] +} + @test "Changing a config option set forced by ENVVAR is not possible via the CLI" { run bash -c './pihole-FTL --config misc.nice -12' printf "%s\n" "${lines[@]}" From 0fbf0d3da7abf9af36a75230f0f3621374d07ddb Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 1 Nov 2023 06:48:40 +0100 Subject: [PATCH 07/14] Add the (fuzzy) Bitap algorithm and suggest up to three alternatives for typos Signed-off-by: DL6ER --- src/config/CMakeLists.txt | 4 +- src/config/env.c | 12 +- src/config/env.h | 2 + src/config/levenshtein.c | 126 ----------- src/config/suggest.c | 285 ++++++++++++++++++++++++ src/config/{levenshtein.h => suggest.h} | 5 +- 6 files changed, 301 insertions(+), 133 deletions(-) delete mode 100644 src/config/levenshtein.c create mode 100644 src/config/suggest.c rename src/config/{levenshtein.h => suggest.h} (70%) diff --git a/src/config/CMakeLists.txt b/src/config/CMakeLists.txt index 15f91874b..07522ed37 100644 --- a/src/config/CMakeLists.txt +++ b/src/config/CMakeLists.txt @@ -21,10 +21,10 @@ set(sources inotify.h legacy_reader.c legacy_reader.h - levenshtein.c - levenshtein.h password.c password.h + suggest.c + suggest.h toml_writer.c toml_writer.h toml_reader.c diff --git a/src/config/env.c b/src/config/env.c index 55ed1ffd6..054b3431a 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -18,7 +18,7 @@ // cli_tick() #include "args.h" // suggest_closest() -#include "config/levenshtein.h" +#include "config/suggest.h" struct env_item { bool used; @@ -101,8 +101,14 @@ void printFTLenv(void) continue; } // else: print warning - const char *closest_match = suggest_closest(env_keys, sizeof(env_keys) / sizeof(*env_keys), item->key); - log_info("%s %s is unknown and ignored, did you mean %s ?", cli_qst(), item->key, closest_match); + unsigned int N = 0; + char **matches = suggest_closest(env_keys, sizeof(env_keys) / sizeof(*env_keys), item->key, &N); + + // Print the closest matches + log_info("%s %s is unknown and ignored, did you mean any of these?", cli_qst(), item->key); + for(size_t i = 0; i < N; ++i) + log_info(" - %s", matches[i]); + free(matches); } } diff --git a/src/config/env.h b/src/config/env.h index 0b4c9d80a..d26c7e751 100644 --- a/src/config/env.h +++ b/src/config/env.h @@ -18,6 +18,8 @@ #define FTLCONF_PREFIX "FTLCONF_" +int dist(const char *str); + void getEnvVars(void); void freeEnvVars(void); void printFTLenv(void); diff --git a/src/config/levenshtein.c b/src/config/levenshtein.c deleted file mode 100644 index d135638c4..000000000 --- a/src/config/levenshtein.c +++ /dev/null @@ -1,126 +0,0 @@ - -/* Pi-hole: A black hole for Internet advertisements -* (c) 2023 Pi-hole, LLC (https://pi-hole.net) -* Network-wide ad blocking via your own hardware. -* -* FTL Engine -* Levenshtein distance routines -* -* This file is copyright under the latest version of the EUPL. -* Please see LICENSE file for your rights under this license. */ - -#include "config/levenshtein.h" - -// Returns the minimum of three size_t values -static size_t min3(size_t x, size_t y, size_t z) -{ - const size_t tmp = x < y ? x : y; - return tmp < z ? tmp : z; -} - -// Simply swaps two size_t pointers in memory -static void swap(size_t **a, size_t **b) -{ - size_t *tmp = *a; - *a = *b; - *b = tmp; -} - -// The Levenshtein distance is a string metric for measuring the difference -// between two sequences. Informally, the Levenshtein distance between two words -// is the minimum number of single-character edits (insertions, deletions or -// substitutions) required to change one word into the other. It is named after -// the Soviet mathematician Vladimir Levenshtein, who considered this distance -// in 1965. (Wikipedia) -// -// For example, the Levenshtein distance between "kitten" and "sitting" is 3, -// since the following 3 edits change one into the other, and there is no way to -// do it with fewer than 3 edits: -// kitten -> sitten (substitution of "s" for "k"), -// sitten -> sittin (substitution of "i" for "e"), -// sittin -> sitting (insertion of "g" at the end). -// -// Our implementation follows the algorithm described in Wikipedia but was -// inspired by https://stackoverflow.com/a/71810739/2087442 -static size_t levenshtein_distance(const char *s1, const size_t len1, const char *s2, const size_t len2) -{ - // Allocate two vectors of size len2 + 1 - size_t *v0 = calloc(len2 + 1, sizeof(size_t)); - size_t *v1 = calloc(len2 + 1, sizeof(size_t)); - - // Initialize v0 - // v0[i] = the Levenshtein distance between s1[0..i] and the empty string - // v0[i] = i - for (size_t j = 0; j <= len2; ++j) - v0[j] = j; - - // Calculate v1 - // v1[i] = the Levenshtein distance between s1[0..i] and s2[0..j] - // v1[i] = min(v0[j] + 1, v1[j - 1] + 1, v0[j - 1] + (s1[i] == s2[j] ? 0 : 1)) - for (size_t i = 0; i < len1; ++i) - { - // Initialize v1 - v1[0] = i + 1; - - // Loop over remaining columns - for (size_t j = 0; j < len2; ++j) - { - // Calculate deletion, insertion and substitution costs - const size_t delcost = v0[j + 1] + 1; - const size_t inscost = v1[j] + 1; - const size_t subcost = s1[i] == s2[j] ? v0[j] : v0[j] + 1; - - // Take the minimum of the three costs (see above) - v1[j + 1] = min3(delcost, inscost, subcost); - } - - // Swap addresses to avoid copying data around - swap(&v0, &v1); - } - - // Return the Levenshtein distance between s1 and s2 - size_t dist = v0[len2]; - free(v0); - free(v1); - return dist; -} - -// Returns the the closest matching string -const char *__attribute__((pure)) suggest_closest(const char *strings[], size_t nstrings, const char *string) -{ - size_t mindist = 0; - ssize_t minidx = -1; - - // The Levenshtein distance is at most the length of the longer string - for(size_t i = 0; i < nstrings; ++i) - { - const size_t len = strlen(strings[i]); - if(len > mindist) - mindist = len; - } - - // Loop over all strings and find the closest match - for (size_t i = 0; i < nstrings; ++i) - { - // Calculate the Levenshtein distance between the current string - // (out of nstrings) and the string we are checking against - const char *current = strings[i]; - size_t dist = levenshtein_distance(current, strlen(current), string, strlen(string)); - - // If the distance is smaller than the smallest minimum we found - // so far, update the minimum and the index of the closest match - if (mindist >= dist) - { - mindist = dist; - minidx = i; - } - } - - // Return "---" if no match was found (this can only happen if no - // strings were given) - if(minidx == -1) - return "---"; - - // else: Return the closest match - return strings[minidx]; -} \ No newline at end of file diff --git a/src/config/suggest.c b/src/config/suggest.c new file mode 100644 index 000000000..b694330aa --- /dev/null +++ b/src/config/suggest.c @@ -0,0 +1,285 @@ + +/* Pi-hole: A black hole for Internet advertisements +* (c) 2023 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* String suggestion routines +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#include "config/suggest.h" + +// Returns the minimum of three size_t values +static size_t min3(size_t x, size_t y, size_t z) +{ + const size_t tmp = x < y ? x : y; + return tmp < z ? tmp : z; +} + +// Simply swaps two size_t pointers in memory +static void swap(size_t **a, size_t **b) +{ + size_t *tmp = *a; + *a = *b; + *b = tmp; +} + +// The Levenshtein distance is a string metric for measuring the difference +// between two sequences. Informally, the Levenshtein distance between two words +// is the minimum number of single-character edits (insertions, deletions or +// substitutions) required to change one word into the other. It is named after +// the Soviet mathematician Vladimir Levenshtein, who considered this distance +// in 1965. (Wikipedia) +// +// For example, the Levenshtein distance between "kitten" and "sitting" is 3, +// since the following 3 edits change one into the other, and there is no way to +// do it with fewer than 3 edits: +// kitten -> sitten (substitution of "s" for "k"), +// sitten -> sittin (substitution of "i" for "e"), +// sittin -> sitting (insertion of "g" at the end). +// +// Our implementation follows the algorithm described in Wikipedia but was +// inspired by https://stackoverflow.com/a/71810739/2087442 +static size_t levenshtein_distance(const char *s1, const size_t len1, const char *s2, const size_t len2) +{ + // Allocate two vectors of size len2 + 1 + size_t *v0 = calloc(len2 + 1, sizeof(size_t)); + size_t *v1 = calloc(len2 + 1, sizeof(size_t)); + + // Initialize v0 + // v0[i] = the Levenshtein distance between s1[0..i] and the empty string + // v0[i] = i + for (size_t j = 0; j <= len2; ++j) + v0[j] = j; + + // Calculate v1 + // v1[i] = the Levenshtein distance between s1[0..i] and s2[0..j] + // v1[i] = min(v0[j] + 1, v1[j - 1] + 1, v0[j - 1] + (s1[i] == s2[j] ? 0 : 1)) + for (size_t i = 0; i < len1; ++i) + { + // Initialize v1 + v1[0] = i + 1; + + // Loop over remaining columns + for (size_t j = 0; j < len2; ++j) + { + // Calculate deletion, insertion and substitution costs + const size_t delcost = v0[j + 1] + 1; + const size_t inscost = v1[j] + 1; + const size_t subcost = s1[i] == s2[j] ? v0[j] : v0[j] + 1; + + // Take the minimum of the three costs (see above) + v1[j + 1] = min3(delcost, inscost, subcost); + } + + // Swap addresses to avoid copying data around + swap(&v0, &v1); + } + + // Return the Levenshtein distance between s1 and s2 + size_t dist = v0[len2]; + free(v0); + free(v1); + return dist; +} + +// The Bitap algorithm (also known as the shift-or, shift-and or Baeza-Yates- +// Gonnet algorithm) is an approximate string matching algorithm. The algorithm +// tells whether a given text contains a substring which is "approximately equal" +// to a given pattern, where approximate equality is defined in terms of Levenshtein +// distance — if the substring and pattern are within a given distance k of each +// other, then the algorithm considers them equal. (Wikipedia) +// +// Bitap distinguishes itself from other well-known string searching algorithms in +// its natural mapping onto simple bitwise operations +// +// Notice that in this implementation, counterintuitively, each bit with value +// zero indicates a match, and each bit with value 1 indicates a non-match. The +// same algorithm can be written with the intuitive semantics for 0 and 1, but +// in that case we must introduce another instruction into the inner loop to set +// R |= 1. In this implementation, we take advantage of the fact that +// left-shifting a value shifts in zeros on the right, which is precisely the +// behavior we need. +// +// This implementation is based on https://en.wikipedia.org/wiki/Bitap_algorithm +static const char *bitap_fuzzy_bitwise_search(const char *text, const char *pattern, unsigned int k) +{ + const size_t m = strlen(pattern); + const char *result = NULL; + unsigned long pattern_mask[256]; + unsigned long *R; + + // Sanity checks + if (pattern[0] == '\0') + return text; + if (m > 31) + return "The pattern is too long!"; + + // Initialize the bit array R + // To perform fuzzy string searching using the bitap algorithm, it is + // necessary to extend the bit array R into a second dimension. Instead + // of having a single array R that changes over the length of the text, + // we now have k distinct arrays R1..k. Array Ri holds a representation + // of the prefixes of pattern that match any suffix of the current + // string with i or fewer errors. In this context, an "error" may be an + // insertion, deletion, or substitution; see Levenshtein distance for + // more information on these operations. + R = calloc(k + 1, sizeof(*R)); + for (unsigned int i = 0; i <= k; ++i) + R[i] = ~1; + + // Initialize the pattern bitmasks + for (unsigned int i = 0; i < sizeof(pattern_mask)/sizeof(*pattern_mask); ++i) + pattern_mask[i] = ~0; + for (unsigned int i = 0; i < m; ++i) + pattern_mask[(unsigned char)pattern[i]] &= ~(1UL << i); + + // Iterate over all characters in the text + for (unsigned int i = 0; text[i] != '\0'; ++i) + { + // Update the bit arrays + // R[d] = (R[d] | pattern_mask[text[i]]) << 1; + unsigned long old_Rd1 = R[0]; + R[0] |= pattern_mask[(unsigned char)text[i]]; + R[0] <<= 1; + for (unsigned int d = 1; d <= k; ++d) + { + unsigned long tmp = R[d]; + // This algorithm only pays attention to substitutions, + // not to insertions or deletions – in other words, a + // Hamming distance of k. + R[d] = (old_Rd1 & (R[d] | pattern_mask[(unsigned char)text[i]])) << 1; + old_Rd1 = tmp; + } + + // If the last bit of R[k] is 0, we found a match + if (0 == (R[k] & (1UL << m))) + { + result = (text+i - m) + 1; + break; + } + } + + // Free the memory we allocated + free(R); + + // Return the result + return result; +} + +// Returns the the closest matching string using the Levenshtein distance +static const char *__attribute__((pure)) suggest_levenshtein(const char *strings[], size_t nstrings, const char *string) +{ + size_t mindist = 0; + ssize_t minidx = -1; + + // Convert the string to lowercase + // Skip the first 8 characters (i.e., "FTLCONF_") + char *lower_string = strdup(string); + const size_t m = strlen(lower_string); + for(size_t i = 8; i < strlen(lower_string); ++i) + lower_string[i] = tolower(lower_string[i]); + + // The Levenshtein distance is at most the length of the longer string + for(size_t i = 0; i < nstrings; ++i) + { + const size_t len = strlen(strings[i]); + if(len > mindist) + mindist = len; + } + + // Loop over all strings and find the closest match + for (size_t i = 0; i < nstrings; ++i) + { + // Calculate the Levenshtein distance between the current string + // (out of nstrings) and the string we are checking against + const char *current = strings[i]; + size_t dist = levenshtein_distance(current, strlen(current), lower_string, m); + + // If the distance is smaller than the smallest minimum we found + // so far, update the minimum and the index of the closest match + if (mindist >= dist) + { + mindist = dist; + minidx = i; + } + } + + // Free the memory we allocated + free(lower_string); + + // Return NULL if no match was found (this can only happen if no + // strings were given) + if(minidx == -1) + return NULL; + + // else: Return the closest match + return strings[minidx]; +} + +// Returns the the closest matching string using fuzzy searching +static unsigned int __attribute__((pure)) suggest_bitap(const char *strings[], size_t nstrings, const char *string, + char **results, unsigned int num_results) +{ + // Convert the string to lowercase + // Skip the first 8 characters (i.e., "FTLCONF_") + char *lower_string = strdup(string); + const size_t m = strlen(lower_string); + for(size_t i = 8; i < strlen(lower_string); ++i) + lower_string[i] = tolower(lower_string[i]); + + unsigned int found = 0; + + // Try to find a match with at most j errors + for(unsigned int j = 0; j < m; j++) + { + // Iterate over all strings and try to find a match + for(unsigned int i = 0; i < nstrings; ++i) + { + // Get the current string + const char *current = strings[i]; + + // Use the Bitap algorithm to find a match + const char *result = bitap_fuzzy_bitwise_search(current, lower_string, j); + + // If we found a match, add it to the list of results + if(result != NULL) + results[found++] = (char*)result; + + // If we found enough matches, stop searching + if(found >= num_results) + break; + } + + // If we found enough matches, stop searching + if(found >= num_results) + break; + } + + // Free the memory we allocated + free(lower_string); + + // Return the number of matches we found + return found; +} + +// Try to find up to two matches using the Bitap algorithm and one using the +// Levenshtein distance +#define MAX_MATCHES 3 +char **__attribute__((pure)) suggest_closest(const char *strings[], size_t nstrings, + const char *string, unsigned int *N) +{ + // Allocate memory for MAX_MATCHES matches + char** matches = calloc(MAX_MATCHES, sizeof(char*)); + + // Try to find (MAX_MATCHES - 1) matches using the Bitap algorithm + *N = suggest_bitap(strings, nstrings, string, matches, MAX_MATCHES - 1); + + // Try to find a last match using the Levenshtein distance + matches[(*N)++] = (char*)suggest_levenshtein(strings, nstrings, string); + + // Return the list of matches + return matches; +} diff --git a/src/config/levenshtein.h b/src/config/suggest.h similarity index 70% rename from src/config/levenshtein.h rename to src/config/suggest.h index 16252cf63..7087aa03a 100644 --- a/src/config/levenshtein.h +++ b/src/config/suggest.h @@ -3,7 +3,7 @@ * Network-wide ad blocking via your own hardware. * * FTL Engine -* Levenshtein distance prototypes +* String suggestion prototypes * * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ @@ -14,6 +14,7 @@ // union conf_value #include "config.h" -const char *suggest_closest(const char *strings[], size_t nstrings, const char *string) __attribute__((pure)); +char **suggest_closest(const char *strings[], size_t nstrings, + const char *string, unsigned int *N) __attribute__((pure)); #endif //LEVENSHTEIN_H From 5c20f4095f9d3c32dcb5be948edc23c6543ed187 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 1 Nov 2023 06:52:51 +0200 Subject: [PATCH 08/14] Use WARNING for unknown and ERROR for invalid env vars, also rename "ERR" to "ERROR" when printing errors (we also have INFO, WARNING, DEBUG, NOTICE, ALERT, ...) Signed-off-by: DL6ER --- src/config/env.c | 9 +++++---- src/config/suggest.c | 18 ++++++++++++++++++ src/log.c | 4 ++-- test/test_suite.bats | 8 ++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/config/env.c b/src/config/env.c index 054b3431a..dfd5f6d6d 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -95,9 +95,9 @@ void printFTLenv(void) if(item->used) { if(item->valid) - log_info("%s %s is used", cli_tick(), item->key); + log_info(" %s %s is used", cli_tick(), item->key); else - log_info("%s %s is invalid, using default", cli_cross(), item->key); + log_err(" %s %s is invalid, using default", cli_cross(), item->key); continue; } // else: print warning @@ -105,9 +105,10 @@ void printFTLenv(void) char **matches = suggest_closest(env_keys, sizeof(env_keys) / sizeof(*env_keys), item->key, &N); // Print the closest matches - log_info("%s %s is unknown and ignored, did you mean any of these?", cli_qst(), item->key); + log_warn("%s %s is unknown, did you mean any of these?", cli_qst(), item->key); for(size_t i = 0; i < N; ++i) - log_info(" - %s", matches[i]); + if(matches[i] != NULL) + log_warn(" - %s", matches[i]); free(matches); } } diff --git a/src/config/suggest.c b/src/config/suggest.c index b694330aa..b0cfdad7a 100644 --- a/src/config/suggest.c +++ b/src/config/suggest.c @@ -280,6 +280,24 @@ char **__attribute__((pure)) suggest_closest(const char *strings[], size_t nstri // Try to find a last match using the Levenshtein distance matches[(*N)++] = (char*)suggest_levenshtein(strings, nstrings, string); + // Loop over matches and remove duplicates + for(unsigned int i = 0; i < *N; ++i) + { + // Skip if there is no match here + if(matches[i] == NULL) + continue; + + // Loop over all matches after the current one + for(unsigned int j = i + 1; j < *N; ++j) + { + // If the current match is a duplicate, set it to NULL + if(matches[j] != NULL && strcmp(matches[i], matches[j]) == 0) + { + matches[j] = NULL; + } + } + } + // Return the list of matches return matches; } diff --git a/src/log.c b/src/log.c index 3b5902d10..ea7dafbea 100644 --- a/src/log.c +++ b/src/log.c @@ -54,7 +54,7 @@ void init_FTL_log(const char *name) if((logfile = fopen(config.files.log.ftl.v.s, "a+")) == NULL) { syslog(LOG_ERR, "Opening of FTL\'s log file failed, using syslog instead!"); - printf("ERR: Opening of FTL log (%s) failed!\n",config.files.log.ftl.v.s); + printf("ERROR: Opening of FTL log (%s) failed!\n",config.files.log.ftl.v.s); config.files.log.ftl.v.s = NULL; } @@ -141,7 +141,7 @@ static const char *priostr(const int priority, const enum debug_flag flag) return "CRIT"; // error conditions case LOG_ERR: - return "ERR"; + return "ERROR"; // warning conditions case LOG_WARNING: return "WARNING"; diff --git a/test/test_suite.bats b/test/test_suite.bats index f7cca9258..1819f5f43 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1163,10 +1163,10 @@ [[ "${lines[@]}" != *"ERROR"* ]] } -@test "No ERROR messages in FTL.log (besides known index.html error)" { - run bash -c 'grep "ERR: " /var/log/pihole/FTL.log' +@test "No ERROR messages in FTL.log (besides known/intended error)" { + run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" - run bash -c 'grep "ERR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)"' + run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)|(FTLCONF_debug_api is invalid)"' printf "count: %s\n" "${lines[@]}" [[ ${lines[0]} == "0" ]] } @@ -1316,7 +1316,7 @@ } @test "Unknown environmental variable is logged, a useful alternative is suggested" { - run bash -c 'grep -q "FTLCONF_dns_upstrrr is unknown and ignored, did you mean FTLCONF_dns_upstreams" /var/log/pihole/FTL.log' + run bash -c 'grep -q "FTLCONF_dns_upstrrr is unknown" /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" [[ $status == 0 ]] } From a23dad8abf3a15c37b304307b5b19b5cb2cb2152 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 1 Nov 2023 06:58:11 +0200 Subject: [PATCH 09/14] Explicitly log why env vars were invalid in the overview Signed-off-by: DL6ER --- src/config/env.c | 91 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 72 insertions(+), 19 deletions(-) diff --git a/src/config/env.c b/src/config/env.c index dfd5f6d6d..7877cfa27 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -25,6 +25,8 @@ struct env_item bool valid; char *key; char *value; + const char *error; + const char *allowed; struct env_item *next; }; @@ -52,6 +54,8 @@ void getEnvVars(void) new_item->used = false; new_item->key = strdup(key); new_item->value = strdup(value); + new_item->error = NULL; + new_item->allowed = NULL; new_item->next = env_list; env_list = new_item; } @@ -97,7 +101,18 @@ void printFTLenv(void) if(item->valid) log_info(" %s %s is used", cli_tick(), item->key); else - log_err(" %s %s is invalid, using default", cli_cross(), item->key); + { + if(item->error != NULL && item->allowed == NULL) + log_err(" %s %s is invalid (%s)", + cli_cross(), item->key, item->error); + else if(item->error != NULL && item->allowed != NULL) + log_err(" %s %s is invalid (%s, allowed options are: %s)", + cli_cross(), item->key, item->error, item->allowed); + else + log_err(" %s %s is invalid", + cli_cross(), item->key); + } + continue; } // else: print warning @@ -175,7 +190,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type bool", conf_item->e); + item->error = "not of type bool"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -194,7 +210,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type bool", conf_item->e); + item->error = "not of type bool"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -209,7 +226,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type integer", conf_item->e); + item->error = "not of type integer"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -224,7 +242,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type unsigned integer", conf_item->e); + item->error = "not of type unsigned integer"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -239,7 +258,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type unsigned integer (16 bit)", conf_item->e); + item->error = "not of type unsigned integer (16 bit"; + log_warn("ENV %s is %s)", conf_item->e, item->error); item->valid = false; } break; @@ -254,7 +274,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type long", conf_item->e); + item->error = "not of type long"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -269,7 +290,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type unsigned long", conf_item->e); + item->error = "not of type unsigned long"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -284,7 +306,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is not of type double", conf_item->e); + item->error = "not of type double"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -309,7 +332,10 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + item->error = "not an allowed option"; + item->allowed = conf_item->h; + log_warn("ENV %s is %s, allowed options are: %s", + conf_item->e, item->error, item->allowed); item->valid = false; } break; @@ -324,7 +350,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + + item->error = "not an allowed option"; + item->allowed = conf_item->h; + log_warn("ENV %s is %s, allowed options are: %s", + conf_item->e, item->error, item->allowed); item->valid = false; } break; @@ -339,7 +369,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + + item->error = "not an allowed option"; + item->allowed = conf_item->h; + log_warn("ENV %s is %s, allowed options are: %s", + conf_item->e, item->error, item->allowed); item->valid = false; } break; @@ -354,7 +388,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + + item->error = "not an allowed option"; + item->allowed = conf_item->h; + log_warn("ENV %s is %s, allowed options are: %s", + conf_item->e, item->error, item->allowed); item->valid = false; } break; @@ -369,7 +407,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + + item->error = "not an allowed option"; + item->allowed = conf_item->h; + log_warn("ENV %s is %s, allowed options are: %s", + conf_item->e, item->error, item->allowed); item->valid = false; } break; @@ -384,7 +426,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + + item->error = "not an allowed option"; + item->allowed = conf_item->h; + log_warn("ENV %s is %s, allowed options are: %s", + conf_item->e, item->error, item->allowed); item->valid = false; } break; @@ -399,7 +445,11 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid, allowed options are: %s", conf_item->e, conf_item->h); + + item->error = "not an allowed option"; + item->allowed = conf_item->h; + log_warn("ENV %s is %s, allowed options are: %s", + conf_item->e, item->error, item->allowed); item->valid = false; } break; @@ -414,7 +464,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid (not of type integer or outside allowed bounds)", conf_item->e); + item->error = "not of type integer or outside allowed bounds"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -434,7 +485,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid (not of type IPv4 address)", conf_item->e); + item->error = "not of type IPv4 address"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; @@ -454,7 +506,8 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) } else { - log_warn("ENV %s is invalid (not of type IPv6 address)", conf_item->e); + item->error = "not of type IPv6 address"; + log_warn("ENV %s is %s", conf_item->e, item->error); item->valid = false; } break; From 01d1644102da9b95cb68f166815c642fdf4bc697 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Nov 2023 10:11:49 +0100 Subject: [PATCH 10/14] Also suggest possible alternatives for mistyped config keys on the CLI Signed-off-by: DL6ER --- src/config/cli.c | 20 +++++- src/config/env.c | 13 +--- src/config/suggest.c | 155 +++++++++++++++++++++---------------------- src/config/suggest.h | 3 +- test/test_suite.bats | 2 +- 5 files changed, 96 insertions(+), 97 deletions(-) diff --git a/src/config/cli.c b/src/config/cli.c index c27c9928b..8fe6e1de1 100644 --- a/src/config/cli.c +++ b/src/config/cli.c @@ -22,6 +22,8 @@ #include "config/password.h" // check_capability() #include "capabilities.h" +// suggest_closest_conf_key() +#include "config/suggest.h" // Read a TOML value from a table depending on its type static bool readStringValue(struct conf_item *conf_item, const char *value, struct config *newconf) @@ -417,7 +419,13 @@ int set_config_from_CLI(const char *key, const char *value) // Check if we found the config option if(new_item == NULL) { - log_err("Unknown config option: %s", key); + unsigned int N = 0; + char **matches = suggest_closest_conf_key(false, key, &N); + log_err("Unknown config option %s, did you mean:", key); + for(unsigned int i = 0; i < N; i++) + log_err(" - %s", matches[i]); + free(matches); + free_config(&newconf); return 4; } @@ -512,8 +520,14 @@ int get_config_from_CLI(const char *key, const bool quiet) // Check if we found the config option if(key != NULL && conf_item == NULL) { - log_err("Unknown config option: %s", key); - return 2; + unsigned int N = 0; + char **matches = suggest_closest_conf_key(false, key, &N); + log_err("Unknown config option %s, did you mean:", key); + for(unsigned int i = 0; i < N; i++) + log_err(" - %s", matches[i]); + free(matches); + + return 4; } // Use return status if this is a boolean value diff --git a/src/config/env.c b/src/config/env.c index 7877cfa27..d332a3483 100644 --- a/src/config/env.c +++ b/src/config/env.c @@ -31,7 +31,6 @@ struct env_item }; static struct env_item *env_list = NULL; -static const char *env_keys[sizeof(config) / sizeof(struct conf_item)] = { NULL }; void getEnvVars(void) { @@ -61,13 +60,6 @@ void getEnvVars(void) } } - // Add all config item env keys to the list - for(unsigned int i = 0; i < sizeof(env_keys) / sizeof(*env_keys); i++) - { - struct conf_item *conf_item = get_conf_item(&config, i); - env_keys[i] = conf_item->e; - } - } void printFTLenv(void) @@ -117,13 +109,12 @@ void printFTLenv(void) } // else: print warning unsigned int N = 0; - char **matches = suggest_closest(env_keys, sizeof(env_keys) / sizeof(*env_keys), item->key, &N); + char **matches = suggest_closest_conf_key(true, item->key, &N); // Print the closest matches log_warn("%s %s is unknown, did you mean any of these?", cli_qst(), item->key); for(size_t i = 0; i < N; ++i) - if(matches[i] != NULL) - log_warn(" - %s", matches[i]); + log_warn(" - %s", matches[i]); free(matches); } } diff --git a/src/config/suggest.c b/src/config/suggest.c index b0cfdad7a..2f87bd79b 100644 --- a/src/config/suggest.c +++ b/src/config/suggest.c @@ -104,84 +104,59 @@ static size_t levenshtein_distance(const char *s1, const size_t len1, const char // behavior we need. // // This implementation is based on https://en.wikipedia.org/wiki/Bitap_algorithm -static const char *bitap_fuzzy_bitwise_search(const char *text, const char *pattern, unsigned int k) +static const char *__attribute__((pure)) bitap_bitwise_search(const char *text, const char *pattern, + const size_t pattern_len, unsigned int k) { - const size_t m = strlen(pattern); - const char *result = NULL; + // The bit array R is used to keep track of the current state of the + // search. + unsigned long R = ~1; + + // The pattern bitmask pattern_mask is used to represent the pattern + // string in a bitwise format. We use a size of 256 because our alphabet + // is all values of an unsigned char (0-255). unsigned long pattern_mask[256]; - unsigned long *R; // Sanity checks if (pattern[0] == '\0') return text; - if (m > 31) - return "The pattern is too long!"; - - // Initialize the bit array R - // To perform fuzzy string searching using the bitap algorithm, it is - // necessary to extend the bit array R into a second dimension. Instead - // of having a single array R that changes over the length of the text, - // we now have k distinct arrays R1..k. Array Ri holds a representation - // of the prefixes of pattern that match any suffix of the current - // string with i or fewer errors. In this context, an "error" may be an - // insertion, deletion, or substitution; see Levenshtein distance for - // more information on these operations. - R = calloc(k + 1, sizeof(*R)); - for (unsigned int i = 0; i <= k; ++i) - R[i] = ~1; + + if (pattern_len > 31) + return NULL; // Initialize the pattern bitmasks - for (unsigned int i = 0; i < sizeof(pattern_mask)/sizeof(*pattern_mask); ++i) + // First sets all bits in the bitmask to 1, ... + for (unsigned int i = 0; i < sizeof(pattern_mask) / sizeof(*pattern_mask); ++i) pattern_mask[i] = ~0; - for (unsigned int i = 0; i < m; ++i) + // ... and then set the corresponding bit in the bitmask to 0 for each + // character in the pattern + for (unsigned int i = 0; i < pattern_len; ++i) pattern_mask[(unsigned char)pattern[i]] &= ~(1UL << i); - // Iterate over all characters in the text - for (unsigned int i = 0; text[i] != '\0'; ++i) - { - // Update the bit arrays - // R[d] = (R[d] | pattern_mask[text[i]]) << 1; - unsigned long old_Rd1 = R[0]; - R[0] |= pattern_mask[(unsigned char)text[i]]; - R[0] <<= 1; - for (unsigned int d = 1; d <= k; ++d) - { - unsigned long tmp = R[d]; - // This algorithm only pays attention to substitutions, - // not to insertions or deletions – in other words, a - // Hamming distance of k. - R[d] = (old_Rd1 & (R[d] | pattern_mask[(unsigned char)text[i]])) << 1; - old_Rd1 = tmp; - } - - // If the last bit of R[k] is 0, we found a match - if (0 == (R[k] & (1UL << m))) - { - result = (text+i - m) + 1; - break; - } + // Loop over all characters in the text + for (unsigned int i = 0; text[i] != '\0'; ++i) { + // Update the bit array R based on the pattern bitmask + R |= pattern_mask[(unsigned char)text[i]]; + // Shift R one bit to the left + R <<= 1; + + // If the bit at the position corresponding to the pattern + // length in `R` is 0, an approximate match of the pattern has + // been found. Return the pointer to the start of this match + if ((R & (1UL << pattern_len)) == 0) + return (text + i - pattern_len) + 1; } - // Free the memory we allocated - free(R); - - // Return the result - return result; + // No match was found with the given allowed number of errors (k) + return NULL; } // Returns the the closest matching string using the Levenshtein distance -static const char *__attribute__((pure)) suggest_levenshtein(const char *strings[], size_t nstrings, const char *string) +static const char *__attribute__((pure)) suggest_levenshtein(const char *strings[], size_t nstrings, + const char *string, const size_t string_len) { size_t mindist = 0; ssize_t minidx = -1; - // Convert the string to lowercase - // Skip the first 8 characters (i.e., "FTLCONF_") - char *lower_string = strdup(string); - const size_t m = strlen(lower_string); - for(size_t i = 8; i < strlen(lower_string); ++i) - lower_string[i] = tolower(lower_string[i]); - // The Levenshtein distance is at most the length of the longer string for(size_t i = 0; i < nstrings; ++i) { @@ -196,7 +171,7 @@ static const char *__attribute__((pure)) suggest_levenshtein(const char *strings // Calculate the Levenshtein distance between the current string // (out of nstrings) and the string we are checking against const char *current = strings[i]; - size_t dist = levenshtein_distance(current, strlen(current), lower_string, m); + size_t dist = levenshtein_distance(current, strlen(current), string, string_len); // If the distance is smaller than the smallest minimum we found // so far, update the minimum and the index of the closest match @@ -207,9 +182,6 @@ static const char *__attribute__((pure)) suggest_levenshtein(const char *strings } } - // Free the memory we allocated - free(lower_string); - // Return NULL if no match was found (this can only happen if no // strings were given) if(minidx == -1) @@ -220,20 +192,14 @@ static const char *__attribute__((pure)) suggest_levenshtein(const char *strings } // Returns the the closest matching string using fuzzy searching -static unsigned int __attribute__((pure)) suggest_bitap(const char *strings[], size_t nstrings, const char *string, +static unsigned int __attribute__((pure)) suggest_bitap(const char *strings[], size_t nstrings, + const char *string, const size_t string_len, char **results, unsigned int num_results) { - // Convert the string to lowercase - // Skip the first 8 characters (i.e., "FTLCONF_") - char *lower_string = strdup(string); - const size_t m = strlen(lower_string); - for(size_t i = 8; i < strlen(lower_string); ++i) - lower_string[i] = tolower(lower_string[i]); - unsigned int found = 0; // Try to find a match with at most j errors - for(unsigned int j = 0; j < m; j++) + for(unsigned int j = 0; j < string_len; j++) { // Iterate over all strings and try to find a match for(unsigned int i = 0; i < nstrings; ++i) @@ -242,7 +208,7 @@ static unsigned int __attribute__((pure)) suggest_bitap(const char *strings[], s const char *current = strings[i]; // Use the Bitap algorithm to find a match - const char *result = bitap_fuzzy_bitwise_search(current, lower_string, j); + const char *result = bitap_bitwise_search(current, string, string_len, j); // If we found a match, add it to the list of results if(result != NULL) @@ -258,9 +224,6 @@ static unsigned int __attribute__((pure)) suggest_bitap(const char *strings[], s break; } - // Free the memory we allocated - free(lower_string); - // Return the number of matches we found return found; } @@ -268,17 +231,18 @@ static unsigned int __attribute__((pure)) suggest_bitap(const char *strings[], s // Try to find up to two matches using the Bitap algorithm and one using the // Levenshtein distance #define MAX_MATCHES 3 -char **__attribute__((pure)) suggest_closest(const char *strings[], size_t nstrings, - const char *string, unsigned int *N) +static char **__attribute__((pure)) suggest_closest(const char *strings[], size_t nstrings, + const char *string, const size_t string_len, + unsigned int *N) { // Allocate memory for MAX_MATCHES matches char** matches = calloc(MAX_MATCHES, sizeof(char*)); // Try to find (MAX_MATCHES - 1) matches using the Bitap algorithm - *N = suggest_bitap(strings, nstrings, string, matches, MAX_MATCHES - 1); + *N = suggest_bitap(strings, nstrings, string, string_len, matches, MAX_MATCHES - 1); // Try to find a last match using the Levenshtein distance - matches[(*N)++] = (char*)suggest_levenshtein(strings, nstrings, string); + matches[(*N)++] = (char*)suggest_levenshtein(strings, nstrings, string, string_len); // Loop over matches and remove duplicates for(unsigned int i = 0; i < *N; ++i) @@ -290,7 +254,7 @@ char **__attribute__((pure)) suggest_closest(const char *strings[], size_t nstri // Loop over all matches after the current one for(unsigned int j = i + 1; j < *N; ++j) { - // If the current match is a duplicate, set it to NULL + // Set all duplicates to NULL if(matches[j] != NULL && strcmp(matches[i], matches[j]) == 0) { matches[j] = NULL; @@ -298,6 +262,37 @@ char **__attribute__((pure)) suggest_closest(const char *strings[], size_t nstri } } + // Remove NULL entries from the list of matches + unsigned int j = 0; + for(unsigned int i = 0; i < *N; ++i) + { + // If the i-th element is not NULL, the i-th element is assigned + // to the j-th position in the array, and j is incremented by 1. + // This effectively moves non-NULL elements towards the front of + // the array. + if(matches[i] != NULL) + matches[j++] = matches[i]; + } + // Update the number of matches to the number of non-NULL elements + *N = j; + // Return the list of matches return matches; } + +char **suggest_closest_conf_key(const bool env, const char *string, unsigned int *N) +{ + // Collect all config item keys in a static list + const char *conf_keys[CONFIG_ELEMENTS] = { NULL }; + for(unsigned int i = 0; i < CONFIG_ELEMENTS; i++) + { + struct conf_item *conf_item = get_conf_item(&config, i); + if(!conf_item) + continue; + // Use either the environment key or the config key + conf_keys[i] = env ? conf_item->e : conf_item->k; + } + + // Return the list of closest matches + return suggest_closest(conf_keys, CONFIG_ELEMENTS, string, strlen(string), N); +} diff --git a/src/config/suggest.h b/src/config/suggest.h index 7087aa03a..f0b543e14 100644 --- a/src/config/suggest.h +++ b/src/config/suggest.h @@ -14,7 +14,6 @@ // union conf_value #include "config.h" -char **suggest_closest(const char *strings[], size_t nstrings, - const char *string, unsigned int *N) __attribute__((pure)); +char **suggest_closest_conf_key(const bool env, const char *string, unsigned int *N); #endif //LEVENSHTEIN_H diff --git a/test/test_suite.bats b/test/test_suite.bats index 1819f5f43..ee51d70f3 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1310,7 +1310,7 @@ } @test "Invalid environmental variable is logged" { - run bash -c 'grep -q "FTLCONF_debug_api is invalid, using default" /var/log/pihole/FTL.log' + run bash -c 'grep -q "FTLCONF_debug_api is invalid" /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" [[ $status == 0 ]] } From e8039ede14b2c76e8eeca9bd3b35be1b90256d89 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Nov 2023 10:12:08 +0100 Subject: [PATCH 11/14] Log config file statistics after writing Signed-off-by: DL6ER --- src/config/toml_writer.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/config/toml_writer.c b/src/config/toml_writer.c index add2a1e1c..0522ee16b 100644 --- a/src/config/toml_writer.c +++ b/src/config/toml_writer.c @@ -70,6 +70,7 @@ bool writeFTLtoml(const bool verbose) // Iterate over configuration and store it into the file char *last_path = (char*)""; + unsigned int modified = 0, env_vars = 0; for(unsigned int i = 0; i < CONFIG_ELEMENTS; i++) { // Get pointer to memory location of this conf_item @@ -106,7 +107,10 @@ bool writeFTLtoml(const bool verbose) // Print info if this value is overwritten by an env var if(conf_item->f & FLAG_ENV_VAR) + { print_comment(fp, ">>> This config is overwritten by an environmental variable <<<", "", 85, level-1); + env_vars++; + } // Write value indentTOML(fp, level-1); @@ -126,12 +130,26 @@ bool writeFTLtoml(const bool verbose) { fprintf(fp, " ### CHANGED, default = "); writeTOMLvalue(fp, -1, conf_item->t, &conf_item->d); + modified++; } // Add newlines after each entry fputs("\n\n", fp); } + // Log some statistics in verbose mode + if(verbose || config.debug.config.v.b) + { + log_info("Wrote config file:"); + log_info(" - %zu total entries", CONFIG_ELEMENTS); + log_info(" - %zu %s default", CONFIG_ELEMENTS - modified, + CONFIG_ELEMENTS - modified == 1 ? "entry is" : "entries are"); + log_info(" - %u %s modified", modified, + modified == 1 ? "entry is" : "entries are"); + log_info(" - %u %s forced through environment", env_vars, + env_vars == 1 ? "entry is" : "entries are"); + } + // Close file and release exclusive lock closeFTLtoml(fp); From fc4bcebf1e23dc9e7ad6959d133364662cb3f2ce Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 1 Nov 2023 07:11:10 +0200 Subject: [PATCH 12/14] Standardize CLI --config exit codes and add tests Signed-off-by: DL6ER --- .github/.codespellignore | 1 + src/config/cli.c | 25 +++++++++++++++++-------- test/test_suite.bats | 16 ++++++++++++++-- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/.github/.codespellignore b/.github/.codespellignore index 9d8b4f3ff..cdccd1cda 100644 --- a/.github/.codespellignore +++ b/.github/.codespellignore @@ -6,3 +6,4 @@ doubleclick requestor requestors punycode +bitap diff --git a/src/config/cli.c b/src/config/cli.c index 8fe6e1de1..584612ca6 100644 --- a/src/config/cli.c +++ b/src/config/cli.c @@ -25,6 +25,15 @@ // suggest_closest_conf_key() #include "config/suggest.h" +enum exit_codes { + OKAY = 0, + FAIL = 1, + VALUE_INVALID = 2, + DNSMASQ_TEST_FAILED = 3, + KEY_UNKNOWN = 4, + ENV_VAR_FORCED = 5, +} __attribute__((packed)); + // Read a TOML value from a table depending on its type static bool readStringValue(struct conf_item *conf_item, const char *value, struct config *newconf) { @@ -403,7 +412,7 @@ int set_config_from_CLI(const char *key, const char *value) { log_err("Config option %s is read-only (set via environmental variable)", key); free_config(&newconf); - return 5; + return ENV_VAR_FORCED; } // This is the config option we are looking for @@ -427,14 +436,14 @@ int set_config_from_CLI(const char *key, const char *value) free(matches); free_config(&newconf); - return 4; + return KEY_UNKNOWN; } // Parse value if(!readStringValue(new_item, value, &newconf)) { free_config(&newconf); - return 2; + return VALUE_INVALID; } // Check if value changed compared to current value @@ -454,7 +463,7 @@ int set_config_from_CLI(const char *key, const char *value) // Test failed log_debug(DEBUG_CONFIG, "Config item %s: dnsmasq config test failed", conf_item->k); free_config(&newconf); - return 3; + return DNSMASQ_TEST_FAILED; } } else if(conf_item == &config.dns.hosts) @@ -482,7 +491,7 @@ int set_config_from_CLI(const char *key, const char *value) putchar('\n'); writeFTLtoml(false); - return EXIT_SUCCESS; + return OKAY; } int get_config_from_CLI(const char *key, const bool quiet) @@ -527,13 +536,13 @@ int get_config_from_CLI(const char *key, const bool quiet) log_err(" - %s", matches[i]); free(matches); - return 4; + return KEY_UNKNOWN; } // Use return status if this is a boolean value // and we are in quiet mode if(quiet && conf_item->t == CONF_BOOL) - return conf_item->v.b ? EXIT_SUCCESS : EXIT_FAILURE; + return conf_item->v.b ? OKAY : FAIL; - return EXIT_SUCCESS; + return OKAY; } diff --git a/test/test_suite.bats b/test/test_suite.bats index ee51d70f3..c8c16ef9a 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1316,9 +1316,21 @@ } @test "Unknown environmental variable is logged, a useful alternative is suggested" { - run bash -c 'grep -q "FTLCONF_dns_upstrrr is unknown" /var/log/pihole/FTL.log' + run bash -c 'grep -A1 "FTLCONF_dns_upstrrr is unknown" /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" - [[ $status == 0 ]] + [[ ${lines[0]} == *"WARNING: [?] FTLCONF_dns_upstrrr is unknown, did you mean any of these?" ]] + [[ ${lines[1]} == *"WARNING: - FTLCONF_dns_upstreams" ]] +} + +@test "CLI complains about unknown config key and offers a suggestion" { + run bash -c './pihole-FTL --config dbg.all' + [[ ${lines[0]} == "Unknown config option dbg.all, did you mean:" ]] + [[ ${lines[1]} == " - debug.all" ]] + [[ $status == 4 ]] + run bash -c './pihole-FTL --config misc.privacyLLL' + [[ ${lines[0]} == "Unknown config option misc.privacyLLL, did you mean:" ]] + [[ ${lines[1]} == " - misc.privacylevel" ]] + [[ $status == 4 ]] } @test "Changing a config option set forced by ENVVAR is not possible via the CLI" { From a92e656f38089c1adc8cf23d797c0a370f85ed1a Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 1 Nov 2023 18:40:18 +0100 Subject: [PATCH 13/14] Also suggest keys starting with the given key. This can be helpful to, e.g. find misc.privacylevel is misc.priv is entered as the Hamming distance to misc.nice is shorter Signed-off-by: DL6ER --- src/config/suggest.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/config/suggest.c b/src/config/suggest.c index 2f87bd79b..ed121e44f 100644 --- a/src/config/suggest.c +++ b/src/config/suggest.c @@ -228,9 +228,28 @@ static unsigned int __attribute__((pure)) suggest_bitap(const char *strings[], s return found; } +// Find string from list that starts with the given string +static const char *__attribute__((pure)) startswith(const char *strings[], size_t nstrings, + const char *string, const size_t string_len) +{ + // Loop over all strings + for (size_t i = 0; i < nstrings; ++i) + { + // Get the current string + const char *current = strings[i]; + + // If the current string starts with the given string, return it + if(strncasecmp(current, string, string_len) == 0) + return current; + } + + // Return NULL if no match was found + return NULL; +} + // Try to find up to two matches using the Bitap algorithm and one using the // Levenshtein distance -#define MAX_MATCHES 3 +#define MAX_MATCHES 6 static char **__attribute__((pure)) suggest_closest(const char *strings[], size_t nstrings, const char *string, const size_t string_len, unsigned int *N) @@ -238,8 +257,11 @@ static char **__attribute__((pure)) suggest_closest(const char *strings[], size_ // Allocate memory for MAX_MATCHES matches char** matches = calloc(MAX_MATCHES, sizeof(char*)); - // Try to find (MAX_MATCHES - 1) matches using the Bitap algorithm - *N = suggest_bitap(strings, nstrings, string, string_len, matches, MAX_MATCHES - 1); + // Try to find (MAX_MATCHES - 2) matches using the Bitap algorithm + *N = suggest_bitap(strings, nstrings, string, string_len, matches, MAX_MATCHES - 2); + + // Try to find a match that starts with the given string + matches[(*N)++] = (char*)startswith(strings, nstrings, string, string_len); // Try to find a last match using the Levenshtein distance matches[(*N)++] = (char*)suggest_levenshtein(strings, nstrings, string, string_len); From b6e475225e3f26a8b9169826d2b867d2343f8d87 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 23 Nov 2023 10:19:13 +0100 Subject: [PATCH 14/14] Warnings about incorrect ENV vars are expected and actually intended during CI testing Signed-off-by: DL6ER --- test/test_suite.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_suite.bats b/test/test_suite.bats index c8c16ef9a..b59823986 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -489,8 +489,8 @@ [[ ${lines[0]} == "The Pi-hole FTL engine - "* ]] } -@test "No WARNING messages in FTL.log (besides known capability issues)" { - run bash -c 'grep "WARNING:" /var/log/pihole/FTL.log | grep -v -E "CAP_NET_ADMIN|CAP_NET_RAW|CAP_SYS_NICE|CAP_IPC_LOCK|CAP_CHOWN|CAP_NET_BIND_SERVICE|(Cannot set process priority)"' +@test "No WARNING messages in FTL.log (besides known warnings)" { + run bash -c 'grep "WARNING:" /var/log/pihole/FTL.log | grep -v -E "CAP_NET_ADMIN|CAP_NET_RAW|CAP_SYS_NICE|CAP_IPC_LOCK|CAP_CHOWN|CAP_NET_BIND_SERVICE|(Cannot set process priority)|FTLCONF_"' printf "%s\n" "${lines[@]}" [[ "${lines[@]}" == "" ]] }