From: Karl Fleischmann Date: Thu, 15 Dec 2022 09:32:30 +0000 (+0100) Subject: lib: Warn about ambiguous event filter units X-Git-Tag: 2.4.0~3278 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cc6d15c91dd062e44d5dfa6dac025938065ce9b3;p=thirdparty%2Fdovecot%2Fcore.git lib: Warn about ambiguous event filter units --- diff --git a/src/lib/event-filter-parser.y b/src/lib/event-filter-parser.y index d1fa655942..2c29306ece 100644 --- a/src/lib/event-filter-parser.y +++ b/src/lib/event-filter-parser.y @@ -9,6 +9,8 @@ %defines %{ +#include + #include "lib.h" #include "str-parse.h" #include "wildcard-match.h" @@ -55,6 +57,7 @@ static struct event_filter_node *key_value(struct event_filter_parser_state *sta node->type = type; node->op = op; node->warned_type_mismatch = FALSE; + node->ambiguous_unit = FALSE; switch (type) { case EVENT_FILTER_NODE_TYPE_LOGIC: @@ -113,6 +116,14 @@ static struct event_filter_node *key_value(struct event_filter_parser_state *sta uoff_t bytes; const char *error; int ret = str_parse_get_size(b, &bytes, &error); + if (ret == 0 && i_toupper(b[strlen(b)-1]) == 'M') { + /* Don't accept M, since it's ambiguous + whether it's MB or minutes. A warning will + be logged later on about this. */ + node->field.value_type = EVENT_FIELD_VALUE_TYPE_STR; + node->ambiguous_unit = TRUE; + break; + } if (ret == 0 && bytes <= INTMAX_MAX) { node->field.value.intmax = (intmax_t) bytes; node->field.value_type = EVENT_FIELD_VALUE_TYPE_INTMAX; diff --git a/src/lib/event-filter-private.h b/src/lib/event-filter-private.h index 5019674ecd..2c80b1d14d 100644 --- a/src/lib/event-filter-private.h +++ b/src/lib/event-filter-private.h @@ -85,6 +85,8 @@ struct event_filter_node { } category; struct event_field field; + bool ambiguous_unit:1; + bool warned_ambiguous_unit:1; bool warned_type_mismatch:1; }; diff --git a/src/lib/event-filter.c b/src/lib/event-filter.c index 79b7d87354..335312f829 100644 --- a/src/lib/event-filter.c +++ b/src/lib/event-filter.c @@ -251,6 +251,8 @@ clone_expr(pool_t pool, struct event_filter_node *old) new->field.value.str = p_strdup(pool, old->field.value.str); new->field.value.intmax = old->field.value.intmax; new->field.value.timeval = old->field.value.timeval; + new->ambiguous_unit = old->ambiguous_unit; + new->warned_ambiguous_unit = old->warned_ambiguous_unit; new->warned_type_mismatch = old->warned_type_mismatch; return new; @@ -601,7 +603,24 @@ event_match_field(struct event *event, struct event_filter_node *node, else return wildcard_match_icase(field->value.str, wanted_field->value.str); case EVENT_FIELD_VALUE_TYPE_INTMAX: - if ((wanted_field->value_type != EVENT_FIELD_VALUE_TYPE_INTMAX) && + if (node->ambiguous_unit) { + if (!node->warned_ambiguous_unit) { + const char *name = event->sending_name; + /* Use i_warning to prevent event filter recursions. */ + i_warning("Event filter matches integer field " + "'%s' with value that has an " + "ambiguous unit '%s'. Please use " + "either 'mins' or 'MB' to specify " + "interval or size respectively. " + "(event=%s, source=%s:%u)", + wanted_field->key, + wanted_field->value.str, + name != NULL ? name : "", + source_filename, source_linenum); + node->warned_ambiguous_unit = TRUE; + } + return FALSE; + } else if ((wanted_field->value_type != EVENT_FIELD_VALUE_TYPE_INTMAX) && (node->type != EVENT_FILTER_NODE_TYPE_EVENT_FIELD_NUMERIC_WILDCARD)) { if (!node->warned_type_mismatch) { const char *name = event->sending_name; diff --git a/src/lib/test-event-filter.c b/src/lib/test-event-filter.c index a1e3718a9e..1d7e71b6be 100644 --- a/src/lib/test-event-filter.c +++ b/src/lib/test-event-filter.c @@ -703,7 +703,6 @@ static void test_event_filter_size_values(void) { "field < 1KB", 1024, FALSE }, { "field > 1KB", 1024, FALSE }, - { "field = 1m", 1024 * 1024, TRUE }, { "field = 1MB", 1024 * 1024, TRUE }, { "field >= 1MB", 1024 * 1024, TRUE }, { "field <= 1MB", 1024 * 1024, TRUE }, @@ -769,11 +768,6 @@ static void test_event_filter_interval_values(void) existing event filtering. */ { "field = -1", -1, TRUE }, - /* Make sure that ambiguous "m" defaults to sizes (as it's the - first parsing-branch). */ - { "field = 1m", 1000, FALSE }, - { "field = 1m", 1024 * 1024, TRUE }, - { "field = 1milliseconds", 1000, TRUE }, { "field = 1millisecs", 1000, TRUE }, { "field = 1mseconds", 1000, TRUE }, @@ -853,6 +847,40 @@ static void test_event_filter_interval_values(void) test_end(); } +static void test_event_filter_ambiguous_units(void) +{ + const char *error; + const struct failure_context failure_ctx = { + .type = LOG_TYPE_DEBUG, + }; + + test_begin("event filter: ambiguous units"); + + struct event_filter *filter = event_filter_create(); + + /* Make sure an ambiguous unit creates a warning. */ + struct event *e_int = event_create(NULL); + event_add_int(e_int, "field", 1000); + test_assert(event_filter_parse("field = 1m", filter, &error) == 0); + test_expect_error_string("Event filter matches integer field 'field' " + "with value that has an ambiguous unit '1m'. " + "Please use either 'mins' or 'MB' to specify " + "interval or size respectively."); + test_assert(!event_filter_match(filter, e_int, &failure_ctx)); + test_expect_no_more_errors(); + event_unref(&e_int); + + /* String values should not be considered for ambiguous units. */ + struct event *e_str = event_create(NULL); + event_add_str(e_str, "field", "1m"); + test_assert(event_filter_parse("field = 1m", filter, &error) == 0); + test_assert(event_filter_match(filter, e_str, &failure_ctx)); + event_unref(&e_str); + + event_filter_unref(&filter); + test_end(); +} + void test_event_filter(void) { test_event_filter_override_parent_fields(); @@ -871,4 +899,5 @@ void test_event_filter(void) test_event_filter_numbers(); test_event_filter_size_values(); test_event_filter_interval_values(); + test_event_filter_ambiguous_units(); }