Re: [exim-dev] Refactor optional MAIL args processing

Top Page

Reply to this message
Author: Todd Lyons
To: Jeremy Harris, exim-dev, Phil Pennock
Subject: Re: [exim-dev] Refactor optional MAIL args processing
Sorry for the HTML email, but I wanted to retain long lines.

On Thu, Mar 29, 2012 at 5:26 PM, Phil Pennock <pdp@???> wrote:
>> 2) Does it result in sane looking code?
> I'm almost asleep and haven't applied the patch, but what I get mentally
> from reading the diff looks somewhat sane.

Ok, it ran for almost a week, and then someone reported an error that
looked very much like my code was to blame, so I backed it out. Today I
tracked it down, and it was not the changes I made, instead the error is
due to exim's strict adherence to RFC 5321. The reported error was similar

501 <todd@???> BODY=8BITMIME: malformed address: BODY=8BITMIME may
not follow <todd@???>

You can't see it but there is a trailing tab in the string
(both of them). Exim appears to conform exactly to the RFC which says that
a space must separate the email address from optional arguments. Quoting
RFC 5321:

In some commands and replies, arguments are required following the verb or
reply code. Some commands do not accept arguments (after the verb), and
some reply codes are followed, sometimes optionally, by free form text. In
both cases, where text appears, it is separated from the verb or reply code
by a space character.
MAIL FROM:<reverse-path> [SP <mail-parameters> ] <CRLF>

The function extract_option() in smtp_in.c parses the MAIL FROM commandline
for optional arguments, stores them in various variables, then finally
returns just the email address, which is then extracted using
parse_extract_address(). It accepts any whitespace (!isspace()) as it
traverses the string looking for optional arguments, but it accepts only a
space character as the separator between them in a later check. The
following is valid:


And this is invalid according to exim:


I did some testing and every other MTA I can find accepts a TAB as a valid
separator, exim seems to be the only one who doesn't allow it. A quick fix
doesn't appear to be too intrusive, but there may be better ways of solving
it. This is in the extract_option() function. The line numbers will be
off since I have some other modifications in this file:

diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c
index 6d8d5c7..0e7d8bf 100644
--- a/src/src/smtp_in.c
+++ b/src/src/smtp_in.c
@@ -1001,7 +1001,8 @@ if (*v != '=') return FALSE;
n = v;
while(isalpha(n[-1])) n--;

-if (n[-1] != ' ') return FALSE;
+if (n[-1] == '\t') n[-1] = ' ';
+else if (n[-1] != ' ') return FALSE;

n[-1] = 0;
*name = n;

Is this something that would be considered? This is relaxing adherence to
an RFC to make it more comparable to other MTA's, all others that I tested
(gmail, qmail, sendmail, postfix). IMHO this is so minor that making a
global setting in exim to control it is a waste. But relaxing RFC
compliance is no light matter either.

I have not tested this against the test suite, I still have not
successfully gotten it to run yet.

Always code as if the guy who ends up maintaining your code will be a
violent psychopath who knows where you live. -- Martin Golding