From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Thu, 12 Jan 2023 10:46:07 +0000 (+0200)
Subject: lib: If key already exists, event_add_str(value=NULL) should clear the key
X-Git-Tag: 2.4.0~3098
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=25067ccac7b74a963855d299330bfd3986b750fc;p=thirdparty%2Fdovecot%2Fcore.git

lib: If key already exists, event_add_str(value=NULL) should clear the key

This seems like a more correct logic than not doing anything with NULL
values. It shouldn't affect any of the existing code though.
---

diff --git a/src/lib/lib-event.c b/src/lib/lib-event.c
index 3744bf03c8..4059bbbf13 100644
--- a/src/lib/lib-event.c
+++ b/src/lib/lib-event.c
@@ -1053,8 +1053,12 @@ event_add_str(struct event *event, const char *key, const char *value)
 	struct event_field *field;
 
 	if (value == NULL) {
-		/* silently ignoring is perhaps better than assert-crashing? */
-		return event;
+		/* Silently ignoring is perhaps better than assert-crashing?
+		   However, if the field already exists, this should be the
+		   same as event_field_clear() */
+		if (event_find_field_recursive(event, key) == NULL)
+			return event;
+		value = "";
 	}
 
 	field = event_get_field(event, key, TRUE);
diff --git a/src/lib/test-lib-event.c b/src/lib/test-lib-event.c
index cfc4106377..22beb664eb 100644
--- a/src/lib/test-lib-event.c
+++ b/src/lib/test-lib-event.c
@@ -1,6 +1,47 @@
 /* Copyright (c) 2021 Dovecot authors, see the included COPYING file */
 
 #include "test-lib.h"
+#include "array.h"
+
+static void test_event_fields(void)
+{
+	test_begin("event fields");
+	struct event *event = event_create(NULL);
+	struct event_field *field;
+
+	event_add_str(event, "key", NULL);
+	test_assert(event_find_field_nonrecursive(event, "key") == NULL);
+
+	event_add_str(event, "key", "value1");
+	field = event_find_field_nonrecursive(event, "key");
+	test_assert(field != NULL && field->value_type == EVENT_FIELD_VALUE_TYPE_STR &&
+		    strcmp(field->value.str, "value1") == 0);
+
+	event_add_str(event, "key", NULL);
+	field = event_find_field_nonrecursive(event, "key");
+	test_assert(field != NULL && field->value_type == EVENT_FIELD_VALUE_TYPE_STR &&
+		    field->value.str[0] == '\0');
+
+	event_add_int(event, "key", -1234);
+	field = event_find_field_nonrecursive(event, "key");
+	test_assert(field != NULL && field->value_type == EVENT_FIELD_VALUE_TYPE_INTMAX &&
+		    field->value.intmax == -1234);
+
+	struct timeval tv = { .tv_sec = 123456789, .tv_usec = 654321 };
+	event_add_timeval(event, "key", &tv);
+	field = event_find_field_nonrecursive(event, "key");
+	test_assert(field != NULL && field->value_type == EVENT_FIELD_VALUE_TYPE_TIMEVAL &&
+		    field->value.timeval.tv_sec == tv.tv_sec &&
+		    field->value.timeval.tv_usec == tv.tv_usec);
+
+	event_strlist_append(event, "key", "strlist1");
+	field = event_find_field_nonrecursive(event, "key");
+	test_assert(field != NULL && field->value_type == EVENT_FIELD_VALUE_TYPE_STRLIST &&
+		    strcmp(array_idx_elem(&field->value.strlist, 0), "strlist1") == 0);
+
+	event_unref(&event);
+	test_end();
+}
 
 static void test_event_strlist(void)
 {
@@ -41,6 +82,7 @@ static void test_lib_event_reason_code(void)
 
 void test_lib_event(void)
 {
+	test_event_fields();
 	test_event_strlist();
 	test_lib_event_reason_code();
 }