diff options
author | Sergey Poznyakoff <gray@gnu.org.ua> | 2010-06-13 19:22:36 +0300 |
---|---|---|
committer | Sergey Poznyakoff <gray@gnu.org.ua> | 2010-06-13 19:22:36 +0300 |
commit | 91e8e4d57abe109393f8dc4ad31f09b1e65f0ef6 (patch) | |
tree | 3fc2c4d5a57c82a910eabd50099f5e6598844758 | |
parent | a1d5a1b82fa0381fe23ceba98cdd9b23962f02c4 (diff) | |
download | mailfromd-91e8e4d57abe109393f8dc4ad31f09b1e65f0ef6.tar.gz mailfromd-91e8e4d57abe109393f8dc4ad31f09b1e65f0ef6.tar.bz2 |
Warn about risky uses of accept. Provide ways to properly handle them.
* mfd/builtin/mmq.bi: New source.
* builtin/Makefile.am (BI_FILES): Add mmq.bi
* mfd/debug.hin (init_debug_modlist)
(debug_init): New prototypes.
* mfd/engine.c (message_data): Rename hdr to mmq. All uses
updated.
(priv_store_msgmod_closure): Purge the queue if cmd is NULL.
(mlfi_eom): Emit warning if accept is called when the MMQ is
not empty.
* mfd/gram.y: Handle NULL closure.
* mfd/mailfromd.h (env_clear_msgmod): New proto.
* mfd/prog.c (ENVF_MSGMOD): New constant.
(exception_context)<flags>: New member.
(runtime_warning): Rewrite using mu_diag_printf.
(instr_result): Warn user if `accept' is called
after commands that modify message headers or
body.
(env_clear_msgmod): New function.
(env_msgmod): Set ENVF_MSGMOD flag.
* NEWS, doc/mailfromd.texi: Document mmq_purge and changes to accept.
-rw-r--r-- | NEWS | 32 | ||||
-rw-r--r-- | doc/mailfromd.texi | 104 | ||||
-rw-r--r-- | mfd/builtin/Makefile.am | 1 | ||||
-rw-r--r-- | mfd/builtin/mmq.bi | 28 | ||||
-rw-r--r-- | mfd/debug.hin | 3 | ||||
-rw-r--r-- | mfd/engine.c | 35 | ||||
-rw-r--r-- | mfd/gram.y | 7 | ||||
-rw-r--r-- | mfd/mailfromd.h | 1 | ||||
-rw-r--r-- | mfd/prog.c | 31 |
9 files changed, 209 insertions, 33 deletions
@@ -1,4 +1,4 @@ -Mailfromd NEWS -- history of user-visible changes. 2010-06-12 +Mailfromd NEWS -- history of user-visible changes. 2010-06-13 Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Sergey Poznyakoff See the end of file for copying conditions. @@ -214,6 +214,29 @@ Standard error is redirected to the given syslog facility and, if specified, priority. If the latter is omitted, LOG_ERR is assumed. +** Message Modifications and Accept. + +Calling `accept' causes any modifications to the +message (applied by, e.g., header_add and the like) to be ignored. +This is due to requirements of the Milter protocol. + +Previously it would be hard to diagnose and would cause some false bug +reports. Now, calling `accept' after any modifications to the message +results in the following warning message: + + RUNTIME WARNING near /etc/mailfromd.mf:36: `accept' causes previous + message modification commands to be ignored; call mmq_purge() prior + to `accept', to suppress this warning + +It is only warning, the `accept' action itself is, of course, honored. +If you see this diagnostics in your log, do the following: + + - if it was intended, call mmq_purge, as suggested (see below for the + description of this function); + - if it was not, read the manual, subsection `4.12.1.8 Message + Modification Queue', for information on how to handle it. + FIXME: section number is subject to change. + ** MFL functions *** progress @@ -244,6 +267,13 @@ The `debug_spec' function takes an optional numeric argument, which sets the minimum debugging level. The function returns only those debug specifications whose level is greater than or equal to the minimum. +*** mmq_purge + +The function `mmq_purge' purges internal message modification queue. +This undoes the effect of the following functions, if they had been +called previously: rcpt_add, rcpt_delete, header_add, header_insert, +header_delete, header_replace, replbody, quarantine + ** New built-in constant `__git__'. The `__git__' built-in constant is defined for alpha versions only. diff --git a/doc/mailfromd.texi b/doc/mailfromd.texi index 30198bb9..682af925 100644 --- a/doc/mailfromd.texi +++ b/doc/mailfromd.texi @@ -235,6 +235,7 @@ Built-in and Library Functions * Envelope modification functions:: * Header modification functions:: * Body Modification Functions:: +* Message modification queue:: * Mail header functions:: * Mail body functions:: * EOM Functions:: @@ -5509,6 +5510,7 @@ in version @value{VERSION}. * Envelope modification functions:: * Header modification functions:: * Body Modification Functions:: +* Message modification queue:: * Mail header functions:: * Mail body functions:: * EOM Functions:: @@ -6073,7 +6075,7 @@ Checks for hexadecimal digits, i.e. one of @samp{0}, @samp{1}, @node Envelope modification functions @subsubsection Envelope Modification Functions - Envelope modification functions add or delete recipient addresses +Envelope modification functions add or delete recipient addresses from the message envelope. This allows @acronym{MFL} scripts to redirect messages to another addresses. @@ -6085,16 +6087,16 @@ Add the e-mail @var{address} to the envelope. Remove @var{address} from the envelope. @end deftypefn - The following example code implements a simple alias-like -capability: + The following example code uses these functions to implement a +simple alias-like capability: @smallexample prog envrcpt do - string alias dbget(%aliasdb, $1, "NULL", 1) - if %alias != "NULL" + string alias dbget(aliasdb, $1, "NULL", 1) + if alias != "NULL" rcpt_delete($1) - rcpt_add(%alias) + rcpt_add(alias) fi done @end smallexample @@ -6172,7 +6174,7 @@ instance to replace. @subsubsection Body Modification Functions Body modification is an experimental feature of @acronym{MFL}. -Version @value{VERSION} provides only one function for that purpose. +The version @value{VERSION} provides only one function for that purpose. @deftypefn {Built-in Function} void replbody (string @var{text}) Replace the body of the message with @var{text}. Notice, that @@ -6188,6 +6190,90 @@ Example: No restrictions are imposed on the format of @var{text}. @end deftypefn +@node Message modification queue +@subsubsection Message Modification Queue +@cindex message modification queue + Message modification functions described in the previous subsections +do not take effect immediately, in the moment they are called. +Instead they store the requested changes in the internal @dfn{message +modification queue}. These changes are applied at the end of +processing, before @samp{eom} stage finishes +(@pxref{milter-control-flow}). + + One important consequence of this way of operation is that calling +any @acronym{MTA} action (@pxref{Actions}), causes all prior +modifications to the message to be ignored. That is because after +receiving the action command, @acronym{MTA} will not call filter +for that message any more. In particular, the @samp{eom} handler will +not be called, and the message modification queue will not be flushed. +While it is logical for such actions as @code{reject} or +@code{tempfail}, it may be quite confusing for @code{accept}. +Consider, for example, the following code: + +@smallexample +@group +prog envfrom +do + if $1 == "" + header_add("X-Filter", "foo") + accept + fi +done +@end group +@end smallexample + + Obviously, the intention was to add a @samp{X-Filter} header and +accept the message if it was sent from the null address. What happens +in reality, however, is a bit different: the message is accepted, but +no header is added to it. If you need to accept the message and +retain any modifications you have done to it, you need to use an +auxiliary variable, e.g.: + +@smallexample +@group +number accepted 0 +prog envfrom +do + if $1 == "" + header_add("X-Filter", "foo") + set accepted 1 + fi +done +@end group +@end smallexample + + Then, test this variable for non-zero value at the beginning of each +subsequent handler, e.g.: + +@smallexample +prog data +do + if accepted + continue + fi + ... +done +@end smallexample + + To help you trace such problematic usages of @code{accept}, +@command{mailfromd} emits the following warning: + +@smallexample +RUNTIME WARNING near /etc/mailfromd.mf:36: `accept' causes previous +header modification commands to be ignored; call mmq_purge() prior +to `accept', to suppress this warning +@end smallexample + + If it is OK to lose all modifications, call @code{mmq_purge}, as +suggested in this message. + +@deftypefn {Built-in Function} void mmq_drop () + Remove all modification requests from the queue. This function undoes +the effect of any of the following functions, if they had been called +previously: @code{rcpt_add}, @code{rcpt_delete}, @code{header_add}, +@code{header_insert}, @code{header_delete}, @code{header_replace}, +@code{replbody}, @code{quarantine}. +@end deftypefn @node Mail header functions @subsubsection Mail Header Functions @@ -7136,7 +7222,7 @@ machine is located. If @command{mailfromd} is compiled without @smallexample m4_ifdef(`WITH_GEOIP',` - add "X-Originator-Country" geoip_country_code_by_addr($client_addr) + header_add("X-Originator-Country", geoip_country_code_by_addr($client_addr)) ') @end smallexample @@ -10488,7 +10574,7 @@ argument must be a literal string (not a variable or expression). Secondly, there is no way to select a particular header instance to delete or replace, which may be necessary to properly handle multiple headers (e.g. @samp{Received}). For more elaborate ways of -header modifications, @ref{Header modification functions}. +header modifications, see @ref{Header modification functions}. @node Assignments @subsection Variable Assignments diff --git a/mfd/builtin/Makefile.am b/mfd/builtin/Makefile.am index c217f90e..d0091751 100644 --- a/mfd/builtin/Makefile.am +++ b/mfd/builtin/Makefile.am @@ -34,6 +34,7 @@ BI_FILES=\ macro.bi\ mail.bi\ mbox.bi\ + mmq.bi\ msg.bi\ mudebug.bi\ poll.bi\ diff --git a/mfd/builtin/mmq.bi b/mfd/builtin/mmq.bi new file mode 100644 index 00000000..0eec6bb4 --- /dev/null +++ b/mfd/builtin/mmq.bi @@ -0,0 +1,28 @@ +/* This file is part of Mailfromd. -*- c -*- + Copyright (C) 2010 Sergey Poznyakoff + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3, or (at your option) + any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Clear the list of message modification requests. This function undoes + the effect of the following functions, if they had been called + previously: rcpt_add, rcpt_delete, header_add, header_insert, + header_delete, header_replace, replbody, quarantine */ + +MF_DEFUN(mmq_purge, VOID) +{ + env_clear_msgmod(env); +} +END + +MF_INIT diff --git a/mfd/debug.hin b/mfd/debug.hin index e5ff587b..d8a9f8fa 100644 --- a/mfd/debug.hin +++ b/mfd/debug.hin @@ -16,6 +16,9 @@ MAKEDEBUGSYM(SRCLIST) +void init_debug_modlist(void); + +void debug_init(char **names); int debug_level_p(int modn, int level); void debug_enable_module(const char *file, int level); int debug_module_level(const char *modname, int *plev); diff --git a/mfd/engine.c b/mfd/engine.c index 7a9219b8..2e3fa0b1 100644 --- a/mfd/engine.c +++ b/mfd/engine.c @@ -49,7 +49,7 @@ static void ctx_msgmod(void *data, struct msgmod_closure *cmd); struct message_data { eval_environ_t env; /* Evaluation environment */ - mu_list_t hdr; /* List of header modifications */ + mu_list_t mmq; /* Message Modification Queue */ char *helostr; /* Domain name obtained in HELO phase */ char msgid[64]; /* Message ID */ }; @@ -60,7 +60,7 @@ test_message_data_init(eval_environ_t env) { test_message_data = xmalloc(sizeof(*test_message_data)); test_message_data->env = env; - test_message_data->hdr = NULL; + test_message_data->mmq = NULL; test_message_data->helostr = NULL; test_message_data->msgid[0] = 0; } @@ -89,7 +89,7 @@ priv_get(SMFICTX *ctx) ctx_msgmod, ctx); clear_rcpt_count(md->env); - md->hdr = NULL; + md->mmq = NULL; md->helostr = NULL; md->msgid[0] = 0; gacopyz_setpriv(ctx, md); @@ -150,14 +150,21 @@ priv_store_msgmod_closure(SMFICTX *ctx, struct msgmod_closure *cmd) struct message_data *md = priv_get(ctx); if (!md) return 1; - if (!md->hdr) { - mu_list_create(&md->hdr); - mu_list_set_destroy_item(md->hdr, destroy_msgmod_closure); + + if (!cmd) { + debug(10, "clearing msgmod list"); + mu_list_destroy(&md->mmq); + return 0; + } + + if (!md->mmq) { + mu_list_create(&md->mmq); + mu_list_set_destroy_item(md->mmq, destroy_msgmod_closure); } debug4(10, "adding msgmod_closure: %s \"%s\" %s %u", msgmod_opcode_str(cmd->opcode), cmd->name, SP(cmd->value), cmd->idx); - mu_list_append(md->hdr, cmd); + mu_list_append(md->mmq, cmd); return 0; } @@ -360,7 +367,7 @@ filter_cleanup(SMFICTX *ctx) xeval(md->env, smtp_state_end); free(md->helostr); destroy_environment(md->env); - mu_list_destroy(&md->hdr); + mu_list_destroy(&md->mmq); free(md); gacopyz_setpriv(ctx, NULL); } @@ -731,14 +738,14 @@ mlfi_eom(SMFICTX *ctx) env_make_frame(md->env); status = mlfi_eval(ctx, smtp_state_eom); env_leave_frame(md->env, 0); - if ((status == SMFIS_ACCEPT || status == SMFIS_CONTINUE) && md->hdr) { - debug(50,"executing header commands"); - mu_list_do(md->hdr, run_msgmod, ctx); + if ((status == SMFIS_ACCEPT || status == SMFIS_CONTINUE) && md->mmq) { + debug(50,"flushing message modification queue"); + mu_list_do(md->mmq, run_msgmod, ctx); } mf_proctitle_format("%sfinished", md->msgid); clear_rcpt_count(md->env); - mu_list_destroy(&md->hdr); + mu_list_destroy(&md->mmq); return status; } @@ -750,8 +757,8 @@ mlfi_abort(SMFICTX *ctx) debug(70, "Abort"); mf_proctitle_format("%saborting", md->msgid); - mu_list_destroy(&md->hdr); - md->hdr = NULL; + mu_list_destroy(&md->mmq); + md->mmq = NULL; md->msgid[0] = 0; /* Note: the value of helostr is not discarded: RFC 2822, section 4.1.1.5 states that RSET issued immediately @@ -3186,8 +3186,11 @@ dbg_setreply(void *data, char *code, char *xcode, char *message) static void dbg_msgmod(void *data, struct msgmod_closure *clos) { - printf("%s %s: %s %u\n", msgmod_opcode_str(clos->opcode), - clos->name, SP(clos->value), clos->idx); + if (!clos) + printf("clearing msgmod list\n"); + else + printf("%s %s: %s %u\n", msgmod_opcode_str(clos->opcode), + clos->name, SP(clos->value), clos->idx); } static const char * diff --git a/mfd/mailfromd.h b/mfd/mailfromd.h index 558c765d..14e1f53f 100644 --- a/mfd/mailfromd.h +++ b/mfd/mailfromd.h @@ -964,6 +964,7 @@ int env_capture_write_args(eval_environ_t env, ...); void env_final_gc(eval_environ_t env); void env_msgmod(eval_environ_t env, enum msgmod_opcode opcode, const char *name, const char *value, unsigned idx); +void env_clear_msgmod(eval_environ_t env); void capture_on(void); const char *env_get_macro(eval_environ_t env, const char *symbol); @@ -222,6 +222,8 @@ disable_prog_trace(const char *modlist) See comment to env_fixup_autos, below. */ #define MAX_AUTO_PTR 128 +#define ENVF_MSGMOD 0x01 /* message modofication instruction has been used */ + struct exception_context { prog_counter_t pc; prog_counter_t tos; @@ -261,6 +263,8 @@ struct eval_environ { int (*setreply)(void *data, char *code, char *xcode, char *message); void (*msgmod)(void *data, struct msgmod_closure *c); + int flags; + /* Regular expression matching */ regmatch_t *matches; /* Match map */ size_t matchcount; /* Number of used entries in matches */ @@ -518,16 +522,14 @@ runtime_stack_trace(eval_environ_t env) void runtime_warning(eval_environ_t env, const char *fmt, ...) { - int n; va_list ap; - char buf[512]; - n = snprintf(buf, sizeof buf, _("RUNTIME WARNING near %s:%lu: "), - env->locus.file, (unsigned long) env->locus.line); + mu_diag_printf(MU_DIAG_WARNING, + _("RUNTIME WARNING near %s:%lu: "), + env->locus.file, (unsigned long) env->locus.line); va_start(ap, fmt); - vsnprintf(buf + n, sizeof buf - n, fmt, ap); + mu_diag_output(MU_DIAG_WARNING, fmt, ap); va_end(ap); - mu_error(buf); } void @@ -1502,7 +1504,12 @@ instr_result(eval_environ_t env) code = NULL; if (xcode[0] == 0) xcode = NULL; - + + if (status == SMFIS_ACCEPT && (env->flags & ENVF_MSGMOD)) + runtime_warning(env, _("`accept' causes previous message " + "modification commands to be ignored; " + "call mmq_purge() prior to `accept', " + "to suppress this warning")); env->status = status; env->setreply(env->data, code, xcode, message); advance_pc(env, 1); @@ -2312,6 +2319,15 @@ env_capture_write_args(eval_environ_t env, ...) } void +env_clear_msgmod(eval_environ_t env) +{ + if (PROG_TRACE_ENGINE) + prog_trace(env, "Clearing message modification queue"); + env->msgmod(env->data, NULL); + env->flags &= ~ENVF_MSGMOD; +} + +void env_msgmod(eval_environ_t env, enum msgmod_opcode opcode, const char *name, const char *value, unsigned idx) { @@ -2326,6 +2342,7 @@ env_msgmod(eval_environ_t env, enum msgmod_opcode opcode, cp->value = value ? xstrdup(value) : NULL; cp->idx = idx; env->msgmod(env->data, cp); + env->flags |= ENVF_MSGMOD; } |