Merge pull request #5051 from fatalbanana/fix_ratelimit_tests

[Test] Fix ratelimit tests

5 files changed, 99 insertions, 72 deletions

diff --git a/src/libserver/cfg_utils.cxx b/src/libserver/cfg_utils.cxx
index dd85eddfb..1344bc4f9 100644
--- a/src/libserver/cfg_utils.cxx
+++ b/src/libserver/cfg_utils.cxx
@@ -57,7 +57,7 @@
 #ifdef HAVE_SYS_RESOURCE_H
 #include <sys/resource.h>
 #endif
-#include <math.h>
+#include <cmath>
 #include "libserver/composites/composites.h"
 
 #include "blas-config.h"
@@ -69,6 +69,7 @@
 #include "cxx/util.hxx"
 #include "frozen/unordered_map.h"
 #include "frozen/string.h"
+#include "contrib/expected/expected.hpp"
 #include "contrib/ankerl/unordered_dense.h"
 
 #define DEFAULT_SCORE 10.0
@@ -135,14 +136,14 @@ struct rspamd_actions_list {
 	void sort()
 	{
 		std::sort(actions.begin(), actions.end(), [](const action_ptr &a1, const action_ptr &a2) -> bool {
-			if (!isnan(a1->threshold) && !isnan(a2->threshold)) {
+			if (!std::isnan(a1->threshold) && !std::isnan(a2->threshold)) {
 				return a1->threshold < a2->threshold;
 			}
 
-			if (isnan(a1->threshold) && isnan(a2->threshold)) {
+			if (std::isnan(a1->threshold) && std::isnan(a2->threshold)) {
 				return false;
 			}
-			else if (isnan(a1->threshold)) {
+			else if (std::isnan(a1->threshold)) {
 				return true;
 			}
 
@@ -826,8 +827,7 @@ gboolean
 rspamd_config_post_load(struct rspamd_config *cfg,
 						enum rspamd_post_load_options opts)
 {
-
-	auto ret = TRUE;
+	auto ret = tl::expected<void, std::string>{};
 
 	rspamd_adjust_clocks_resolution(cfg);
 	rspamd_logger_configure_modules(cfg->debug_modules);
@@ -867,14 +867,15 @@ rspamd_config_post_load(struct rspamd_config *cfg,
 			else {
 				if (opts & RSPAMD_CONFIG_INIT_VALIDATE) {
 					msg_err_config("no url_tld option has been specified");
-					ret = FALSE;
+					ret = tl::make_unexpected(std::string{"no url_tld option has been specified"});
 				}
 			}
 		}
 		else {
 			if (access(cfg->tld_file, R_OK) == -1) {
 				if (opts & RSPAMD_CONFIG_INIT_VALIDATE) {
-					ret = FALSE;
+					ret = tl::make_unexpected(fmt::format("cannot access tld file {}: {}",
+														  cfg->tld_file, strerror(errno)));
 					msg_err_config("cannot access tld file %s: %s", cfg->tld_file,
 								   strerror(errno));
 				}
@@ -905,13 +906,17 @@ rspamd_config_post_load(struct rspamd_config *cfg,
 	if (!rspamd_config_parse_log_format(cfg)) {
 		msg_err_config("cannot parse log format, task logging will not be available");
 		if (opts & RSPAMD_CONFIG_INIT_VALIDATE) {
-			ret = FALSE;
+			ret = tl::make_unexpected(std::string{"cannot parse log format"});
 		}
 	}
 
 	if (opts & RSPAMD_CONFIG_INIT_SYMCACHE) {
 		/* Init config cache */
-		ret = rspamd_symcache_init(cfg->cache) && ret;
+		auto symcache_ret = rspamd_symcache_init(cfg->cache);
+
+		if (!symcache_ret) {
+			ret = tl::make_unexpected(std::string{"symcache init failed"});
+		}
 
 		/* Init re cache */
 		rspamd_re_cache_init(cfg->re_cache, cfg);
@@ -928,9 +933,9 @@ rspamd_config_post_load(struct rspamd_config *cfg,
 
 	if (opts & RSPAMD_CONFIG_INIT_LIBS) {
 		/* Config other libraries */
-		ret = rspamd_config_libs(cfg->libs_ctx, cfg) && ret;
+		auto libs_ret = rspamd_config_libs(cfg->libs_ctx, cfg);
 
-		if (!ret) {
+		if (!libs_ret) {
 			msg_err_config("cannot configure libraries, fatal error");
 			return FALSE;
 		}
@@ -959,7 +964,11 @@ rspamd_config_post_load(struct rspamd_config *cfg,
 							" Rspamd features will be broken");
 		}
 
-		ret = rspamd_symcache_validate(cfg->cache, cfg, FALSE) && ret;
+		auto val_ret = rspamd_symcache_validate(cfg->cache, cfg, FALSE);
+
+		if (!val_ret) {
+			ret = tl::make_unexpected(std::string{"symcache validation failed"});
+		}
 	}
 
 	if (opts & RSPAMD_CONFIG_INIT_POST_LOAD_LUA) {
@@ -970,7 +979,14 @@ rspamd_config_post_load(struct rspamd_config *cfg,
 		rspamd_map_preload(cfg);
 	}
 
-	return ret;
+	if (ret) {
+		return true;
+	}
+	else {
+		msg_err_config("error on post-init stage: %s", ret.error().c_str());
+
+		return false;
+	}
 }
 
 struct rspamd_classifier_config *
@@ -1525,7 +1541,7 @@ rspamd_config_new_symbol(struct rspamd_config *cfg, const char *symbol,
 		rspamd_mempool_alloc0_type(cfg->cfg_pool, struct rspamd_symbol);
 	score_ptr = rspamd_mempool_alloc_type(cfg->cfg_pool, double);
 
-	if (isnan(score)) {
+	if (std::isnan(score)) {
 		/* In fact, it could be defined later */
 		msg_debug_config("score is not defined for symbol %s, set it to zero",
 						 symbol);
@@ -1636,7 +1652,7 @@ rspamd_config_add_symbol(struct rspamd_config *cfg,
 		}
 
 		if (sym_def->priority > priority &&
-			(isnan(score) || !(sym_def->flags & RSPAMD_SYMBOL_FLAG_UNSCORED))) {
+			(std::isnan(score) || !(sym_def->flags & RSPAMD_SYMBOL_FLAG_UNSCORED))) {
 			msg_debug_config("symbol %s has been already registered with "
 							 "priority %ud, do not override (new priority: %ud)",
 							 symbol,
@@ -1657,7 +1673,7 @@ rspamd_config_add_symbol(struct rspamd_config *cfg,
 		}
 		else {
 
-			if (!isnan(score)) {
+			if (!std::isnan(score)) {
 				msg_debug_config("symbol %s has been already registered with "
 								 "priority %ud, override it with new priority: %ud, "
 								 "old score: %.2f, new score: %.2f",
@@ -1997,7 +2013,7 @@ rspamd_config_action_from_ucl(struct rspamd_config *cfg,
 
 	/* TODO: add lua references support */
 
-	if (isnan(threshold) && !(flags & RSPAMD_ACTION_NO_THRESHOLD)) {
+	if (std::isnan(threshold) && !(flags & RSPAMD_ACTION_NO_THRESHOLD)) {
 		msg_err_config("action %s has no threshold being set and it is not"
 					   " a no threshold action",
 					   act->name);
diff --git a/src/rspamd.c b/src/rspamd.c
index 117f3b995..b6c361cb2 100644
--- a/src/rspamd.c
+++ b/src/rspamd.c
@@ -979,12 +979,16 @@ load_rspamd_config(struct rspamd_main *rspamd_main,
 
 		if (init_modules) {
 			if (!rspamd_init_filters(cfg, reload, false)) {
+				msg_err_main("init filters failed");
+
 				return FALSE;
 			}
 		}
 
 		/* Do post-load actions */
 		if (!rspamd_config_post_load(cfg, opts)) {
+			msg_err_main("post load failed");
+
 			return FALSE;
 		}
 	}
diff --git a/test/functional/cases/500_ratelimit.conf b/test/functional/cases/500_ratelimit.conf
deleted file mode 100644
index 425df34be..000000000
--- a/test/functional/cases/500_ratelimit.conf
+++ /dev/null
@@ -1,44 +0,0 @@
-*** Settings ***
-Suite Setup     Rspamd Setup
-Suite Teardown  Rspamd Teardown
-Library         ${RSPAMD_TESTDIR}/lib/rspamd.py
-Resource        ${RSPAMD_TESTDIR}/lib/rspamd.robot
-Variables       ${RSPAMD_TESTDIR}/lib/vars.py
-
-*** Variables ***
-${CONFIG}               ${RSPAMD_TESTDIR}/configs/ratelimit.conf
-${MESSAGE}              ${RSPAMD_TESTDIR}/messages/ham.eml
-${RSPAMD_SCOPE}         Suite
-${SETTINGS_REPLIES}     {symbols_enabled = [RATELIMIT_CHECK, RATELIMIT_UPDATE]}
-
-*** Keywords ***
-Recipient Test
-  [Arguments]  ${from}  ${rcpt}  ${howmany}
-  FOR  ${index}  IN RANGE  ${howmany}
-    Scan File  ${HAM_MESSAGE}
-    ...  From=${from}
-    ...  IP=1.1.1.1
-    ...  Settings=${SETTINGS_RATELIMIT}
-    ...  Rcpt=${rcpt}
-    Expect Action  no action
-  END
-  Scan File  ${HAM_MESSAGE}
-  ...  From=${from}
-  ...  IP=1.1.1.1
-  ...  Settings=${SETTINGS_RATELIMIT}
-  ...  Rcpt=${rcpt}
-  Expect Action  soft reject
-  Sleep 1s
-  Scan File  ${HAM_MESSAGE}
-  ...  From=${from}
-  ...  IP=1.1.1.1
-  ...  Settings=${SETTINGS_RATELIMIT}
-  ...  Rcpt=${rcpt}
-  Expect Action  no action
-
-*** Test Cases ***
-CHECK BASIC
-  Recipient Test  ${EMPTY}  foobar@example.net  4
-
-CHECK SELECTOR
-  Recipient Test  foo@example.net  special@example.net  2
diff --git a/test/functional/cases/500_ratelimit.robot b/test/functional/cases/500_ratelimit.robot
new file mode 100644
index 000000000..85e7b353c
--- /dev/null
+++ b/test/functional/cases/500_ratelimit.robot
@@ -0,0 +1,48 @@
+*** Settings ***
+Suite Setup     Rspamd Redis Setup
+Suite Teardown  Rspamd Redis Teardown
+Library         ${RSPAMD_TESTDIR}/lib/rspamd.py
+Resource        ${RSPAMD_TESTDIR}/lib/rspamd.robot
+Variables       ${RSPAMD_TESTDIR}/lib/vars.py
+
+*** Variables ***
+${CONFIG}               ${RSPAMD_TESTDIR}/configs/ratelimit.conf
+${MESSAGE}              ${RSPAMD_TESTDIR}/messages/zip.eml
+${REDIS_SCOPE}          Suite
+${RSPAMD_SCOPE}         Suite
+${RSPAMD_URL_TLD}       ${RSPAMD_TESTDIR}/../lua/unit/test_tld.dat
+${SETTINGS_RATELIMIT}   {symbols_enabled = [RATELIMIT_CHECK, RATELIMIT_UPDATE]}
+
+*** Keywords ***
+Basic Test Scan
+  [Arguments]  ${from}  ${rcpt}
+  Scan File  ${MESSAGE}
+  ...  From=${from}
+  ...  IP=1.1.1.1
+  ...  Settings=${SETTINGS_RATELIMIT}
+  ...  Rcpt=${rcpt}
+
+Basic Test
+  [Arguments]  ${from}  ${rcpt}  ${howmany}
+  # Should be able to send up to burst
+  FOR  ${index}  IN RANGE  ${howmany}
+    Basic Test Scan  ${from}  ${rcpt}
+    Expect Action  no action
+  END
+  # Should then be ratelimited
+  Basic Test Scan  ${from}  ${rcpt}
+  Expect Action  soft reject
+  # Should be able to send 1 message 1 second later
+  Sleep  1s
+  Basic Test Scan  ${from}  ${rcpt}
+  Expect Action  no action
+  # Ratelimited again
+  Basic Test Scan  ${from}  ${rcpt}
+  Expect Action  soft reject
+
+*** Test Cases ***
+RATELIMIT CHECK BUILTIN
+  Basic Test  ${EMPTY}  foobar@example.net  4
+
+RATELIMIT CHECK SELECTOR
+  Basic Test  foo@example.net  special@example.net  2
diff --git a/test/functional/configs/ratelimit.conf b/test/functional/configs/ratelimit.conf
index ab0c44b42..36bae88fe 100644
--- a/test/functional/configs/ratelimit.conf
+++ b/test/functional/configs/ratelimit.conf
@@ -1,20 +1,23 @@
 .include(duplicate=append,priority=0) "{= env.TESTDIR =}/configs/plugins.conf"
 
-lua = "{= env.LUA_SCRIPT =}";
 redis {
   servers = "{= env.REDIS_ADDR =}:{= env.REDIS_PORT =}";
 }
 
 ratelimit {
-  bounce_to_ip {
-    bucket {
-      burst = 4;
-      rate = "1 / 1s";
+  rates {
+    bounce_to_ip {
+      bucket {
+        burst = 4;
+        rate = "1 / 1s";
+      }
+    }
+    to_selector_xxx {
+      selector = "id('special');to.in('special@example.net')";
+      bucket {
+        burst = 2;
+        rate = "1 / 1s";
+      }
     }
-  }
-  to_selector {
-    selector = "to:in{special@example.net}";
-    burst = 2;
-    rate = "1 / 1s";
   }
 }