PATCH: allow regular expressions for address matching to be easily overridden

[Jared Johnson]

Hi,

I recently asked this list about 'user@[IP]' syntax and reversed myself
shortly afterward, realizing that this syntax is RFC-valid and, as
another list member pointed out, necessary for backward compatibility.
However, after finding that most of our clients' MTA's were
(incorrectly) rejecting these messages, we decided to modify our copy of
Qpsmtpd to reject the syntax as well. Now I would like to revert this
fork. I've attached a patch to allow easy overriding of the various
expressions used to parse addresses in Qpsmtpd::Address::canonify().
This makes it so that one can do, for instance:

$Qpsmtpd::Address::domain_expr =
'(?:[a-zA-Z0-9](?:[-a-zA-Z0-9]*[a-zA-Z0-9])?)(?:.(?:[a-zA-Z0-9](?:[-a-zA-Z0-9]*[a-zA-Z0-9])?))*';

from hook_pre_connection in order to override the $domain regular
expression that was previously local to canonify() and prevent [IP]
syntax from being accepted. I actually special-cased $domain_expr and
$address_literal so that:

undef $Qpsmtpd::Address::address_literal;

could act as a bit of a shortcut to the exact same override. Changing
$domain alone would have been enough, but it seemed appropriate to lend
the same flexibility to the rest of the regexp's used in canonify()

Hope this is useful!

-Jared

[Robin Bowes]

Jared Johnson wrote:
> $Qpsmtpd::Address::domain_expr =
> '(?:[a-zA-Z0-9](?:[-a-zA-Z0-9]*[a-zA-Z0-9])?)(?:.(?:[a-zA-Z0-9](?:[-a-zA-Z0-9]*[a-zA-Z0-9])?))*';

Any reason this, and most/all the other regex definitions are not qr{...} ?

R.

[Jared Johnson]

Robin Bowes wrote:
> Any reason this, and most/all the other regex definitions are not qr{...} ?

Good idea! Attached a new patch. Tested with normal address syntax and
user@[IP] syntax, with $Qpsmtpd::Address::address_literal_expr set and
unset... it might need some closer review for any other cases of syntax
that I don't really understand.

This patch also includes a showstopper fix I made in testing that didn't
make into my first patch:

- my ($localpart, $domainpart) = ($path =~ /^(.*)\@(${domain_expr})$/);
+ my ($localpart, $domainpart) = ($path =~ /^(.*)\@(${domain})$/);

Enjoy!

-Jared

[Jared Johnson]

Good counterpoint...

Attached a patch with <= 80 char lines and no qr//. Note that I also
changed $atom_expr:

- '[a-zA-Z0-9!#\$\%\&\x27\*\+\x2D\/=\?\^_`{\|}~]+';
+ '[a-zA-Z0-9!#%&*+=?^_`{|}~\$\x27\x2D\/]+';

In my testing of the addition of qr// I confirmed that a lot of the
escaping in the original string was not needed in qr//, and thought that
was an advantage of using qr//; it turns out the escaping was just never
needed at all.

-Jared

Robert Spier wrote:
>
> There's no point to use qr here, as these are mostly strings which are
> substituted into other regular expressions.
>
> Also, please try and avoid lines over 80 characters.
>
> -R