From 55ec0f5776109efcd191c1a91e5107ca52a41c83 Mon Sep 17 00:00:00 2001 From: Vsevolod Stakhov Date: Wed, 8 Dec 2010 18:03:51 +0300 Subject: [PATCH] Add module options checker Improve time limits handling Fix smtp proxy options parsing --- src/cfg_file.h | 16 +++++-- src/cfg_utils.c | 100 ++++++++++++++++++++++++++++++++++++----- src/cfg_xml.c | 90 ++++++++++++++++++++++++++++++++----- src/cfg_xml.h | 12 ++++- src/greylist_storage.c | 4 +- src/map.c | 34 +++++++++----- src/map.h | 4 ++ src/plugins/surbl.c | 5 ++- src/smtp.c | 2 +- 9 files changed, 228 insertions(+), 39 deletions(-) diff --git a/src/cfg_file.h b/src/cfg_file.h index 9045d3a7a..2595cc779 100644 --- a/src/cfg_file.h +++ b/src/cfg_file.h @@ -42,6 +42,15 @@ struct classifier; enum { VAL_UNDEF=0, VAL_TRUE, VAL_FALSE }; +/** + * Type of time configuration parameter + */ +enum time_type { + TIME_SECONDS = 0, + TIME_MILLISECONDS, + TIME_MINUTES, + TIME_HOURS +}; /** * Types of rspamd bind lines */ @@ -357,11 +366,12 @@ gchar* get_module_opt (struct config_file *cfg, gchar *module_name, gchar *opt_n gsize parse_limit (const gchar *limit); /** - * Parse seconds + * Parse time * @param t string representation of seconds (eg. 1D) - * @return numeric value of string + * @param default_type dimension of time if no suffix is specified + * @return value of time in milliseconds */ -guint parse_seconds (const gchar *t); +guint parse_time (const gchar *t, enum time_type default_type); /** * Parse flag diff --git a/src/cfg_utils.c b/src/cfg_utils.c index 6432b2474..2a0f548b8 100644 --- a/src/cfg_utils.c +++ b/src/cfg_utils.c @@ -266,6 +266,7 @@ parse_limit (const gchar *limit) if (!limit || *limit == '\0') return 0; + errno = 0; result = strtoul (limit, &err_str, 10); if (*err_str != '\0') { @@ -281,45 +282,124 @@ parse_limit (const gchar *limit) else if (*err_str == 'g' || *err_str == 'G') { result *= 1073741824L; } + else { + msg_warn ("invalid limit value '%s' at position '%s'", limit, err_str); + result = 0; + } } return result; } guint -parse_seconds (const gchar *t) +parse_time (const gchar *t, enum time_type default_type) { - guint result = 0; + union { + guint i; + double d; + } result; + gboolean use_double = FALSE; gchar *err_str; if (!t || *t == '\0') return 0; - result = strtoul (t, &err_str, 10); + errno = 0; + result.i = strtoul (t, &err_str, 10); if (*err_str != '\0') { + if (*err_str == '.') { + /* Try to handle decimal point */ + errno = 0; + result.d = strtod (t, &err_str); + use_double = TRUE; + } /* Seconds */ if (*err_str == 's' || *err_str == 'S') { - result *= 1000; + if (use_double) { + result.d *= 1000.; + } + else { + result.i *= 1000; + } } /* Minutes */ else if (*err_str == 'm' || *err_str == 'M') { /* Handle ms correctly */ - if (*(err_str + 1) == 's' || *(err_str + 1) == 'S') { - result *= 60 * 1000; + if (*(err_str + 1) != 's' && *(err_str + 1) != 'S') { + if (use_double) { + result.d *= 60. * 1000.; + } + else { + result.i *= 60 * 1000; + } } } /* Hours */ else if (*err_str == 'h' || *err_str == 'H') { - result *= 60 * 60 * 1000; + if (use_double) { + result.d *= 60. * 60. * 1000.; + } + else { + result.i *= 60 * 60 * 1000; + } } /* Days */ else if (*err_str == 'd' || *err_str == 'D') { - result *= 24 * 60 * 60 * 1000; + if (use_double) { + result.d *= 24. * 60. * 60. * 1000.; + } + else { + result.i *= 24 * 60 * 60 * 1000; + } + } + else { + msg_warn ("invalid time value '%s' at position '%s'", t, err_str); + if (use_double) { + result.d = 0.; + } + else { + result.i = 0; + } } } - - return result; + else { + /* Switch to default time multiplier */ + switch (default_type) { + case TIME_HOURS: + if (use_double) { + result.d *= 60. * 60. * 1000.; + } + else { + result.i *= 60 * 60 * 1000; + } + break; + case TIME_MINUTES: + if (use_double) { + result.d *= 60. * 1000.; + } + else { + result.i *= 60 * 1000; + } + break; + case TIME_SECONDS: + if (use_double) { + result.d *= 1000.; + } + else { + result.i *= 1000; + } + break; + case TIME_MILLISECONDS: + break; + } + } + if (use_double) { + return rint (result.d); + } + else { + return result.i; + } } gchar diff --git a/src/cfg_xml.c b/src/cfg_xml.c index ddf995481..5ed8229bb 100644 --- a/src/cfg_xml.c +++ b/src/cfg_xml.c @@ -35,6 +35,7 @@ #include "tokenizers/tokenizers.h" #include "lua/lua_common.h" #include "view.h" +#include "map.h" #include "expressions.h" #include "settings.h" @@ -815,7 +816,8 @@ handle_module_opt (struct config_file *cfg, struct rspamd_xml_userdata *ctx, con is_lua = TRUE; } } - /* XXX: in fact we cannot check for lua modules and need to do it in post-config procedure + /* + * XXX: in fact we cannot check for lua modules and need to do it in post-config procedure * so just insert any options provided and try to handle them in further process */ @@ -1151,7 +1153,7 @@ handle_statfile_binlog_rotate (struct config_file *cfg, struct rspamd_xml_userda if (st->binlog == NULL) { st->binlog = memory_pool_alloc0 (cfg->cfg_pool, sizeof (struct statfile_binlog_params)); } - st->binlog->rotate_time = parse_seconds (data); + st->binlog->rotate_time = parse_time (data, TIME_SECONDS); return TRUE; } @@ -1226,7 +1228,7 @@ xml_handle_seconds (struct config_file *cfg, struct rspamd_xml_userdata *ctx, GH guint32 *dest; dest = (guint32 *)G_STRUCT_MEMBER_P (dest_struct, offset); - *dest = parse_seconds (data); + *dest = parse_time (data, TIME_SECONDS); return TRUE; } @@ -1686,9 +1688,79 @@ rspamd_xml_error (GMarkupParseContext *context, GError *error, gpointer user_dat } /* Register handlers for specific parts of config */ + +/* Checker for module options */ +static gboolean +check_module_option (struct config_file *cfg, const gchar *mname, const gchar *optname, const gchar *data) +{ + struct xml_config_param *param; + enum module_opt_type type; + GHashTable *module; + gchar *err_str; + + if (module_options == NULL) { + msg_warn ("no module options registered while checking option %s for module %s", mname, optname); + return FALSE; + } + if ((module = g_hash_table_lookup (module_options, mname)) == NULL) { + msg_warn ("module %s has not registered any options while checking for option %s", mname, optname); + return FALSE; + } + + if ((param = g_hash_table_lookup (module, optname)) == NULL) { + msg_warn ("module %s has not registered option %s", mname, optname); + return FALSE; + } + + type = param->offset; + + /* Now handle option of each type */ + switch (type) { + case MODULE_OPT_TYPE_STRING: + case MODULE_OPT_TYPE_ANY: + /* Allways OK */ + return TRUE; + case MODULE_OPT_TYPE_INT: + (void)strtol (data, &err_str, 10); + if (*err_str != '\0') { + msg_warn ("non-numeric data for option: '%s' for module: '%s' at position: '%s'", optname, mname, err_str); + return FALSE; + } + break; + case MODULE_OPT_TYPE_UINT: + (void)strtoul (data, &err_str, 10); + if (*err_str != '\0') { + msg_warn ("non-numeric data for option: '%s' for module: '%s' at position: '%s'", optname, mname, err_str); + return FALSE; + } + break; + case MODULE_OPT_TYPE_TIME: + (void)parse_time (data, TIME_SECONDS); + if (errno != 0) { + msg_warn ("non-numeric data for option: '%s' for module: '%s': %s", optname, mname, strerror (errno)); + return FALSE; + } + break; + case MODULE_OPT_TYPE_SIZE: + (void)parse_limit (data); + if (errno != 0) { + msg_warn ("non-numeric data for option: '%s' for module: '%s': %s", optname, mname, strerror (errno)); + return FALSE; + } + break; + case MODULE_OPT_TYPE_MAP: + if (!check_map_proto (data, NULL, NULL)) { + return FALSE; + } + break; + } + + return TRUE; +} + /* Register new module option */ void -register_module_opt (const gchar *mname, const gchar *optname, element_handler_func func, gpointer dest_struct, gint offset) +register_module_opt (const gchar *mname, const gchar *optname, enum module_opt_type type) { struct xml_config_param *param; GHashTable *module; @@ -1703,9 +1775,8 @@ register_module_opt (const gchar *mname, const gchar *optname, element_handler_f if ((param = g_hash_table_lookup (module, optname)) == NULL) { /* Register new param */ param = g_malloc (sizeof (struct xml_config_param)); - param->handler = func; - param->user_data = dest_struct; - param->offset = offset; + param->handler = NULL; + param->offset = type; param->name = optname; g_hash_table_insert (module, (char *)optname, param); } @@ -1714,9 +1785,8 @@ register_module_opt (const gchar *mname, const gchar *optname, element_handler_f msg_warn ("replace old handler for param '%s'", optname); g_free (param); param = g_malloc (sizeof (struct xml_config_param)); - param->handler = func; - param->user_data = dest_struct; - param->offset = offset; + param->handler = NULL; + param->offset = type; param->name = optname; g_hash_table_insert (module, (char *)optname, param); } diff --git a/src/cfg_xml.h b/src/cfg_xml.h index dc55d2c86..7c976e78e 100644 --- a/src/cfg_xml.h +++ b/src/cfg_xml.h @@ -28,6 +28,16 @@ enum xml_read_state { XML_END }; +enum module_opt_type { + MODULE_OPT_TYPE_STRING = 0, + MODULE_OPT_TYPE_INT, + MODULE_OPT_TYPE_UINT, + MODULE_OPT_TYPE_TIME, + MODULE_OPT_TYPE_MAP, + MODULE_OPT_TYPE_SIZE, + MODULE_OPT_TYPE_ANY +}; + struct rspamd_xml_userdata { enum xml_read_state state; struct config_file *cfg; @@ -137,7 +147,7 @@ gboolean handle_statfile_binlog_rotate (struct config_file *cfg, struct rspamd_x gboolean handle_statfile_binlog_master (struct config_file *cfg, struct rspamd_xml_userdata *ctx, GHashTable *attrs, gchar *data, gpointer user_data, gpointer dest_struct, gint offset); /* Register new module option */ -void register_module_opt (const gchar *mname, const gchar *optname, element_handler_func func, gpointer dest_struct, gint offset); +void register_module_opt (const gchar *mname, const gchar *optname, enum module_opt_type type); /* Register new worker's options */ void register_worker_opt (gint wtype, const gchar *optname, element_handler_func func, gpointer dest_struct, gint offset); diff --git a/src/greylist_storage.c b/src/greylist_storage.c index 2217c4d53..293cc1f7f 100644 --- a/src/greylist_storage.c +++ b/src/greylist_storage.c @@ -301,10 +301,10 @@ config_greylist_worker (struct rspamd_worker *worker) ctx->expire_time = DEFAULT_EXPIRE_TIME; if ((value = g_hash_table_lookup (worker->cf->params, "greylist_time")) != NULL) { - ctx->greylist_time = parse_seconds (value) / 1000; + ctx->greylist_time = parse_time (value, TIME_SECONDS) / 1000; } if ((value = g_hash_table_lookup (worker->cf->params, "expire_time")) != NULL) { - ctx->expire_time = parse_seconds (value) / 1000; + ctx->expire_time = parse_time (value, TIME_SECONDS) / 1000; } worker->ctx = ctx; diff --git a/src/map.c b/src/map.c index a5ad317e4..298c0c2e5 100644 --- a/src/map.c +++ b/src/map.c @@ -430,6 +430,29 @@ read_map_file (struct rspamd_map *map, struct file_map_data *data) *map->user_data = cbdata.cur_data; } +gboolean +check_map_proto (const gchar *map_line, gint *res, const gchar **pos) +{ + if (g_ascii_strncasecmp (map_line, "http://", sizeof ("http://") - 1) == 0) { + if (res && pos) { + *res = PROTO_HTTP; + *pos = map_line + sizeof ("http://") - 1; + } + } + else if (g_ascii_strncasecmp (map_line, "file://", sizeof ("file://") - 1) == 0) { + if (res && pos) { + *res = PROTO_FILE; + *pos = map_line + sizeof ("file://") - 1; + } + } + else { + msg_warn ("invalid map fetching protocol: %s", map_line); + return FALSE; + } + + return TRUE; +} + gboolean add_map (const gchar *map_line, map_cb_t read_callback, map_fin_cb_t fin_callback, void **user_data) { @@ -443,16 +466,7 @@ add_map (const gchar *map_line, map_cb_t read_callback, map_fin_cb_t fin_callbac struct hostent *hent; /* First of all detect protocol line */ - if (strncmp (map_line, "http://", sizeof ("http://") - 1) == 0) { - proto = PROTO_HTTP; - def = map_line + sizeof ("http://") - 1; - } - else if (strncmp (map_line, "file://", sizeof ("file://") - 1) == 0) { - proto = PROTO_FILE; - def = map_line + sizeof ("file://") - 1; - } - else { - msg_warn ("invalid map fetching protocol: %s", map_line); + if (!check_map_proto (map_line, (int *)&proto, &def)) { return FALSE; } /* Constant pool */ diff --git a/src/map.h b/src/map.h index eb427b600..de95c1153 100644 --- a/src/map.h +++ b/src/map.h @@ -69,6 +69,10 @@ struct rspamd_map { void *map_data; }; +/** + * Check map protocol + */ +gboolean check_map_proto (const gchar *map_line, gint *res, const gchar **pos); /** * Add map from line */ diff --git a/src/plugins/surbl.c b/src/plugins/surbl.c index bcaa30021..85f457a7e 100644 --- a/src/plugins/surbl.c +++ b/src/plugins/surbl.c @@ -224,6 +224,7 @@ surbl_module_init (struct config_file *cfg, struct module_ctx **ctx) *ctx = (struct module_ctx *)surbl_module_ctx; register_protocol_command ("urls", urls_command_handler); + /* Register module options */ return 0; } @@ -280,13 +281,13 @@ surbl_module_config (struct config_file *cfg) surbl_module_ctx->url_expire = DEFAULT_SURBL_URL_EXPIRE; } if ((value = get_module_opt (cfg, "surbl", "redirector_connect_timeout")) != NULL) { - surbl_module_ctx->connect_timeout = parse_seconds (value); + surbl_module_ctx->connect_timeout = parse_time (value, TIME_SECONDS); } else { surbl_module_ctx->connect_timeout = DEFAULT_REDIRECTOR_CONNECT_TIMEOUT; } if ((value = get_module_opt (cfg, "surbl", "redirector_read_timeout")) != NULL) { - surbl_module_ctx->read_timeout = parse_seconds (value); + surbl_module_ctx->read_timeout = parse_time (value, TIME_SECONDS); } else { surbl_module_ctx->read_timeout = DEFAULT_REDIRECTOR_READ_TIMEOUT; diff --git a/src/smtp.c b/src/smtp.c index d1bef023a..793f33e3a 100644 --- a/src/smtp.c +++ b/src/smtp.c @@ -958,7 +958,7 @@ config_smtp_worker (struct rspamd_worker *worker) return FALSE; } /* Create smtp banner */ - if ((ctx->smtp_banner_str) != NULL) { + if ((value = ctx->smtp_banner_str) != NULL) { parse_smtp_banner (ctx, value); } -- 2.39.5