On 2006-03-04 09:26:37 +0100, Hanno Hecker wrote:
> I've stumbled across a small problem and I'm not sure how to fix it
> correctly. 
> Whenever a relay client sends an address which is not accepted by
> Qpsmtpd::Address he never gets an error message. The mail is silently
> redirected (in our case the admin's address). Some examples:
> 
> RCPT TO:<[EMAIL PROTECTED]>   
> 250 <>, recipient ok
> RCPT TO:<[EMAIL PROTECTED]>
> 250 <>, recipient ok
> RCPT TO:<[EMAIL PROTECTED] >
> 250 <"\<hah"@uu-x.de>, recipient ok

Hmm, that used to work in older versions of qpsmtpd:

220 XXXXXXX.XXXX.at ESMTP qpsmtpd 0.28 ready; send us your mail, but not
your spam.
[...]
RCPT TO:<[EMAIL PROTECTED]>
501 could not parse recipient
RCPT TO:<[EMAIL PROTECTED]>
501 could not parse recipient
RCPT TO:<[EMAIL PROTECTED] >
501 could not parse recipient

> Sidenote: Qmail and Postfix both accept the trailing space or dot as
> valid recipients from relay clients and the mail is delivered to the
> "correct" recipient address. 

I think that one is a different bug. Qpsmtpd simply splits the line at
whitespace, so the Qpsmtpd::Address gets the address "<[EMAIL PROTECTED]",
which is syntactically incorrect. However, whitespace is allowed at
least in the local part, so the correct command

RCPT TO:<foo\ [EMAIL PROTECTED]>

is treated incorrectly as a delivery attempt to address "<foo\" with an
argument of "[EMAIL PROTECTED]>".

> Some questions (even some are without a "?" :-)):
>  - should Qpsmtpd::Address accept mails with a trailing "."? Like
>    $ host -t MX perl.org.
>    perl.org mail is handled by 5 mx.develooper.com.

RFC 2821 says no.

(But it also says the angle brackets are mandatory and I was the only
one to argue that we should enforce them)

Pragmatically: So far I haven't heard a single complaint that a
legitimate mail was dropped because of the trailing dot. It catches a
(very) small percentage of spam.


>  - Qpsmtpd::SMTP: adjust parsing of mail addresses in rcpt() to the 
>    one in mail(), i.e. 
>    my ($from) = ($from_parameter =~ m/^from:\s*(<[^>]*>)/i)[0];
>    and not
>    my ($rcpt) = ($_[0] =~ m/to:(.*)/i)[0];
>    which causes the weird "rewriting".
>    ... diff attached.

Yup.


>  - Qpsmtpd::Address should be able to tell if it's a parsing error or
>    really an empty address. Then a plugin or Qpsmtpd::SMTP can drop an
>    error in the RCPT phase if it's an parsing error from an MAIL FROM
>    address.

Originally, the Qpsmtpd::Address::parse did return undef if the address
was unparseable:

sub parse {
    my ($class, $line) = @_;
    my $a = $class->canonify($line);
    return ($class->new($a)) if (defined $a);
    return undef;
}

I think this got lost during the "why do we need both new and parse?"
discussion a few months ago. That would explain why it worked with 0.28.

the attached patch should fix it.

        hp

-- 
   _  | Peter J. Holzer    | Ich sehe nun ein, dass Computer wenig
|_|_) | Sysadmin WSR       | geeignet sind, um sich was zu merken.
| |   | [EMAIL PROTECTED]         |
__/   | http://www.hjp.at/ |    -- Holger Lembke in dan-am
--- Address.pm.2006-03-04T19:31:28      2006-01-23 22:40:10.000000000 +0100
+++ Address.pm  2006-03-04 19:31:54.000000000 +0100
@@ -61,6 +61,7 @@
     my $self = {};
     if ($user =~ /^<(.*)>$/ ) {
        ($user, $host) = $class->canonify($user)
+       return undef unless defined $user;
     }
     elsif ( not defined $host ) {
        my $address = $user;

Attachment: pgpxHjwtfjFSV.pgp
Description: PGP signature

Reply via email to