From 1cf40237ffca59c3cc7dfd2e632fa8770fce8716 Mon Sep 17 00:00:00 2001
From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Thu, 25 Jan 2024 04:30:04 +0200
Subject: [PATCH] lib-settings, config: Change base settings to be an empty
 filter

This simplifies the code, and makes it possible for the following commits to
reorder when base settings are applied.
---
 src/config/config-dump-full.c                 | 33 ++++----
 src/lib-master/test-master-service-settings.c | 81 ++++---------------
 src/lib-settings/settings.c                   | 81 ++++++-------------
 3 files changed, 54 insertions(+), 141 deletions(-)

diff --git a/src/config/config-dump-full.c b/src/config/config-dump-full.c
index 9c3c040adb..33030b6f9c 100644
--- a/src/config/config-dump-full.c
+++ b/src/config/config-dump-full.c
@@ -35,19 +35,12 @@
      <32bit big-endian: settings count>
      <NUL-terminated string: key>[settings count]
 
-     <64bit big-endian: base settings size>
-     <NUL-terminated string: error string - if client attempts to access this
-                             settings block, it must fail with this error.
-			     NUL = no error, followed by settings>
-     Repeat until "base settings size" is reached:
-       <32bit big-endian: key index number>
-       [<strlist/boollist key>]
-       <NUL-terminated string: value>
-
      <32bit big-endian: filter count>
      Repeat for "filter count":
        <64bit big-endian: filter settings size>
-       <NUL-terminated string: error string>
+       <NUL-terminated string: error string - if client attempts to access this
+			       settings block, it must fail with this error.
+			       NUL = no error, followed by settings>
        Repeat until "filter settings size" is reached:
          <32bit big-endian: key index number>
 	 [<strlist/boollist key>]
@@ -568,7 +561,20 @@ int config_dump_full(struct config_parsed *config,
 		}
 		ctx.filter_output_count = 0;
 
+		uoff_t filter_count_offset = output->offset;
+		uint32_t filter_count = 0;
+		if (dest != CONFIG_DUMP_FULL_DEST_STDOUT) {
+			o_stream_nsend(output, &filter_count,
+				       sizeof(filter_count));
+		}
+
 		uoff_t blob_size_offset = output->offset;
+		/* Write base settings - add it as an empty filter */
+		ctx.filter_indexes_be32[ctx.filter_output_count] = 0;
+		ctx.filter_offsets_be64[ctx.filter_output_count] =
+			cpu64_to_be(blob_size_offset);
+		ctx.filter_output_count++;
+
 		if (dest != CONFIG_DUMP_FULL_DEST_STDOUT) {
 			o_stream_nsend(output, &blob_size, sizeof(blob_size));
 			o_stream_nsend(output, "", 1); /* no error */
@@ -583,12 +589,7 @@ int config_dump_full(struct config_parsed *config,
 			if (output_blob_size(output, blob_size_offset) < 0)
 				break;
 		}
-		uoff_t filter_count_offset = output->offset;
-		uint32_t filter_count = 0;
-		if (dest != CONFIG_DUMP_FULL_DEST_STDOUT) {
-			o_stream_nsend(output, &filter_count,
-				       sizeof(filter_count));
-		}
+
 		int ret;
 		T_BEGIN {
 			ret = config_dump_full_sections(&ctx, i, info,
diff --git a/src/lib-master/test-master-service-settings.c b/src/lib-master/test-master-service-settings.c
index 0f05fc904e..12f782b9ce 100644
--- a/src/lib-master/test-master-service-settings.c
+++ b/src/lib-master/test-master-service-settings.c
@@ -99,103 +99,56 @@ static const struct {
 	       "\x00\x00\x01\x00"), // settings count
 	  "'setting key' points outside area" },
 
-	/* base settings size is truncated */
-	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x1D" // full size
-	       "\x00\x00\x00\x01" // event filter count
-	       "\x00" // event filter[0]
-	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x0F" // block size
-	       "N\x00" // block name
-	       "\x00\x00\x00\x01" // settings count
-	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00"),
-	  "Area too small when reading size of 'base settings size'" },
-	/* base settings size is zero */
-	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x1E" // full size
-	       "\x00\x00\x00\x01" // event filter count
-	       "\x00" // event filter[0]
-	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x10" // block size
-	       "N\x00" // block name
-	       "\x00\x00\x00\x01" // settings count
-	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x00"), // base settings size
-	  "'base settings error' points outside area" },
-	/* base settings error is not NUL-terminated */
-	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x20" // full size
-	       "\x00\x00\x00\x01" // event filter count
-	       "\x00" // event filter[0]
-	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x12" // block size
-	       "N\x00" // block name
-	       "\x00\x00\x00\x01" // settings count
-	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "E" // base settings error
-	       "\x00"), // trailing garbage so we can have NUL
-	  "'base settings error' points outside area" },
-
 	/* filter count is truncated */
 	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x22" // full size
+	       "\x00\x00\x00\x00\x00\x00\x00\x19" // full size
 	       "\x00\x00\x00\x01" // event filter count
 	       "\x00" // event filter[0]
 	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x14" // block size
+	       "\x00\x00\x00\x00\x00\x00\x00\x0B" // block size
 	       "N\x00" // block name
 	       "\x00\x00\x00\x01" // settings count
 	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "\x00" // base settings error
 	       "\x00\x00\x00"), // filter count
 	  "Area too small when reading uint of 'filter count'" },
 
 	/* filter settings size is truncated */
 	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x2A" // full size
+	       "\x00\x00\x00\x00\x00\x00\x00\x21" // full size
 	       "\x00\x00\x00\x01" // event filter count
 	       "\x00" // event filter[0]
 	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x1B" // block size
+	       "\x00\x00\x00\x00\x00\x00\x00\x12" // block size
 	       "N\x00" // block name
 	       "\x00\x00\x00\x01" // settings count
 	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "\x00" // base settings error
 	       "\x00\x00\x00\x01" // filter count
 	       "\x00\x00\x00\x00\x00\x00\x00"), // filter settings size
 	  "Area too small when reading size of 'filter settings size'" },
 
 	/* filter settings is truncated */
 	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x2B" // full size
+	       "\x00\x00\x00\x00\x00\x00\x00\x22" // full size
 	       "\x00\x00\x00\x01" // event filter count
 	       "\x00" // event filter[0]
 	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x1D" // block size
+	       "\x00\x00\x00\x00\x00\x00\x00\x14" // block size
 	       "N\x00" // block name
 	       "\x00\x00\x00\x01" // settings count
 	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "\x00" // base settings error
 	       "\x00\x00\x00\x01" // filter count
 	       "\x00\x00\x00\x00\x00\x00\x10\x00"), // filter settings size
 	  "'filter settings size' points outside area" },
 	/* filter error is missing */
 	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x38" // full size
+	       "\x00\x00\x00\x00\x00\x00\x00\x2F" // full size
 	       "\x00\x00\x00\x01" // event filter count
 	       "\x00" // event filter[0]
 	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x2A" // block size
+	       "\x00\x00\x00\x00\x00\x00\x00\x21" // block size
 	       "N\x00" // block name
 	       "\x00\x00\x00\x01" // settings count
 	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "\x00" // base settings error
 	       "\x00\x00\x00\x01" // filter count
 	       "\x00\x00\x00\x00\x00\x00\x00\x00" // filter settings size
 	       "\x00\x00\x00\x00" // event filter index
@@ -204,16 +157,14 @@ static const struct {
 	  "'filter error string' points outside area" },
 	/* filter error is not NUL-terminated */
 	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x46" // full size
+	       "\x00\x00\x00\x00\x00\x00\x00\x3D" // full size
 	       "\x00\x00\x00\x01" // event filter count
 	       "\x00" // event filter[0]
 	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x38" // block size
+	       "\x00\x00\x00\x00\x00\x00\x00\x2F" // block size
 	       "master_service\x00" // block name
 	       "\x00\x00\x00\x01" // settings count
 	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "\x00" // base settings error
 	       "\x00\x00\x00\x01" // filter count
 	       "\x00\x00\x00\x00\x00\x00\x00\x01" // filter settings size
 	       "E" // filter error string
@@ -223,16 +174,14 @@ static const struct {
 	  "'filter error string' points outside area" },
 	/* invalid filter string */
 	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x3B" // full size
+	       "\x00\x00\x00\x00\x00\x00\x00\x32" // full size
 	       "\x00\x00\x00\x01" // event filter count
 	       "F\x00" // event filter[0]
 	       "F\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x2B" // block size
+	       "\x00\x00\x00\x00\x00\x00\x00\x22" // block size
 	       "N\x00" // block name
 	       "\x00\x00\x00\x01" // settings count
 	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "\x00" // base settings error
 	       "\x00\x00\x00\x01" // filter count
 	       "\x00\x00\x00\x00\x00\x00\x00\x01" // filter settings size
 	       "\x00" // filter error string
@@ -243,16 +192,14 @@ static const struct {
 
 	/* Duplicate block name */
 	{ DATA("DOVECOT-CONFIG\t1.0\n"
-	       "\x00\x00\x00\x00\x00\x00\x00\x43" // full size
+	       "\x00\x00\x00\x00\x00\x00\x00\x3A" // full size
 	       "\x00\x00\x00\x01" // event filter count
 	       "\x00" // event filter[0]
 	       "\x00" // override event filter[0]
-	       "\x00\x00\x00\x00\x00\x00\x00\x2B" // block size
+	       "\x00\x00\x00\x00\x00\x00\x00\x22" // block size
 	       "N\x00" // block name
 	       "\x00\x00\x00\x01" // settings count
 	       "K\x00" // setting[0] key
-	       "\x00\x00\x00\x00\x00\x00\x00\x01" // base settings size
-	       "\x00" // base settings error
 	       "\x00\x00\x00\x01" // filter count
 	       "\x00\x00\x00\x00\x00\x00\x00\x01" // filter settings size
 	       "\x00" // filter error string
diff --git a/src/lib-settings/settings.c b/src/lib-settings/settings.c
index 849d5a0609..4580967f2c 100644
--- a/src/lib-settings/settings.c
+++ b/src/lib-settings/settings.c
@@ -81,9 +81,6 @@ struct settings_mmap_block {
 	const char *name;
 	size_t block_end_offset;
 
-	const char *error; /* if non-NULL, accessing the block must fail */
-	size_t base_start_offset, base_end_offset;
-
 	uint32_t filter_count;
 	size_t filter_indexes_start_offset;
 	size_t filter_offsets_start_offset;
@@ -374,27 +371,6 @@ settings_block_read(struct settings_mmap *mmap, size_t *_offset,
 			return -1;
 	}
 
-	/* <base settings size> */
-	uint64_t base_settings_size;
-	if (settings_block_read_size(mmap, &offset, block_end_offset,
-				     "base settings size", &base_settings_size,
-				     error_r) < 0)
-		return -1;
-	block->base_end_offset = offset + base_settings_size;
-
-	/* <base settings error string> */
-	if (settings_block_read_str(mmap, &offset,
-				    block->base_end_offset,
-				    "base settings error", &error,
-				    error_r) < 0)
-		return -1;
-	if (error[0] != '\0')
-		block->error = error;
-	block->base_start_offset = offset;
-
-	/* skip over the key-value pairs */
-	offset = block->base_end_offset;
-
 	/* <filter count> */
 	if (settings_block_read_uint32(mmap, &offset, block_end_offset,
 				       "filter count", &block->filter_count,
@@ -756,10 +732,6 @@ settings_mmap_apply(struct settings_apply_ctx *ctx, const char **error_r)
 			ctx->info->name);
 		return -1;
 	}
-	if (block->error != NULL) {
-		*error_r = block->error;
-		return -1;
-	}
 
 	if (!block->settings_validated &&
 	    (ctx->flags & SETTINGS_GET_NO_KEY_VALIDATION) == 0) {
@@ -772,8 +744,9 @@ settings_mmap_apply(struct settings_apply_ctx *ctx, const char **error_r)
 		.type = LOG_TYPE_DEBUG,
 	};
 
-	/* go through the filters in reverse sorted order, so we always set the
-	   setting just once, never overriding anything. */
+	/* So through the filters in reverse sorted order, so we always set the
+	   setting just once, never overriding anything. A filter for the base
+	   settings is expected to always exist. */
 	bool seen_filter = FALSE;
 	for (uint32_t i = block->filter_count; i > 0; ) {
 		i--;
@@ -811,7 +784,8 @@ settings_mmap_apply(struct settings_apply_ctx *ctx, const char **error_r)
 			}
 			filter_offset += strlen(filter_error) + 1;
 
-			if (ctx->filter_name != NULL && !seen_filter) {
+			if (ctx->filter_name != NULL && !seen_filter &&
+			    event_filter != EVENT_FILTER_MATCH_ALWAYS) {
 				bool op_not;
 				const char *value =
 					event_filter_find_field_exact(
@@ -825,15 +799,21 @@ settings_mmap_apply(struct settings_apply_ctx *ctx, const char **error_r)
 					seen_filter = TRUE;
 			}
 
-			if (event_filter != EVENT_FILTER_MATCH_ALWAYS) {
-				int ret = settings_instance_override(ctx,
-						mmap->override_event_filters[event_filter_idx],
-						error_r);
-				if (ret < 0)
-					return -1;
-				if (ret > 0)
-					seen_filter = TRUE;
-			}
+			/* Apply overrides specific to this filter before the
+			   filter settings themselves. For base settings the
+			   filter is EVENT_FILTER_MATCH_ALWAYS, which applies
+			   the rest of the overrides that weren't already
+			   handled. This way global setting overrides don't
+			   override named filters' settings, unless the
+			   override is specifically using the filter name
+			   as prefix. */
+			int ret = settings_instance_override(ctx,
+					mmap->override_event_filters[event_filter_idx],
+					error_r);
+			if (ret < 0)
+				return -1;
+			if (ret > 0)
+				seen_filter = TRUE;
 
 			if (settings_mmap_apply_blob(ctx, block,
 					filter_offset, filter_end_offset,
@@ -841,22 +821,6 @@ settings_mmap_apply(struct settings_apply_ctx *ctx, const char **error_r)
 				return -1;
 		}
 	}
-
-	/* Apply any leftover overrides after filters and
-	   filter overrides were already handled. This way global
-	   setting overrides don't override named filters' settings,
-	   unless the override is specifically using the filter name
-	   as prefix. */
-	int ret = settings_instance_override(ctx, NULL, error_r);
-	if (ret < 0)
-		return -1;
-	if (ret > 0)
-		seen_filter = TRUE;
-
-	/* apply the base settings last after all filters */
-	if (settings_mmap_apply_blob(ctx, block, block->base_start_offset,
-				     block->base_end_offset, error_r) < 0)
-		return -1;
 	return seen_filter ? 1 : 0;
 
 }
@@ -1456,7 +1420,7 @@ settings_instance_override(struct settings_apply_ctx *ctx,
 		/* If we're being called while applying filters, only apply
 		   the overrides that have a matching filter. This preserves
 		   the expected order in which settings are applied. */
-		if (event_filter != NULL && !set->always_match &&
+		if (!set->always_match &&
 		    event_filter != EVENT_FILTER_MATCH_ALWAYS &&
 		    (set->filter_event == NULL ||
 		     !event_filter_match(event_filter, set->filter_event,
@@ -1627,7 +1591,8 @@ settings_instance_get(struct settings_apply_ctx *ctx,
 		}
 	} else {
 		/* No configuration file - apply all overrides */
-		ret = settings_instance_override(ctx, NULL, error_r);
+		ret = settings_instance_override(ctx, EVENT_FILTER_MATCH_ALWAYS,
+						 error_r);
 	}
 	if (ret > 0)
 		seen_filter = TRUE;
-- 
2.47.3