Alessandro Vesely wrote:
Lloyd Zusman wrote:
Lloyd Zusman <[EMAIL PROTECTED]> writes:
[ ... ]
But now, I have a new challenge: coming up with a patch/enhancement to
the filtering mechanism which will cause a message to be accepted
without any further filtering. In other words, every filtering step
would result in a three-possibility outcome:
1. Reject the message.
2. Pass the message on to the next filtering step.
3. Accept the message without any further filtering.
[...]
Those two points can be developed using the same kind of logic, that is
encoding the filter response in something that is not a valid SMTP response.
For that reason, I would use, say, 'A' (for Accept) rather than '0' as the
leading character. That will lead to uniform syntax that will be nicer to
document whenever more functionalities are added. E.g. a filter may return
(syntax to be refined)
"Accept; 250 accepted unconditionally",
"Drop; 251 the message will be dropped", or
"Var SPAM_GRADE=75%; 200 message accepted with some reservations".
or maybe also
"Header X-blahblah: insert this on rewriting"
We are still not allowing a filter to change the MIME text of the message,
but paving the way to a more significant enhancement.
Shall I provide an alternative patch?
I've spent some hours attempting a patch along those lines, but I need some
feedback before I continue.
The attached patch is not working, I barely checked it compiles.
Relevant decisions are (urgency order):
i) I changed Lloyd's code in order to sort _all_ filters, as if they were in
the same directory. Otherwise, option 3 (Accept) is ambiguous as more filters
may be invoked after it.
ii) Some features can be implemented in cdfilters.C, i.e. using the same
code as a filter would to mark all recipients as delivered, some need to
modify some behaviour in other source files. What feature in which way?
Meanwhile I slightly modified submit2 to smuggle some data from the filter
to it.
iii) Additional variables should perhaps be saved in control files?
iv) would it be feasible/worth to evolve the whitelistening concept
from "two-sets" to "wrt-order" something?
diff -du courier-0.53.1.20060318/courier/cdfilters.C ./cdfilters.C
--- courier-0.53.1.20060318/courier/cdfilters.C 2003-10-11 03:03:08.000000000
+0200
+++ ./cdfilters.C 2006-03-27 18:13:26.641036000 +0200
@@ -17,72 +17,294 @@
#include "mydirent.h"
#include "localstatedir.h"
+
using namespace std;
-static int dofilter(CString,
- const char *,
- unsigned,
- CString (*)(unsigned, void *),
- void *);
+static int dofilter(global_filter&, CString, const char*);
-int run_filter(const char *filename,
- unsigned nmsgids,
- int iswhitelisted,
- CString (*msgidfunc)(unsigned, void *),
- void *funcarg)
+/*
+* POD is needed to work with qsort, so I do a C-like implementation.
+* There are two alternatives to this kind of coding:
+* 1. Do proper C++ coding with c'tor, d'tor and operator<(),
+* then use STL stable sort. This might require newer compiler and
+* an STL library. Is thius a issue?
+* 2. code a variable-length structure (or place the flag as the first
+* char in string). This would be slightly more efficient but more
+* difficult to maintain and/or to migrate to proper C++ coding.
+*/
+
+#include <stdlib.h> // qsort
+
+struct enabled_filter
+{
+ char* name;
+ int all; // alignment
+};
+
+namespace { // anonymous namespace rather than "static", because of "extern"
+ extern "C" int compare_enabled_filter(const void* va, const void *vb)
+ {
+ const enabled_filter* const a = static_cast<const
enabled_filter*>(va);
+ const enabled_filter* const b = static_cast<const
enabled_filter*>(vb);
+ return strcmp(a->name, b->name);
+ }
+}
+
+static enabled_filter *filterlist = NULL;
+static int filterlistsize = 0;
+static int nfilters = 0;
+
+static const int FILTER_LIST_INCREMENT = 8;
+static const char MEMORY_ERROR[] = "432 Out of memory when processing mail
filters.\n";
+static const char OPENDIR_ERROR[] = "432 Out of memory when processing mail
filters.\n";
+
+static void free_filters()
+{
+ if (filterlist != NULL)
+ {
+ for (int n = 0; n < nfilters; n++)
+ {
+ free(filterlist[n].name);
+ }
+ }
+ nfilters = 0;
+}
+
+static int add_filter(const char *filter, bool all)
+{
+char *dupfilter = strdup(filter);
+
+ if (dupfilter == NULL)
+ {
+ cout << MEMORY_ERROR << flush;
+ return (1);
+ }
+
+ if (nfilters >= filterlistsize)
+ {
+ if (filterlist == NULL)
+ {
+ filterlist = (enabled_filter*)
malloc(sizeof(enabled_filter) *
+ FILTER_LIST_INCREMENT);
+ }
+ else
+ {
+ filterlist = (enabled_filter*) realloc(filterlist,
+ sizeof (enabled_filter) *
+ (filterlistsize +
+ FILTER_LIST_INCREMENT));
+ }
+ if (filterlist == NULL)
+ {
+ free(dupfilter);
+ cout << MEMORY_ERROR << flush;
+ return (1);
+ }
+ filterlistsize += FILTER_LIST_INCREMENT;
+ }
+
+ enabled_filter& last = filterlist[nfilters++];
+ last.name = dupfilter;
+ last.all = all;
+ return (0);
+}
+
+
+static int add_filters_from_dir(const char *dirname, bool all)
+{
+int rc = 0;
+DIR *dirp = opendir(dirname);
+
+ if (dirp)
+ {
+ struct dirent *de;
+ while (rc == 0 && (de=readdir(dirp)) != 0)
+ {
+ if (de->d_name[0] == '.')
+ continue;
+
+ rc = add_filter(de->d_name, all);
+ }
+ closedir(dirp);
+ }
+ else if (errno != ENOENT)
+ {
+ cout << OPENDIR_ERROR << flush;
+ rc = 1;
+ }
+
+ return rc;
+}
+
+int global_filter::run_filter(const char *filename)
{
if (nmsgids == 0) return (0);
-DIR *dirp;
-struct dirent *de;
CString sockname;
+int rc;
+ free_filters();
if (!iswhitelisted)
{
- dirp=opendir(FILTERSOCKETDIR);
- while (dirp && (de=readdir(dirp)) != 0)
+ if (add_filters_from_dir(FILTERSOCKETDIR, 0))
+ return (1);
+ }
+
+ if (add_filters_from_dir(ALLFILTERSOCKETDIR, 1))
+ return (1);
+
+ qsort(filterlist, nfilters, sizeof filterlist[0],
compare_enabled_filter);
+
+ for (int n = 0; n < nfilters; ++n)
+ {
+ enabled_filter& filter = filterlist[n];
+ if (filter.all || !iswhitelisted)
{
- if (de->d_name[0] == '.') continue;
+ sockname = filter.all ? ALLFILTERSOCKETDIR :
FILTERSOCKETDIR;
+ sockname += "/";
+ sockname += filter.name;
+ rc = dofilter(*this, sockname, filename);
+ if (rc)
+ {
+ return (rc < 0 ? 0 : rc);
+ }
+ }
+ }
- sockname = FILTERSOCKETDIR "/";
- sockname += de->d_name;
- if (dofilter( sockname,
- filename, nmsgids,
- msgidfunc,
- funcarg))
+ return (0);
+}
+
+static int filter_variable(global_filter& gf, CString& sockname,
afxiopipestream& sockstream)
+{
+int len = sockname.GetLength();
+int equal = sockname.Find('=');
+CString envar;
+int ch;
+
+ if (equal > 0)
+ {
+#if defined SUPPORT_A_SEMICOLON_IN_THE_VARIABLE_VALUE
+ if (equal + 1 < len && sockname[equal + 1] == '\"')
+ /*
+ * check for VAR_NAME="string with semicolon (;) and/or escaped
quote (\")"
+ */
+ {
+ // todo
+ }
+ else
+#endif
+ /*
+ * semicolon ends value, if any
+ */
+ {
+ int semicol = sockname.Find(';');
+ if (semicol > equal)
{
- closedir(dirp);
- return (1);
+ envar = sockname.Left(semicol);
+ sockname = sockname.Right(len - semicol - 1);
+ ch = ';';
+ }
+ else
+ {
+ envar = sockname;
+ sockname = "";
+ ch = 0;
}
}
- if (dirp) closedir(dirp);
+
+ if (envar.GetLength() > 0)
+ {
+ envar += '\0';
+ gf.xtra_env += envar;
+ }
+ }
+ else
+ {
+ clog_msg_start_err();
+ clog_msg_str("invalid filter variable: ");
+ clog_msg_str(sockname);
+ ch = 0;
}
+ return (ch);
+}
- dirp=opendir(ALLFILTERSOCKETDIR);
- while (dirp && (de=readdir(dirp)) != 0)
+static int filter_header(global_filter& gf, CString& sockname,
afxiopipestream& sockstream)
+{
+int ch = EOF;
+CString headvar = sockname;
+ while ((sockname << sockstream) == 0)
{
- if (de->d_name[0] == '.') continue;
+ ch = (sockname.GetLength() > 0 ? sockname[0] : 0);
+ if (!isspace(ch))
+ break;
+ headvar += '\n';
+ headvar += sockname;
+ ch = EOF;
+ }
+
+ if (headvar.GetLength() > 0)
+ {
+ headvar += '\0';
+ gf.xtra_head += headvar;
+ }
+
+ return ch;
+}
- sockname = ALLFILTERSOCKETDIR "/";
- sockname += de->d_name;
- if (dofilter( sockname,
- filename, nmsgids,
- msgidfunc,
- funcarg))
+static int filter_command(global_filter& gf, CString& sockname,
afxiopipestream& sockstream)
+{
+int len = 0;
+const unsigned char *p;
+
+ sockname.TrimLeft();
+ p = (unsigned char const*)(char const*)sockname;
+ if (p)
+ {
+ int ch;
+ while ((ch = *p++) != 0)
{
- closedir(dirp);
- return (1);
+ ++len;
+ if (isspace(ch) || ch == ';')
+ {
+ CString cmd = sockname.Left(--len);
+
+ sockname = (char const *)p;
+ sockname.TrimLeft();
+
+ cmd.MakeLower();
+ if (cmd.Compare("accept") == 0)
+ {
+ ++gf.accept;
+ }
+ else if (cmd.Compare("drop") == 0)
+ {
+ ++gf.drop;
+ }
+ else if (cmd.Compare("var") == 0)
+ {
+ ++gf.var;
+ ch = filter_variable(gf, sockname,
sockstream);
+ }
+ else if (cmd.Compare("header") == 0)
+ {
+ ++gf.header;
+ ch = filter_header(gf, sockname,
sockstream);
+ }
+ else
+ {
+ clog_msg_start_err();
+ clog_msg_str("invalid filter command:
");
+ clog_msg_str(sockname);
+ }
+
+ return ch;
+ }
}
}
- if (dirp) closedir(dirp);
- return (0);
+ return 0;
}
-static int dofilter(CString sockname,
- const char *filename,
- unsigned nmsgids,
- CString (*msgidfunc)(unsigned, void *),
- void *funcarg)
+static int dofilter(global_filter& gf, CString sockname, const char* filename)
{
int s;
struct sockaddr_un ssun;
@@ -152,9 +374,9 @@
unsigned i;
- for (i=0; i<nmsgids; i++)
+ for (i=0; i<gf.nmsgids; i++)
{
- sockname= (*msgidfunc)(i, funcarg);
+ sockname= (*gf.msgidfunc)(i, gf.voidp);
if (sockname.GetLength() == 0)
sockname=" ";
@@ -185,35 +407,62 @@
int d=sockname[0];
- if (isdigit(d))
+ while (!isdigit(d))
{
- if (d != '4' && d != '5')
+ int end = filter_command(gf, sockname, sockstream);
+ if (end == 0 &&
+ (sockname << sockstream) != 0)
+ break;
+ if (end == ';' || end == 0)
+ d = sockname[0];
+ else
{
- while (isdigit(sockname[0]) &&
- isdigit(sockname[1]) &&
- isdigit(sockname[2]) &&
- sockname[3] == '-')
- {
- if ( (sockname << sockstream) != 0)
- break;
- }
- sockstream.close();
- close(s);
- return (0);
+ clog_msg_start_err();
+ clog_msg_str("invalid filter command end: ");
+ clog_msg_str(sockname);
+ break;
}
}
- cout << sockname << "\n";
-
- while (isdigit(sockname[0]) && isdigit(sockname[1]) &&
- isdigit(sockname[2]) && sockname[3] == '-')
+ if (isdigit(d))
{
- if ( (sockname << sockstream) != 0)
- break;
- cout << sockname << "\n";
+ const bool reject = d == '4' || d == '5';
+
+ gf.outmsg = sockname;
+ gf.outmsg += '\n';
+ while (sockname.GetLength() >= 4 &&
+ isdigit(sockname[0]) &&
+ isdigit(sockname[1]) &&
+ isdigit(sockname[2]) &&
+ sockname[3] == '-')
+ {
+ if ( (sockname << sockstream) != 0)
+ break;
+ gf.outmsg += sockname;
+ gf.outmsg += '\n';
+ }
+ if (reject)
+ {
+ cout << gf.outmsg << flush;
+ gf.outmsg = "";
+ }
+ sockstream.close();
+ close(s);
+ return (reject);
}
- cout << flush;
+ else if (sockname.GetLength() > 0)
+ {
+ clog_msg_start_err();
+ clog_msg_str("invalid filter message: ");
+ clog_msg_str(sockname);
+ }
+
+ // we cannot output any possibly sensible stuff from the filter
+ // so we assume that no output from the filter implies std okmsg
+ // cout << sockname << "\n";
+
sockstream.close();
close(s);
- return (1);
+ return (0);
}
+
diff -du courier-0.53.1.20060318/courier/cdfilters.h ./cdfilters.h
--- courier-0.53.1.20060318/courier/cdfilters.h 2000-01-18 04:28:38.000000000
+0100
+++ ./cdfilters.h 2006-03-27 16:47:21.233010000 +0200
@@ -16,15 +16,38 @@
#include "afx/afx.h"
-int run_filter(
- const char *, // Filename containing the message
- unsigned, // How many message IDs (can be >1 if
- // message was split due to too many
- // recipients
- int, // whitelisted flag, used to select
- // filters
- CString (*)(unsigned, void *), // Function to return message id
- // #n (n is 0..count-1)
- void *); // Second argument to function.
+struct global_filter // not a POD
+{
+ // How many message IDs (can be >1 if message was split
+ // due to too many recipients
+ unsigned nmsgids;
+
+ // whitelisted flag, used to select filters
+ int iswhitelisted;
+
+ // Function to return message id #n (n is 0..count-1)
+ CString (*msgidfunc)(unsigned, void *);
+
+ // Second argument to function msgidfunc
+ void * voidp;
+
+
+ // output from filtering: commands
+ int drop, accept, var, header;
+
+ // output from filtering: message string
+ CString outmsg;
+
+ // output from filtering: variables
+ CString xtra_env;
+
+ // output from filtering: headers
+ CString xtra_head;
+
+ // member function to run global filters
+ int run_filter(const char *filename);
+
+ global_filter() : drop(0), accept(0), var(0), outmsg(""), xtra_env("")
{}
+};
#endif
diff -du courier-0.53.1.20060318/courier/submit2.C ./submit2.C
--- courier-0.53.1.20060318/courier/submit2.C 2005-12-13 01:48:50.000000000
+0100
+++ ./submit2.C 2006-03-27 13:28:25.266519000 +0200
@@ -978,13 +978,17 @@
CString dfile=namefile("D", 0);
-SubmitFile *voidp=this;
+global_filter gf;
- if (filter_enabled &&
- run_filter(dfile, num_control_files_created,
- iswhitelisted,
- &SubmitFile::get_msgid_for_filtering, &voidp))
- return (1);
+ if (filter_enabled)
+ {
+ gf.nmsgids = num_control_files_created;
+ gf.iswhitelisted = iswhitelisted;
+ gf.msgidfunc = &SubmitFile::get_msgid_for_filtering;
+ gf.voidp = this;
+ if (gf.run_filter(dfile))
+ return (1);
+ }
if (!mime || strcmp(mime, "none"))
{
@@ -1093,11 +1097,13 @@
if (link(dfile, namefile("D", n)) != 0) clog_msg_errno();
}
-CString okmsg("250 Ok. ");
+ if (gf.outmsg.GetLength() <= 0)
+ gf.outmsg = "250 Ok. ";
- okmsg += basemsgid;
+ if (gf.outmsg[gf.outmsg.GetLength() - 1] == ' ')
+ gf.outmsg += basemsgid;
-int hasxerror=datafilter(dfile, rcptnum, okmsg);
+int hasxerror=datafilter(dfile, rcptnum, gf.outmsg /* todo: pass gf for
xtra_env */);
current_submit_file=0;
if (num_control_files_created == 1)
@@ -1128,7 +1134,7 @@
}
#endif
- cout << okmsg << endl << flush;
+ cout << gf.outmsg << endl << flush;
}
trigger(TRIGGER_NEWMSG);
