commit e0ae137df5ca4849e98cc957fc3081cc3f203d70 Author: Nick Mathewson nickm@torproject.org Date: Wed Oct 23 16:00:25 2019 -0400
Write a new set of config validation callbacks.
Unlike legacy_validate_fn_t, these callbacks have separation of duties, into:
* early normalization and computation. * validation * transition checking * late normalization and computation
Only the first and last steps get mutable objects. Only the transition-checking step gets to see the previous values of this object. --- src/app/config/config.c | 4 ++- src/lib/confmgt/confparse.c | 46 ++++++++++++++++++------ src/lib/confmgt/confparse.h | 86 +++++++++++++++++++++++++++++++++++++-------- 3 files changed, 110 insertions(+), 26 deletions(-)
diff --git a/src/app/config/config.c b/src/app/config/config.c index b372791a4..deed98d1a 100644 --- a/src/app/config/config.c +++ b/src/app/config/config.c @@ -3252,7 +3252,9 @@ STATIC int options_validate(const or_options_t *old_options, or_options_t *options, char **msg) { - return config_validate(get_options_mgr(), old_options, options, msg); + validation_status_t vs; + vs = config_validate(get_options_mgr(), old_options, options, msg); + return vs < 0 ? -1 : 0; }
#define REJECT(arg) \ diff --git a/src/lib/confmgt/confparse.c b/src/lib/confmgt/confparse.c index f1bec76c0..655fc3483 100644 --- a/src/lib/confmgt/confparse.c +++ b/src/lib/confmgt/confparse.c @@ -1148,10 +1148,11 @@ config_init(const config_mgr_t *mgr, void *options) * in order to set ancillary data. If `old_options` is provided, make sure * that the transition from `old_options` to `options` is permitted. * - * On success return 0; on failure set *msg_out to a newly allocated string - * explaining what is wrong, and return -1. + * On success return VSTAT_OK; on failure set *msg_out to a newly allocated + * string explaining what is wrong, and return a different validation_status_t + * to describe which step failed. **/ -static int +static validation_status_t config_validate_single(const config_format_t *fmt, const void *old_options, void *options, char **msg_out) @@ -1159,13 +1160,37 @@ config_validate_single(const config_format_t *fmt, tor_assert(fmt); tor_assert(options);
+ if (fmt->pre_normalize_fn) { + if (fmt->pre_normalize_fn(options, msg_out) < 0) { + return VSTAT_PRE_NORMALIZE_ERR; + } + } + if (fmt->legacy_validate_fn) { if (fmt->legacy_validate_fn(old_options, options, msg_out) < 0) { - return -1; + return VSTAT_LEGACY_ERR; } }
- return 0; + if (fmt->validate_fn) { + if (fmt->validate_fn(options, msg_out) < 0) { + return VSTAT_VALIDATE_ERR; + } + } + + if (fmt->check_transition_fn && old_options) { + if (fmt->check_transition_fn(old_options, options, msg_out) < 0) { + return VSTAT_TRANSITION_ERR; + } + } + + if (fmt->post_normalize_fn) { + if (fmt->post_normalize_fn(options, msg_out) < 0) { + return VSTAT_POST_NORMALIZE_ERR; + } + } + + return VSTAT_OK; }
/** @@ -1174,15 +1199,16 @@ config_validate_single(const config_format_t *fmt, * set ancillary data. If `old_options` is provided, make sure that the * transition from `old_options` to `options` is permitted. * - * On success return 0; on failure set *msg_out to a newly allocated string - * explaining what is wrong, and return -1. + * On success return VSTAT_OK; on failure set *msg_out to a newly allocated + * string explaining what is wrong, and return a different validation_status_t + * to describe which step failed. **/ -int +validation_status_t config_validate(const config_mgr_t *mgr, const void *old_options, void *options, char **msg_out) { - int rv; + validation_status_t rv; CONFIG_CHECK(mgr, options); if (old_options) { CONFIG_CHECK(mgr, old_options); @@ -1212,7 +1238,7 @@ config_validate(const config_mgr_t *mgr, if (rv < 0) return rv;
- return 0; + return VSTAT_OK; }
/** Allocate and return a new string holding the written-out values of the vars diff --git a/src/lib/confmgt/confparse.h b/src/lib/confmgt/confparse.h index fea80f916..7829f1aeb 100644 --- a/src/lib/confmgt/confparse.h +++ b/src/lib/confmgt/confparse.h @@ -52,25 +52,59 @@ typedef struct config_deprecation_t { #define PLURAL(tok) { #tok, #tok "s", 0, 0 }
/** - * Type of a callback to validate whether a given configuration is + * Validation function: verify whether a configuation object is well-formed + * and consistent. + * + * On success, return 0. On failure, set <b>msg_out</b> to a newly allocated + * string containing an error message, and return -1. */ +typedef int (*validate_fn_t)(const void *value, char **msg_out); +/** + * Validation function: verify whether a configuration object (`value`) is an + * allowable value given the previous configuration value (`old_value`). + * + * On success, return 0. On failure, set <b>msg_out</b> to a newly allocated + * string containing an error message, and return -1. */ +typedef int (*check_transition_fn_t)(const void *old_value, const void *value, + char **msg_out); +/** + * Validation function: normalize members of `value`, and compute derived + * members. + * + * This function is called before any other validation of `value`, and must + * not assume that validate_fn or check_transition_fn has passed. + * + * On success, return 0. On failure, set <b>msg_out</b> to a newly allocated + * string containing an error message, and return -1. */ +typedef int (*pre_normalize_fn_t)(void *value, char **msg_out); +/** + * Validation function: normalize members of `value`, and compute derived + * members. + * + * This function is called after validation of `value`, and may + * assume that validate_fn or check_transition_fn has passed. + * + * On success, return 0. On failure, set <b>msg_out</b> to a newly allocated + * string containing an error message, and return -1. */ +typedef int (*post_normalize_fn_t)(void *value, char **msg_out); + +/** + * Legacy function to validate whether a given configuration is * well-formed and consistent. * * The configuration to validate is passed as <b>newval</b>. The previous - * configuration, if any, is provided in <b>oldval</b>. The - * <b>default_val</b> argument receives a configuration object initialized - * with default values for all its fields. The <b>from_setconf</b> argument - * is true iff the input comes from a SETCONF controller command. + * configuration, if any, is provided in <b>oldval</b>. + * + * This API is deprecated, since it mixes the responsibilities of + * pre_normalize_fn_t, post_normalize_fn_t, validate_fn_t, and + * check_transition_fn_t. No new instances of this function type should + * be written. * * On success, return 0. On failure, set *<b>msg_out</b> to a newly allocated * error message, and return -1. - * - * REFACTORING NOTE: Currently, this callback type is only used from inside - * config_dump(); later in our refactoring, it will be cleaned up and used - * more generally. */ typedef int (*legacy_validate_fn_t)(const void *oldval, - void *newval, - char **msg_out); + void *newval, + char **msg_out);
struct config_mgr_t;
@@ -98,7 +132,18 @@ typedef struct config_format_t { const config_var_t *vars; /**< List of variables we recognize, their default * values, and where we stick them in the * structure. */ - legacy_validate_fn_t legacy_validate_fn; /**< Function to validate config. */ + + /** Early-stage normalization callback. Invoked by config_validate(). */ + pre_normalize_fn_t pre_normalize_fn; + /** Configuration validation function. Invoked by config_validate(). */ + validate_fn_t validate_fn; + /** Legacy validation function. Invoked by config_validate(). */ + legacy_validate_fn_t legacy_validate_fn; + /** Transition checking function. Invoked by config_validate(). */ + check_transition_fn_t check_transition_fn; + /** Late-stage normalization callback. Invoked by config_validate(). */ + post_normalize_fn_t post_normalize_fn; + clear_cfg_fn_t clear_fn; /**< Function to clear the configuration. */ /** If present, extra denotes a LINELIST variable for unrecognized * lines. Otherwise, unrecognized lines are an error. */ @@ -169,9 +214,20 @@ int config_is_same(const config_mgr_t *fmt, struct config_line_t *config_get_changes(const config_mgr_t *mgr, const void *options1, const void *options2); void config_init(const config_mgr_t *mgr, void *options); -int config_validate(const config_mgr_t *mgr, - const void *old_options, void *options, - char **msg_out); + +/** An enumeration to report which validation step failed. */ +typedef enum { + VSTAT_PRE_NORMALIZE_ERR = -5, + VSTAT_VALIDATE_ERR = -4, + VSTAT_LEGACY_ERR = -3, + VSTAT_TRANSITION_ERR = -2, + VSTAT_POST_NORMALIZE_ERR = -1, + VSTAT_OK = 0, +} validation_status_t; + +validation_status_t config_validate(const config_mgr_t *mgr, + const void *old_options, void *options, + char **msg_out); void *config_dup(const config_mgr_t *mgr, const void *old); char *config_dump(const config_mgr_t *mgr, const void *default_options, const void *options, int minimal,