On Fri, 17 Mar 2006 14:07:35 -0800
Ask Bjørn Hansen <[EMAIL PROTECTED]> wrote:
> On Mar 16, 2006, at 10:38 PM, Hanno Hecker (via RT) wrote:
>
> I'd like to see a patch that makes a better API for reading the
> command and the command parameters. The default can still be having
> it space separated and provided in @_, but the hacks in mail() and
> rcpt() to read the rest aren't very nice so we should have an API to
> make it neater.
>
> Maybe just have a method to give the full unmodified command and use
> that, I don't know.
Ok, this is a first (draft of a ;->) patch. The Qpsmtpd::Command
contents can easily be moved to Qpsmtpd::SMTP if desired.
This patch adds some hooks:
* (helo|ehlo|mail|rcpt|auth|unrecognized_command)_parse:
i.e. all commands, which accept arguments (except VRFY).
these hooks get no arguments except
$self,$transaction/$connection
They MUST return(OK, \&ref_to_parsing_sub); to be the parser
for this command (in fact any return code from Constants.pm
which stops any other following plugin is ok).
The ref_to_parsing_sub() gets $self, $cmd, $line as arguments.
$self is here a Qpsmtpd::Command, which inherits from
Qpsmtpd::SMTP, so the usual $self->log(LOGDEBUG, "bla") stuff
works.
$cmd is the name of the command (i.e. the chars up to the first
space in the line from the client), $line the rest of the line.
This sub MUST return ($parsing_ok, $first, @param), like
the default
return($cmd, split(/ +/, $line));
The parse_addr_withhelo plugin enforces strict RFC 821 parsing,
which does not allow RCPT/MAIL parameters (a quick'n'dirty demo
plugin :-))
* (mail|rcpt)_pre
these hooks get $self,$transaction,$addr. $addr is the $first
from above.
Here the 'dont_require_angelbrackets' plugin adds the "<",">"
if not present to establish the current (i.e. without this patch)
behaviour of accepting addresses without the <>.
This (for example) can also be used to strip a trailing dot from an
address, if wanted
Docs will follow later if it's finished
> Also: Tests, please! It's pretty simple to add tests for "is the
> parsing working right".
ok, done.... sometimes it fails (but never from telnet to port 2525)
this seems to be a strange thing in 'make test'. Does 'make test' run
the commands in the same order every time?
> To add to the bikeshedding, I don't think it should be configurable
> -- it'd be fun to require them and see how it goes. If it doesn't
> work well, then just make them optional again and a rcpt_pre plugin
> can make them required for those who want that. Options in the core
> are bad, generally speaking.
hmm, ok, now we have a plugin which does exactly that... don't load ->
no addresses without brackets are accepted ;-)
Hanno
diff -Nurx.svn ../0.3x/config.sample/plugins ./config.sample/plugins
--- ../0.3x/config.sample/plugins 2006-01-13 07:28:49.000000000 +0100
+++ ./config.sample/plugins 2006-03-18 19:35:28.000000000 +0100
@@ -12,6 +12,13 @@
# from one IP!
hosts_allow
+# enable to accept MAIL FROM:/RCPT TO: addresses without surrounding <>
+# dont_require_angelbrackets
+
+# enable to reject MAIL FROM:/RCPT TO: parameters if client helo was HELO
+# (strict RFC 821)... this is not used in EHLO ...
+# parse_addr_withhelo
+
quit_fortune
check_earlytalker
diff -Nurx.svn ../0.3x/lib/Apache/Qpsmtpd.pm ./lib/Apache/Qpsmtpd.pm
--- ../0.3x/lib/Apache/Qpsmtpd.pm 2005-11-20 10:52:17.000000000 +0100
+++ ./lib/Apache/Qpsmtpd.pm 2006-03-18 19:30:04.000000000 +0100
@@ -131,8 +131,10 @@
while (defined(my $data = $self->getline)) {
$data =~ s/\r?\n$//s; # advanced chomp
$self->log(LOGDEBUG, "dispatching $data");
- defined $self->dispatch(split / +/, $data)
- or $self->respond(502, "command unrecognized: '$data'");
+ $data =~ s/^\s*//;
+ my ($cmd, $line) = split / +/, $data, 2);
+ defined $self->dispatch($cmd, $line)
+ or $self->respond(502, "command unrecognized: '$cmd'");
last if $self->{_quitting};
}
}
diff -Nurx.svn ../0.3x/lib/Qpsmtpd/Command.pm ./lib/Qpsmtpd/Command.pm
--- ../0.3x/lib/Qpsmtpd/Command.pm 1970-01-01 01:00:00.000000000 +0100
+++ ./lib/Qpsmtpd/Command.pm 2006-03-18 18:29:45.000000000 +0100
@@ -0,0 +1,84 @@
+package Qpsmtpd::Command;
+# use Qpsmtpd::SMTP;
+use Qpsmtpd::Constants;
+use vars qw(@ISA);
[EMAIL PROTECTED] = qw(Qpsmtpd::SMTP);
+use strict;
+
+sub parse {
+ my ($me,$cmd,$line,$sub) = @_;
+ my $self = {};
+ bless $self, $me;
+ $cmd = lc $1;
+ if ($sub and (ref($sub) eq 'CODE')) {
+ my @ret = eval { $sub->($self, $cmd, $line); };
+ if ($@) {
+ $self->log(LOGERROR, "Failed to parse command [$cmd]: $@");
+ return (undef, $line, ());
+ }
+ my @log = @ret;
+ for (@log) {
+ $_ ||= "";
+ }
+ $self->log(LOGDEBUG, "parse($cmd) => [".join("], [", @log)."]");
+ return @ret;
+ }
+ my $parse = "parse_$cmd";
+ if ($self->can($parse)) {
+ # print "CMD=$cmd,line=$line\n";
+ my @out = eval { $self->$parse($cmd, $line); };
+ if ($@) {
+ $self->log(LOGERROR, "XXX: $@");
+ return(undef, $line, ());
+ }
+ return @out;
+ }
+ return($cmd, split(/ +/, $line)); # default :)
+}
+
+sub parse_rcpt {
+ my ($self,$cmd,$line) = @_;
+ return undef unless $line =~ s/^to:\s*//i;
+ return &_get_mail_params($cmd, $line);
+}
+
+sub parse_mail {
+ my ($self,$cmd,$line) = @_;
+ return undef unless $line =~ s/^from:\s*//i;
+ return &_get_mail_params($cmd, $line);
+}
+
+sub _get_mail_params {
+ my ($cmd,$line) = @_;
+ my @params = ();
+ $line =~ s/\s*$//;
+ # start stripping parameters from the end, so the last thing not a
+ # parameter must be an address...
+ # see also RFC 1869
+ # while ($line =~ s/ +([A-Za-z0-9][A-Za-z0-9\-]*)=([^= \x00-\x1f]+)$//) {
+ while ($line =~ s/\s+([A-Za-z0-9][A-Za-z0-9\-]*=[^= \x00-\x1f]+)$//) {
+ push @params, $1;
+ }
+
+ # the above will "fail" on something like
+ # MAIL FROM: [EMAIL PROTECTED] FOO=BAR
+ # (not: MAIL FROM:[EMAIL PROTECTED] FOO=bar,
+ # MAIL FROM: <user=example.net> FOO=bar)
+ # so let's see if $line contains nothing and treat the first value
+ # as address:
+ unless ($line) {
+ $line = pop(@params) || "<>"; # treat 'MAIL FROM:' as 'MAIL FROM:<>'
+ }
+
+ # now there's just an address in $line, no spaces around it
+ # ...ready to feed Qpsmtpd::Address...
+
+ #### XXX: No: let this do a plugin, so it's not up to us to decide
+ #### if we require <> around an address :-)
+ # unless ($line =~ /^<.*>$/) {
+ # $line = "<".$line.">";
+ # }
+ return ($cmd, $line, @params);
+}
+
+1;
diff -Nurx.svn ../0.3x/lib/Qpsmtpd/Plugin.pm ./lib/Qpsmtpd/Plugin.pm
--- ../0.3x/lib/Qpsmtpd/Plugin.pm 2006-03-04 08:36:21.000000000 +0100
+++ ./lib/Qpsmtpd/Plugin.pm 2006-03-18 14:03:26.000000000 +0100
@@ -4,11 +4,12 @@
# more or less in the order they will fire
our @hooks = qw(
- logging config pre-connection connect ehlo helo
- auth auth-plain auth-login auth-cram-md5
- rcpt mail data data_post queue_pre queue queue_post
+ logging config pre-connection connect ehlo_parse ehlo helo_parse helo
+ auth_parse auth auth-plain auth-login auth-cram-md5
+ rcpt_parse rcpt_pre rcpt mail_parse mail mail_pre
+ data data_post queue_pre queue queue_post
quit reset_transaction disconnect post-connection
- unrecognized_command deny ok
+ unrecognized_command_parse unrecognized_command deny ok
);
our %hooks = map { $_ => 1 } @hooks;
diff -Nurx.svn ../0.3x/lib/Qpsmtpd/SMTP.pm ./lib/Qpsmtpd/SMTP.pm
--- ../0.3x/lib/Qpsmtpd/SMTP.pm 2006-03-07 14:59:22.000000000 +0100
+++ ./lib/Qpsmtpd/SMTP.pm 2006-03-18 19:27:23.000000000 +0100
@@ -12,6 +12,7 @@
use Qpsmtpd::Constants;
use Qpsmtpd::Auth;
use Qpsmtpd::Address ();
+use Qpsmtpd::Command;
use Mail::Header ();
#use Data::Dumper;
@@ -47,11 +48,15 @@
sub dispatch {
my $self = shift;
my ($cmd) = lc shift;
+ my $line = shift;
$self->{_counter}++;
if ($cmd !~ /^(\w{1,12})$/ or !exists $self->{_commands}->{$1}) {
- my ($rc, $msg) = $self->run_hooks("unrecognized_command", $cmd, @_);
+ my ($rc, $msg) = $self->run_hooks("unrecognized_command_parse");
+ my ($ok, $cmd, @params) = Qpsmtpd::Command->parse($cmd, $line, $msg);
+
+ ($rc, $msg) = $self->run_hooks("unrecognized_command", $cmd, @params);
if ($rc == DENY_DISCONNECT) {
$self->respond(521, $msg);
$self->disconnect;
@@ -70,7 +75,7 @@
$cmd = $1;
if (1 or $self->{_commands}->{$cmd} and $self->can($cmd)) {
- my ($result) = eval { $self->$cmd(@_) };
+ my ($result) = eval { $self->$cmd($line) };
$self->log(LOGERROR, "XX: $@") if $@;
return $result if defined $result;
return $self->fault("command '$cmd' failed unexpectedly");
@@ -140,13 +145,15 @@
sub helo {
- my ($self, $hello_host, @stuff) = @_;
+ my ($self, $line) = @_;
+ my ($rc, $msg) = $self->run_hooks('helo_parse');
+ my ($ok, $hello_host, @stuff) = Qpsmtpd::Command->parse('helo', $line, $msg);
return $self->respond (501,
"helo requires domain/address - see RFC-2821 4.1.1.1") unless $hello_host;
my $conn = $self->connection;
return $self->respond (503, "but you already said HELO ...") if $conn->hello;
- my ($rc, $msg) = $self->run_hooks("helo", $hello_host, @stuff);
+ ($rc, $msg) = $self->run_hooks("helo", $hello_host, @stuff);
if ($rc == DONE) {
# do nothing
} elsif ($rc == DENY) {
@@ -168,13 +175,15 @@
}
sub ehlo {
- my ($self, $hello_host, @stuff) = @_;
+ my ($self, $line) = @_;
+ my ($rc, $msg) = $self->run_hooks('ehlo_parse');
+ my ($ok, $hello_host, @stuff) = Qpsmtpd::Command->parse('ehlo', $line, $msg);
return $self->respond (501,
"ehlo requires domain/address - see RFC-2821 4.1.1.1") unless $hello_host;
my $conn = $self->connection;
return $self->respond (503, "but you already said HELO ...") if $conn->hello;
- my ($rc, $msg) = $self->run_hooks("ehlo", $hello_host, @stuff);
+ ($rc, $msg) = $self->run_hooks("ehlo", $hello_host, @stuff);
if ($rc == DONE) {
# do nothing
} elsif ($rc == DENY) {
@@ -226,8 +235,10 @@
}
sub auth {
- my ( $self, $arg, @stuff ) = @_;
-
+ my ( $self, $line ) = @_;
+ my ($rc, $sub) = $self->run_hooks('auth_parse');
+ my ($ok, $arg, @stuff) = Qpsmtpd::Command->parse('auth', $line, $sub);
+
#they AUTH'd once already
return $self->respond( 503, "but you already said AUTH ..." )
if ( defined $self->{_auth}
@@ -239,9 +250,7 @@
}
sub mail {
- my $self = shift;
- return $self->respond(501, "syntax error in parameters") if !$_[0] or $_[0]
!~ m/^from:/i;
-
+ my ($self,$line) = @_;
# -> from RFC2821
# The MAIL command (or the obsolete SEND, SOML, or SAML commands)
# begins a mail transaction. Once started, a mail transaction
@@ -266,16 +275,29 @@
return $self->respond(503, "please say hello first ...");
}
else {
- my $from_parameter = join " ", @_;
- $self->log(LOGINFO, "full from_parameter: $from_parameter");
-
- my ($from) = ($from_parameter =~ m/^from:\s*(<[^>]*>)/i)[0];
-
- # support addresses without <> ... maybe we shouldn't?
- ($from) = "<" . ($from_parameter =~ m/^from:\s*(\S+)/i)[0] . ">"
- unless $from;
-
+ $self->log(LOGINFO, "full from_parameter: $line");
+ my ($rc, $msg) = $self->run_hooks("mail_parse");
+ my ($ok,$from,@params) = Qpsmtpd::Command->parse('mail', $line, $msg);
+ return $self->respond(501, "syntax error in parameters")
+ unless ($ok and $from);
+ my %param;
+ foreach (@params) {
+ my ($k,$v) = split /=/, $_, 2;
+ $param{lc $k} = $v;
+ }
+ # to support addresses without <> we now require a plugin
+ # hooking "mail_pre" to
+ # return (OK, "<$from>");
+ # or anything else parseable by Qpsmtpd::Address ;->
+ # see also comment in sub rcpt()
+ ($rc, $msg) = $self->run_hooks("mail_pre", $from);
+ # $self->log(LOGALERT, "RC=$rc, MSG=$msg");
+ if ($rc == OK) {
+ $from = $msg;
+ }
$self->log(LOGALERT, "from email address : [$from]");
+ return $self->respond(501, "could not parse your mail from command")
+ unless $from =~ /^<.*>$/;
if ($from eq "<>" or $from =~ m/\[undefined\]/ or $from eq "<[EMAIL
PROTECTED]>") {
$from = Qpsmtpd::Address->new("<>");
@@ -284,8 +306,8 @@
$from = (Qpsmtpd::Address->parse($from))[0];
}
return $self->respond(501, "could not parse your mail from command")
unless $from;
-
- my ($rc, $msg) = $self->run_hooks("mail", $from);
+
+ ($rc, $msg) = $self->run_hooks("mail", $from, %param);
if ($rc == DONE) {
return 1;
}
@@ -320,18 +342,39 @@
}
sub rcpt {
- my $self = shift;
- return $self->respond(501, "syntax error in parameters") unless $_[0] and
$_[0] =~ m/^to:/i;
+ my ($self,$line) = @_;
+ my ($rc, $msg) = $self->run_hooks("rcpt_parse");
+ my ($ok, $rcpt, @param) = Qpsmtpd::Command->parse("rcpt", $line, $msg);
+ return $self->respond(501, "syntax error in parameters")
+ unless ($ok and $rcpt);
return $self->respond(503, "Use MAIL before RCPT") unless
$self->transaction->sender;
- my ($rcpt) = ($_[0] =~ m/to:(.*)/i)[0];
- $rcpt = $_[1] unless $rcpt;
+ my %param;
+ foreach (@param) {
+ my ($k,$v) = split /=/, $_, 2;
+ $param{lc $k} = $v;
+ }
+ # to support addresses without <> we now require a plugin
+ # hooking "rcpt_pre" to
+ # return (OK, "<$rcpt>");
+ # or anything else parseable by Qpsmtpd::Address ;->
+ # this means, a plugin can decide to (pre-)accept
+ # addresses like <[EMAIL PROTECTED]> or <[EMAIL PROTECTED] >
+ # by removing the trailing "."/" " from this example...
+ ($rc, $msg) = $self->run_hooks("rcpt_pre", $rcpt);
+ if ($rc == OK) {
+ $rcpt = $msg;
+ }
$self->log(LOGALERT, "to email address : [$rcpt]");
+ return $self->respond(501, "could not parse recipient")
+ unless $rcpt =~ /^<.*>$/;
+
$rcpt = (Qpsmtpd::Address->parse($rcpt))[0];
- return $self->respond(501, "could not parse recipient") unless $rcpt;
+ return $self->respond(501, "could not parse recipient")
+ unless ($rcpt || $rcpt->format eq '<>');
- my ($rc, $msg) = $self->run_hooks("rcpt", $rcpt);
+ ($rc, $msg) = $self->run_hooks("rcpt", $rcpt, %param);
if ($rc == DONE) {
return 1;
}
diff -Nurx.svn ../0.3x/lib/Qpsmtpd/SelectServer.pm ./lib/Qpsmtpd/SelectServer.pm
--- ../0.3x/lib/Qpsmtpd/SelectServer.pm 2005-11-20 10:52:18.000000000 +0100
+++ ./lib/Qpsmtpd/SelectServer.pm 2006-03-18 19:30:26.000000000 +0100
@@ -121,8 +121,10 @@
}
else {
$qp->log(LOGINFO, "dispatching $req");
- defined $qp->dispatch(split / +/, $req)
- or $qp->respond(502, "command unrecognized: '$req'");
+ $req =~ s/^\s*//;
+ my ($cmd,$line) = split / +/, $req, 2);
+ defined $qp->dispatch($cmd, $line)
+ or $qp->respond(502, "command unrecognized: '$cmd'");
}
}
delete $ready{$client};
diff -Nurx.svn ../0.3x/lib/Qpsmtpd/TcpServer.pm ./lib/Qpsmtpd/TcpServer.pm
--- ../0.3x/lib/Qpsmtpd/TcpServer.pm 2006-01-13 07:28:50.000000000 +0100
+++ ./lib/Qpsmtpd/TcpServer.pm 2006-03-18 14:26:24.000000000 +0100
@@ -63,8 +63,10 @@
$_ =~ s/\r?\n$//s; # advanced chomp
$self->log(LOGDEBUG, "dispatching $_");
$self->connection->notes('original_string', $_);
- defined $self->dispatch(split / +/, $_)
- or $self->respond(502, "command unrecognized: '$_'");
+ $_ =~ s/^\s*//;
+ my ($cmd, $line) = split /\s+/, $_, 2;
+ defined $self->dispatch($cmd, $line)
+ or $self->respond(502, "command unrecognized: '$cmd'");
alarm $timeout;
}
alarm(0);
diff -Nurx.svn ../0.3x/plugins/dont_require_angelbrackets
./plugins/dont_require_angelbrackets
--- ../0.3x/plugins/dont_require_angelbrackets 1970-01-01 01:00:00.000000000
+0100
+++ ./plugins/dont_require_angelbrackets 2006-03-18 19:33:32.000000000
+0100
@@ -0,0 +1,19 @@
+#
+# dont_require_angelbrackets - accept addresses in MAIL FROM:/RCPT TO:
+# commands without surrounding <>
+#
+sub hook_mail_pre {
+ my ($self,$transaction, $addr) = @_;
+ unless ($addr =~ /^<.*>$/) {
+ $addr = "<".$addr.">";
+ }
+ return (OK, $addr);
+}
+
+sub hook_rcpt_pre {
+ my ($self,$transaction, $addr) = @_;
+ unless ($addr =~ /^<.*>$/) {
+ $addr = "<".$addr.">";
+ }
+ return (OK, $addr);
+}
diff -Nurx.svn ../0.3x/plugins/parse_addr_withhelo ./plugins/parse_addr_withhelo
--- ../0.3x/plugins/parse_addr_withhelo 1970-01-01 01:00:00.000000000 +0100
+++ ./plugins/parse_addr_withhelo 2006-03-18 19:01:31.000000000 +0100
@@ -0,0 +1,56 @@
+# parse_addr_withhelo
+#
+# strict RFC 821 forbids parameters after the
+# MAIL FROM:<[EMAIL PROTECTED]>
+# and
+# RCPT TO:<[EMAIL PROTECTED]>
+#
+# load this plugin to enforce, else the default EHLO parsing with
+# parameters is done.
+#
+
+sub hook_mail_parse {
+ my $self = shift;
+ if ($self->qp->connection->hello eq 'helo') {
+ return (OK, \&_parse);
+ }
+ return (DECLINED);
+}
+
+sub hook_rcpt_parse {
+ my $self = shift;
+ if ($self->qp->connection->hello eq 'helo') {
+ return (OK, \&_parse);
+ }
+ return (DECLINED);
+}
+
+sub _parse {
+ my ($self,$cmd,$line) = @_;
+ $self->log(LOGERROR, "_parse() cmd=[$cmd], line=[$line]");
+ if ($cmd eq 'mail') {
+ return(undef, $line, ())
+ unless ($line =~ s/^from://i);
+ }
+ else { # cmd eq 'rcpt' ;->
+ return(undef, $line, ())
+ unless ($line =~ s/^to://i);
+ }
+ if ($line =~ s/^\s*(<.*>)//) {
+ my $addr = $1;
+ if ($line =~ /^\s*\S+.*$/) {
+ return (undef, $line, ());
+ }
+ else {
+ return ($cmd, $addr, ());
+ }
+ }
+ elsif ($line =~ /[EMAIL PROTECTED]/) {
+ return (undef, $line, ());
+ }
+ else {
+ $line =~ s/^\s*//;
+ $line =~ s/\s*$//;
+ return ($cmd, $line, ());
+ }
+}
diff -Nurx.svn ../0.3x/t/addresses.t ./t/addresses.t
--- ../0.3x/t/addresses.t 2005-11-20 10:52:17.000000000 +0100
+++ ./t/addresses.t 2006-03-18 19:20:08.000000000 +0100
@@ -12,19 +12,26 @@
is(($smtpd->command('MAIL FROM:<ask @perl.org>'))[0], 250, 'MAIL FROM:<ask
@perl.org>');
is($smtpd->transaction->sender->address, 'ask @perl.org', 'got the right
sender');
-is(($smtpd->command('MAIL FROM:[EMAIL PROTECTED]'))[0], 250, 'MAIL FROM:[EMAIL
PROTECTED]');
-is($smtpd->transaction->sender->format, '<[EMAIL PROTECTED]>', 'got the right
sender');
+is(($smtpd->command('MAIL FROM:[EMAIL PROTECTED]'))[0], 501, 'MAIL FROM:[EMAIL
PROTECTED]');
+# is($smtpd->transaction->sender->format, '<[EMAIL PROTECTED]>', 'got the
right sender');
my $command = 'MAIL FROM:<[EMAIL PROTECTED]> SIZE=1230';
is(($smtpd->command($command))[0], 250, $command);
is($smtpd->transaction->sender->format, '<[EMAIL PROTECTED]>', 'got the right
sender');
+my $command = 'MAIL FROM:<[EMAIL PROTECTED]> SIZE=1230 INCORRECT';
+is(($smtpd->command($command))[0], 503, $command);
+
$command = 'MAIL FROM:<>';
is(($smtpd->command($command))[0], 250, $command);
is($smtpd->transaction->sender->format, '<>', 'got the right sender');
-$command = 'MAIL FROM:<[EMAIL PROTECTED]> SIZE=1230';
+$command = 'MAIL FROM:<[EMAIL PROTECTED]> SIZE=1230 FOO=12345';
is(($smtpd->command($command))[0], 250, $command);
is($smtpd->transaction->sender->format, '<[EMAIL PROTECTED]>', 'got the right
sender');
-
-
+$smtpd->connection->relay_client(1);
+$command = 'RCPT TO:<[EMAIL PROTECTED]> SIZE=1230 SOMETHING=12345';
+is(($smtpd->command($command))[0], 250, $command);
+is(($smtpd->transaction->recipients)[0]->format, '<[EMAIL PROTECTED]>', 'got
the right rcpt');
+$command = 'RCPT TO:[EMAIL PROTECTED]';
+is(($smtpd->command($command))[0], 501, $command);
