lib: Warn about ambiguous event filter units

diff --git a/src/lib/event-filter-parser.y b/src/lib/event-filter-parser.y
index d1fa655942db5e0e822f0c514f82bc4744430d38..2c29306ece10422a76f349b6a7999e758795a1b7 100644 (file)
--- a/src/lib/event-filter-parser.y
+++ b/src/lib/event-filter-parser.y
@@ -9,6 +9,8 @@
 %defines
 
 %{
+#include <ctype.h>
+
 #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 <num>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 5019674ecd95f19283c12ab243b4d7a3ae0c4fe9..2c80b1d14dc59223261a4344722ad5a720268db6 100644 (file)
--- 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 79b7d8735421765df7cab790fd9b114576463e03..335312f8295ebcab4f7a162006c27d8d1d3d2a61 100644 (file)
--- 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 a1e3718a9e400edeecff0f6fe841f0476565db9e..1d7e71b6be3e4c878cbaaeaf6eaaacc81fadfcd9 100644 (file)
--- 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();
 }