Jared,
Thanks for the cleanup --
I like the patch and I've committed it as r963.
I spent a little bit of time poking at it, and I think this could
also have been done as a rcpt_pre/mail_pre hook -- at least the
blocking portion of it.
-R
Jared Johnson wrote:
>
> 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
> > Jared Johnson wrote:
> >> 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 u...@[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
> >>
>
