access: modify the function to make it build the message to log

this allows better testability and make sure we only log in one place.

diff --git a/include/mlmmj.h b/include/mlmmj.h
index 2aa2a1286304594f29661f883a09418e202912b6..efd32aff8c476d61add9c471e1fe8674e77e958d 100644 (file)
--- a/include/mlmmj.h
+++ b/include/mlmmj.h
@@ -146,7 +146,7 @@ typedef enum {
        ACT_DISCARD,
 } actions_t;
 
-actions_t do_access(strlist *rules, strlist *headers, const char *from, int listfd);
+actions_t do_access(strlist *rules, strlist *headers, const char *from, char **msg);
 
 #define MY_ASSERT(expression) if (!(expression)) { \
                        errno = 0; \

diff --git a/src/access.c b/src/access.c
index ad940e2b740bde38a620c40817433892243e1e9e..dfd317a53f8366ba23cd48461825063c5bd39fd9 100644 (file)
--- a/src/access.c
+++ b/src/access.c
@@ -22,10 +22,10 @@
 
 #include <string.h>
 #include <regex.h>
+#include <errno.h>
 
 #include "mlmmj.h" 
-#include "log_error.h"
-#include "log_oper.h"
+#include "xmalloc.h"
 
 static char *action_strs[] = {
        "allowed",
@@ -36,7 +36,7 @@ static char *action_strs[] = {
 };
 
 actions_t
-do_access(strlist *rules, strlist *headers, const char *from, int listfd)
+do_access(strlist *rules, strlist *headers, const char *from, char **msg)
 {
        unsigned int match;
        char *rule_ptr;
@@ -47,6 +47,8 @@ do_access(strlist *rules, strlist *headers, const char *from, int listfd)
        regex_t regexp;
        char *hdr;
        size_t i = 0;
+       if (msg != NULL)
+               *msg = NULL;
 
        if (rules == NULL)
                return (ACT_ALLOW);
@@ -70,10 +72,8 @@ do_access(strlist *rules, strlist *headers, const char *from, int listfd)
                        act = ACT_DISCARD;
                } else {
                        errno = 0;
-                       log_error(LOG_ARGS, "Unable to parse rule #%d \"%s\":"
-                                       " Missing action keyword. Denying post from \"%s\"",
-                                       i, it->item, from);
-                       log_oper(listfd, OPLOGFNAME, "Unable to parse rule #%d \"%s\":"
+                       if (msg != NULL)
+                               xasprintf(msg, "Unable to parse rule #%d \"%s\":"
                                        " Missing action keyword. Denying post from \"%s\"",
                                        i, it->item, from);
                        return ACT_DENY;
@@ -83,17 +83,16 @@ do_access(strlist *rules, strlist *headers, const char *from, int listfd)
                        rule_ptr++;
                } else if (*rule_ptr == '\0') {
                        /* the rule is a keyword and no regexp */
-                       log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -"
+                       if (msg != NULL)
+                               xasprintf(msg, "access -"
                                        " A mail from \"%s\" was %s by rule #%d \"%s\"",
                                        from, action_strs[act], i, it->item);
                        return act;
                } else {
                        /* we must have space or end of string */
                        errno = 0;
-                       log_error(LOG_ARGS, "Unable to parse rule #%d \"%s\":"
-                                       " Invalid character after action keyword."
-                                       " Denying post from \"%s\"", i, it->item, from);
-                       log_oper(listfd, OPLOGFNAME, "Unable to parse rule #%d \"%s\":"
+                       if (msg != NULL)
+                               xasprintf(msg, "Unable to parse rule #%d \"%s\":"
                                        " Invalid character after action keyword."
                                        " Denying post from \"%s\"", i, it->item, from);
                        return ACT_DENY;
@@ -121,14 +120,11 @@ do_access(strlist *rules, strlist *headers, const char *from, int listfd)
                        regerror(err, &regexp, errbuf, sizeof(errbuf));
                        regfree(&regexp);
                        errno = 0;
-                       log_error(LOG_ARGS, "regcomp() failed for rule #%d \"%s\""
+                       if (msg != NULL)
+                               xasprintf(msg, "regcomp() failed for rule #%d \"%s\""
                                        " (message: '%s') (expression: '%s')"
                                        " Denying post from \"%s\"",
                                        i, it->item, errbuf, rule_ptr, from);
-                       log_oper(listfd, OPLOGFNAME, "regcomp() failed for rule"
-                                       " #%d \"%s\" (message: '%s') (expression: '%s')"
-                                       " Denying post from \"%s\"",
-                                       i, it->item, errbuf, rule_ptr, from);
                        return ACT_DENY;
                }
 
@@ -146,12 +142,14 @@ do_access(strlist *rules, strlist *headers, const char *from, int listfd)
 
                if (match != not) {
                        if (match) {
-                               log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -"
+                               if (msg != NULL)
+                                       xasprintf(msg, "access -"
                                                " A mail from \"%s\" with header \"%s\" was %s by"
                                                " rule #%d \"%s\"", from, hdr, action_strs[act],
                                                i, it->item);
                        } else {
-                               log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -"
+                               if (msg != NULL)
+                                       xasprintf(msg, "access -"
                                                " A mail from \"%s\" was %s by rule #%d \"%s\""
                                                " because no header matched.", from,
                                                action_strs[act], i, it->item);
@@ -161,7 +159,7 @@ do_access(strlist *rules, strlist *headers, const char *from, int listfd)
                i++;
        }
 
-       log_oper(listfd, OPLOGFNAME, "mlmmj-process: access -"
+       xasprintf(msg, "access -"
                        " A mail from \"%s\" didn't match any rules, and"
                        " was denied by default.", from);
        return ACT_DENY;

diff --git a/src/mlmmj-process.c b/src/mlmmj-process.c
index c6ff3e39448aa6428b892fc780b45ec48d8121d5..12f6b7d426c3b5586b5dd1b722669084f7946fa1 100644 (file)
--- a/src/mlmmj-process.c
+++ b/src/mlmmj-process.c
@@ -604,10 +604,14 @@ int main(int argc, char **argv)
        access_rules = ctrlvalues(ml.ctrlfd, "access");
        if (access_rules != NULL) {
                actions_t accret;
+               char *error = NULL;
                /* Don't send a mail about denial to the list, but silently
                 * discard and exit. Also do this in case it's turned off */
                accret = do_access(access_rules, &allheaders,
-                                       posteraddr, ml.fd);
+                                       posteraddr, &error);
+               if (error != NULL) {
+                       log_oper(ml.fd, OPLOGFNAME, "%s", error);
+               }
                if (accret == ACT_DENY) {
                        if ((strcasecmp(ml.addr, posteraddr) == 0) ||
                                    statctrl(ml.ctrlfd, "noaccessdenymails")) {

diff --git a/tests/mlmmj.c b/tests/mlmmj.c
index 09a4a66d8f3c2750c18f9b47e5865930a97ebf69..3c714b41cbe3776641ee170c5aa7d7668a9c1b6a 100644 (file)
--- a/tests/mlmmj.c
+++ b/tests/mlmmj.c
@@ -2926,23 +2926,29 @@ ATF_TC_BODY(parse_content_type, tc)
 
 ATF_TC_BODY(do_access, tc)
 {
-       int listfd = -1;
        strlist rules = tll_init();
        strlist headers = tll_init();
        tll_push_back(rules, "allow");
+       char *msg = NULL;
 
-       ATF_REQUIRE_EQ(do_access(NULL, NULL, "test@plop", listfd), ACT_ALLOW);
-       ATF_REQUIRE_EQ(do_access(&rules, NULL, "test@plop", listfd), ACT_ALLOW);
+       ATF_REQUIRE_EQ(do_access(NULL, NULL, "test@plop", &msg), ACT_ALLOW);
+       ATF_REQUIRE(msg == NULL);
+
+       ATF_REQUIRE_EQ(do_access(&rules, NULL, "test@plop", &msg), ACT_ALLOW);
+       ATF_REQUIRE_STREQ(msg, "access - A mail from \"test@plop\" was allowed by rule #0 \"allow\"");
        tll_pop_back(rules);
        tll_push_back(rules, "deny");
-       ATF_REQUIRE_EQ(do_access(&rules, NULL, "test@plop", listfd), ACT_DENY);
+       ATF_REQUIRE_EQ(do_access(&rules, NULL, "test@plop", &msg), ACT_DENY);
+       ATF_REQUIRE_STREQ(msg, "access - A mail from \"test@plop\" was denied by rule #0 \"deny\"");
        tll_pop_back(rules);
        tll_push_back(rules, "crap");
-       ATF_REQUIRE_EQ(do_access(&rules, NULL, "test@plop", listfd), ACT_DENY);
+       ATF_REQUIRE_EQ(do_access(&rules, NULL, "test@plop", &msg), ACT_DENY);
+       ATF_REQUIRE_STREQ(msg, "Unable to parse rule #0 \"crap\": Missing action keyword. Denying post from \"test@plop\"");
        tll_pop_back(rules);
        tll_push_back(headers, "from: toto@bla");
        tll_push_back(rules, "moderate ^from: toto@");
-       ATF_REQUIRE_EQ(do_access(&rules, &headers, "test@plop", listfd), ACT_MODERATE);
+       ATF_REQUIRE_EQ(do_access(&rules, &headers, "test@plop", &msg), ACT_MODERATE);
+       ATF_REQUIRE_STREQ(msg, "access - A mail from \"test@plop\" with header \"from: toto@bla\" was moderated by rule #0 \"moderate ^from: toto@\"");
 }
 
 ATF_TP_ADD_TCS(tp)