Skip Menu |
Queue is disabled
This queue is disabled and you may not create new tickets in it.
Report information

Subject: RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 07:37:03 +0100
To: bugs-qpsmtpd [...] rt.perl.org
From: Hanno Hecker <hah [...] uu-x.de>
Download (untitled) / with headers
text/plain 318b
This patch adjusts the parsing of the RCPT TO command to the same behaviour as the MAIL FROM. There's one issue, which is also present in the mail from parser: mails without surrounding <> are not parsed correctly if it contains spaces. Suggestion is not to accept addresses without <> in mail() and rcpt(). Hanno

Message body is not shown because sender requested not to inline it.

Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Thu, 16 Mar 2006 23:24:38 -0800
To: qpsmtpd [...] perl.org
From: Robert Spier <rspier [...] pobox.com>
Download (untitled) / with headers
text/plain 724b
Show quoted text
> Suggestion is not to accept addresses without <> in mail() and rcpt().
We shouldn't do that unless some other major mailer is doing it. Otherwise it will bite us. Show quoted text
> + my ($rcpt) = ($rcpt_parameter =~ m/^to:\s*(<[^>]*>)/i)[0]; > + # support addresses without <> ... maybe we shouldn't? > + ($rcpt) = "<" . ($rcpt_parameter =~ m/^to:\s*(\S+)/i)[0] . ">" > + unless $rcpt;
It's perfectly valid perl, but it's hard to read. Show quoted text
> + $rcpt = (Qpsmtpd::Address->parse($rcpt))[0]; > + ### does this one ever happen? > return $self->respond(501, "could not parse recipient") unless $rcpt; > > + return $self->respond(501, "cound not parse recipient") > + if $rcpt->format eq '<>'; > +
candidate for an || -R
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 10:02:30 +0100
To: Robert Spier <rspier [...] pobox.com>
From: Hanno Hecker <vetinari [...] ankh-morp.org>
Download (untitled) / with headers
text/plain 417b
On Thu, 16 Mar 2006 23:24:38 -0800 Robert Spier <rspier@pobox.com> wrote: Show quoted text
> > + my ($rcpt) = ($rcpt_parameter =~ m/^to:\s*(<[^>]*>)/i)[0]; > > + # support addresses without <> ... maybe we shouldn't? > > + ($rcpt) = "<" . ($rcpt_parameter =~ m/^to:\s*(\S+)/i)[0] . ">" > > + unless $rcpt;
> > It's perfectly valid perl, but it's hard to read.
It's nearly a 1:1 copy from sub mail() in same file... Hanno
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 12:03:48 -0500 (EST)
To: Robert Spier <rspier [...] pobox.com>
From: Charlie Brady <charlieb-qpsmtpd [...] budge.apana.org.au>
Download (untitled) / with headers
text/plain 393b
On Thu, 16 Mar 2006, Robert Spier wrote: Show quoted text
>> Suggestion is not to accept addresses without <> in mail() and rcpt().
> > We shouldn't do that unless some other major mailer is doing it. > Otherwise it will bite us.
Addresses without <> are not valid SMTP. I think that mailfront requires <>. I've never noticed any breakage, except that I needed to learn to use <> when testing in telnet.
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 09:09:11 -0800
To: Charlie Brady <charlieb-qpsmtpd [...] budge.apana.org.au>
From: Robert Spier <rspier [...] pobox.com>
Download (untitled) / with headers
text/plain 573b
Show quoted text
> On Thu, 16 Mar 2006, Robert Spier wrote:
> >> Suggestion is not to accept addresses without <> in mail() and rcpt().
> > > > We shouldn't do that unless some other major mailer is doing it. > > Otherwise it will bite us.
> > Addresses without <> are not valid SMTP. > > I think that mailfront requires <>. I've never noticed any breakage, > except that I needed to learn to use <> when testing in telnet.
Sure, but sendmail has accepted addresses without <> forever. I guess the real question is what does qmail do? (Since that's out reference implementation) -R
CC: Robert Spier <rspier [...] pobox.com>, qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Sat, 18 Mar 2006 04:09:44 +1100
To: Charlie Brady <charlieb-qpsmtpd [...] budge.apana.org.au>
From: Gordon Rowell <gordonr [...] gormand.com.au>
Download (untitled) / with headers
text/plain 1.2k
Charlie Brady wrote: Show quoted text
> Addresses without <> are not valid SMTP.
Yep - the angles are mandatory. RFC2821: 4.1.2 Command Argument Syntax The syntax of the argument fields of the above commands (using the syntax specified in [8] where applicable) is given below. Some of the productions given below are used only in conjunction with source routes as described in appendix C. Terminals not defined in this document, such as ALPHA, DIGIT, SP, CR, LF, CRLF, are as defined in the "core" syntax [8 (section 6)] or in the message format syntax [32]. Reverse-path = Path Forward-path = Path Path = "<" [ A-d-l ":" ] Mailbox ">" And RFC821: <reverse-path> ::= <path> <forward-path> ::= <path> <path> ::= "<" [ <a-d-l> ":" ] <mailbox> ">" We (SME Server) have been running with Peter Holtzer's RPMs which apply a patch to enforce angles. Thanks, Gordon -- Gordon Rowell Gordon.Rowell@gormand.com.au http://www.gormand.com.au Gormand Pty Ltd PO Box 239 St Pauls NSW 2031 Australia "The test of our progress is not whether we add more to the abundance of those who have much; it is whether we provide enough for those who have too little." Franklin D Roosevelt, Second Inaugural Address, 1937
CC: Charlie Brady <charlieb-qpsmtpd [...] budge.apana.org.au>, qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 12:18:29 -0500
To: Robert Spier <rspier [...] pobox.com>
From: "Michael C. Toren" <mct [...] toren.net>
Download (untitled) / with headers
text/plain 304b
On Fri, Mar 17, 2006 at 09:09:11AM -0800, Robert Spier wrote: Show quoted text
> Sure, but sendmail has accepted addresses without <> forever. I guess > the real question is what does qmail do? (Since that's out reference > implementation)
qmail-smtpd does not require <>'s in either the MAIL FROM or RCPT TO. -mct
CC: Charlie Brady <charlieb-qpsmtpd [...] budge.apana.org.au>, qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Sat, 18 Mar 2006 04:18:42 +1100
To: Robert Spier <rspier [...] pobox.com>
From: Gordon Rowell <gordonr [...] gormand.com.au>
Download (untitled) / with headers
text/plain 728b
Robert Spier wrote: Show quoted text
> [...] > Sure, but sendmail has accepted addresses without <> forever. I guess > the real question is what does qmail do? (Since that's out reference > implementation) >
qmail-smtpd accepts without angles (see addrparse and the comment therein). qmail-remote always sends with angles. I have yet to see a legitimate mail rejected. Thanks, Gordon -- Gordon Rowell Gordon.Rowell@gormand.com.au http://www.gormand.com.au Gormand Pty Ltd PO Box 239 St Pauls NSW 2031 Australia "The test of our progress is not whether we add more to the abundance of those who have much; it is whether we provide enough for those who have too little." Franklin D Roosevelt, Second Inaugural Address, 1937
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 17:22:27 +0000 (UTC)
To: qpsmtpd ML <qpsmtpd [...] perl.org>
From: sub-qpsm-tpd [...] rope.net
Download (untitled) / with headers
text/plain 813b
On Fri, 17 Mar 2006, Charlie Brady wrote: Show quoted text
> On Thu, 16 Mar 2006, Robert Spier wrote: >
>>> Suggestion is not to accept addresses without <> in mail() and rcpt().
>> >> We shouldn't do that unless some other major mailer is doing it. >> Otherwise it will bite us.
> > Addresses without <> are not valid SMTP. > > I think that mailfront requires <>. I've never noticed any breakage, except > that I needed to learn to use <> when testing in telnet.
This concept was discussed before. It was noted that even though it was not valid SMTP, addresses without <> were accepted because that's what qmail-smtpd did. My only worry with making it stricter is whether or not any major MTAs do not use <>, because we'd be dropping a lot of otherwise legitimate email, which would be unacceptable to our clients.
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 17:31:38 +0000
To: qpsmtpd [...] perl.org
From: Robin Bowes <robin-lists [...] robinbowes.com>
Download (untitled) / with headers
text/plain 515b
sub-qpsm-tpd@rope.net wrote: Show quoted text
> This concept was discussed before. It was noted that even though it > was not valid SMTP, addresses without <> were accepted because that's > what qmail-smtpd did. > > My only worry with making it stricter is whether or not any major > MTAs do not use <>, because we'd be dropping a lot of otherwise > legitimate email, which would be unacceptable to our clients.
Sounds like this should be a configurable option, with default behaviour to accept addresses without <>. R.
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 14:26:13 -0500 (EST)
To: qpsmtpd [...] perl.org
From: Charlie Brady <charlieb-qpsmtpd [...] budge.apana.org.au>
On Fri, 17 Mar 2006, Charlie Brady wrote: Show quoted text
> I think that mailfront requires <>.
It doesn't.
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Sat, 18 Mar 2006 08:12:23 +1100
To: Robin Bowes <robin-lists [...] robinbowes.com>
From: Gordon Rowell <gordonr [...] gormand.com.au>
Download (untitled) / with headers
text/plain 673b
Robin Bowes wrote: Show quoted text
> [...] > Sounds like this should be a configurable option, with default behaviour > to accept addresses without <>. >
I don't object to it being configurable (other than the code involved), but I think we should opt for RFC compliance as the default when the RFC is unambiguous. Thanks, Gordon -- Gordon Rowell Gordon.Rowell@gormand.com.au http://www.gormand.com.au Gormand Pty Ltd PO Box 239 St Pauls NSW 2031 Australia "The test of our progress is not whether we add more to the abundance of those who have much; it is whether we provide enough for those who have too little." Franklin D Roosevelt, Second Inaugural Address, 1937
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 17 Mar 2006 14:07:35 -0800
To: qpsmtpd [...] perl.org
From: Ask Bjørn Hansen <ask [...] develooper.com>
Download (untitled) / with headers
text/plain 913b
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. Also: Tests, please! It's pretty simple to add tests for "is the parsing working right". 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. - ask -- http://askask.com/ - http://develooper.com/
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Sat, 18 Mar 2006 20:44:42 +0100
To: Ask Bjørn Hansen <ask [...] develooper.com>
From: Hanno Hecker <hah [...] uu-x.de>
Download (untitled) / with headers
text/plain 2.7k
On Fri, 17 Mar 2006 14:07:35 -0800 Ask Bjørn Hansen <ask@develooper.com> wrote: Show quoted text
> 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 Show quoted text
> 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? Show quoted text
> 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

Message body is not shown because sender requested not to inline it.

CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Sun, 2 Apr 2006 10:34:56 +0200
To: Ask Bjørn Hansen <ask [...] develooper.com>
From: Hanno Hecker <hah [...] uu-x.de>
Download (untitled) / with headers
text/plain 1.5k
On Fri, 17 Mar 2006 14:07:35 -0800 Ask Bjørn Hansen <ask@develooper.com> wrote: Show quoted text
> 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.
Ok, here we go. The Qpsmtpd::Command module provides a parse() which does 3 things: - by default returning the line split()ed by space, as before. - on MAIL FROM:/RCPT TO: it parses the line by RFC 1869 (SMTP Service Extensions) rules - a plugin can do the parsing on it's own (...read: for all other plugins/the core) by supplying a parsing routine when returning from a _parse hook, see EXAMPLE in perldoc lib/Qpsmtpd/Command.pm The rcpt and mail hooks now get a %param where all parameters from the client are listed in lower case, i.e. sub hook_mail { my ($self, $transaction, $sender, %params) = @_; if ($params{'body'} && uc($params{'body'}) eq 'BINARYMIME') { ... Show quoted text
> 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.
This [accepting addresses without <>] is now a plugin ("dont_require_angelbrackets") which is called in the new "rcpt_pre" hook. Comments, whishes, ...? :) Hanno
Download qp-command.diff
text/plain 24k

Message body is not shown because sender requested not to inline it.

CC: Ask Bjørn Hansen <ask [...] develooper.com>
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctlyparsed
Date: Sun, 02 Apr 2006 08:58:40 -0400
To: qpsmtpd [...] perl.org, Hanno Hecker <hah [...] uu-x.de>
From: John Peacock <jpeacock [...] rowman.com>
Download (untitled) / with headers
text/plain 542b
Hanno Hecker wrote: Show quoted text
> This [accepting addresses without <>] is now a plugin > ("dont_require_angelbrackets") which is called in the new "rcpt_pre" > hook.
I haven't gone over the code in detail, but dont_require_angelbrackets should probably be renamed "dont_require_anglebrackets" for consistency with the rest of the code (i.e. in English). ;-) Other than that, the initial impression I get is that it looks good. I'll go through it in detail later (when the effects of DST kicking in have worn off). ,-O <smiley for yawning> John
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctlyparsed
Date: Fri, 07 Apr 2006 14:59:08 -0400
To: qpsmtpd [...] perl.org, Hanno Hecker <hah [...] uu-x.de>
From: John Peacock <jpeacock [...] rowman.com>
Download (untitled) / with headers
text/plain 1.6k
Hanno Hecker wrote: Show quoted text
> Ok, here we go. > The Qpsmtpd::Command module provides a parse() which does 3 things: > - by default returning the line split()ed by space, as before. > - on MAIL FROM:/RCPT TO: it parses the line by RFC 1869 (SMTP Service > Extensions) rules > - a plugin can do the parsing on it's own (...read: for all other > plugins/the core) by supplying a parsing routine when returning from > a _parse hook, see EXAMPLE in perldoc lib/Qpsmtpd/Command.pm
I'm committing this with a few changes that I thought you should be aware of: 1) You provided tests: Good. The tests didn't work with the patch you submitted: Bad. :( It was something simple, though. The test framework uses the config.sample/plugins file and you had commented out the dont_require_anglebrackets line, so the tests that relied on that being in force failed. Note I also renamed it to correspond to the primary language for the source being English (though I must say that <> look a little like angel's wings)... 2) I took out the unrecognized_command_parse logic, because it was messing with tls (which I'm guessing you didn't test with). My reasoning is that an unrecognized command hook should be able to do what it wants with the line. In this case STARTLS doesn't take any options and putting in the parse was causing it to fail in various ways. That code was confused about $rc and $ok as well; I'm not entirely happy with the $ok vs $rc in the other locations, but I didn't mess with them. For a change, I remembered to add all of the new files to the repository *and* to MANIFEST, as well as update the Changes file. Thanks for the patch! John
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 7 Apr 2006 14:49:14 -0500
To: "Robin Bowes" <robin-lists [...] robinbowes.com>
From: "David Nicol" <davidnicol [...] gmail.com>
When I wrote Tipjar::MTA, after sending lots of mail by telnetting to my qmail-smtpd, I left the angle brackets out. I added them when I could not send a message to any @yahoo.com addresses. Not requiring bracketing makes your smtpd easier to send through when you telnet to it. Fewer keystrokes. I would write more but it appears that my building is on fire. On 3/17/06, Robin Bowes <robin-lists@robinbowes.com> wrote: Show quoted text
> sub-qpsm-tpd@rope.net wrote:
> > This concept was discussed before. It was noted that even though it > > was not valid SMTP, addresses without <> were accepted because that's > > what qmail-smtpd did. > > > > My only worry with making it stricter is whether or not any major > > MTAs do not use <>, because we'd be dropping a lot of otherwise > > legitimate email, which would be unacceptable to our clients.
> > Sounds like this should be a configurable option, with default behaviour > to accept addresses without <>. > > R. > > >
-- David L Nicol Should the bike shed have bunks? Or maybe cots?
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 7 Apr 2006 14:50:41 -0500
To: "Robin Bowes" <robin-lists [...] robinbowes.com>
From: "David Nicol" <davidnicol [...] gmail.com>
Download (untitled) / with headers
text/plain 152b
On 4/7/06, David Nicol <davidnicol@gmail.com> wrote: Show quoted text
> I would write more but it appears that my building is on fire.
false alarm, apparently. Sorry
CC: qpsmtpd [...] perl.org
Subject: Re: [perl #38747] RCPT TO addresses with spaces are not correctly parsed
Date: Fri, 7 Apr 2006 14:52:36 -0500
To: "Robin Bowes" <robin-lists [...] robinbowes.com>
From: "David Nicol" <davidnicol [...] gmail.com>
Download (untitled) / with headers
text/plain 265b
I think NO_ANGLEBRACKETS should be a flag set in the incoming message object, so a simple REFUSE_NO_ANGLEBRACKETS plugin can look for it and deny the message. That would be the qpsmtpd way, AIUI. -- David L Nicol Should the bike shed have bunks? Or maybe cots?
Download (untitled) / with headers
text/plain 103b
Closing again (please don't reply to this thread unless you find a bug in what was committed) ;-) John


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org