This project is archived and is in readonly mode.

ActionMailer doesn't correctly use return-path header

#1842

When sending emails with a return-path set, using the :smtp delivery method, ActionMailer sends an invalid MAIL FROM header. The problem arises from the use of mail['return-path'] in perform_delivery_smtp; TMail returns a TMail::ReturnPathHeader, which picks up an extra set of gt/lt signs when converted to a string. Net::SMTP already encloses the address, and badness ensues.

Short code example (try it in a script/console):


m = TMail::Mail.new
m['return-path'] = 'foo@bar.com'

m['return-path'].to_s
# => "<foo@bar.com>"

m['return-path'].spec
# => "foo@bar.com"

Changing the relevant line (line 675, base.rb) to use mail['return-path'].spec would seem to solve this problem cleanly.

This isn't caught by the tests, as all the tests use MockSMTP, which doesn't exhibit this behavior.

Given that this is a five-character change, does it need a patch?

Reported by Matt Jones · February 2nd, 2009 @ 06:00 AM

Activity

Chris Hapgood
Hmmmm. I just ran a test and can't see the double quotes. Yet I'm pretty sure I have seen them in the past.

My versions: Rails.version => 2.2.2 Net::SMTP::Revision => "11708"

February 3rd, 2009 @ 05:06 PM
Chris Hapgood
Whoops. I had the delivery method set to sendmail on my staging machine. As soon as a I set it to smtp, I do indeed see the double angle brackets around the return path address.

From my sendmail log:
```
Feb  3 13:24:56 osprey sendmail[30343]: n13IOuNd030343: from=<<bounces+chapgood@beta.racht.com>>, size=2379, class=0, nrcpts=1, msgid=<49888bf87ab8d_7684..fdbfb7b822e5@osprey.racht.com.tmail>, proto=ESMTP, daemon=MTA, relay=localhost.localdomain [127.0.0.1]
```
Matt, I would suggest you upload a patch at a minimum. Testing it will indeed be a b!&%#.

+1 to fix this!
February 3rd, 2009 @ 06:30 PM
Matt Jones
- Tag changed from 2.3, actionmailer, bug to 2.3, actionmailer, bug, patch
Worked up a patch - given that Net::SMTP's behavior is known, I added an assertion to the existing Return-Path test to check the address is correctly formatted.

I recommend MailTrap to listen in on the SMTP conversation. Before the patch, the output (in mailtrap.output, not mailtrap.log) from a mailer that set the Return-Path:
```
Helo: EHLO example.com
Seen an EHLO
From: MAIL FROM:<<another@example.com>>
To: RCPT TO:<somebody@example.com>
+ Return-Path: <another@example.com>
+ Date: Tue, 3 Feb 2009 14:34:23 -0500
+ From: anybody@example.com
+ To: somebody@example.com
+ Message-Id: <49889c3f1cd8f_db11a37e3db@matt-jones-computer.local.tmail>
+ Subject: test mail
+ Mime-Version: 1.0
+ Content-Type: text/plain; charset=utf-8
+ 
+ Blah blah blah!!!
+ Blah blah blah!!!
+ Blah blah blah!!!
And we're done with that bozo!
```
Note the extra brackets on the MAIL FROM line. The patch fixes it.
- smtp_fix.patch 1.58 KB
February 3rd, 2009 @ 08:04 PM
Repository
- State changed from new to committed
(from [250dfb18afb58dda3caf4b9f170ddb9c5cf85faf]) Fixed that ActionMailer should send correctly formatted Return-Path in MAIL FROM for SMTP [#1842 ActionMailer doesn't correctly use return-path header state:committed]

Signed-off-by: David Heinemeier Hansson david@loudthinking.com http://github.com/rails/rails/co...

February 5th, 2009 @ 07:54 PM
Yar Dmitriev
Rails 2.3.8, production mode

config.action_mailer.delivery_method = :sendmail

An attempt to specify:

headers "return-path" => "user@domain.tld"

results in the envelope-from value being <user@domain.tld>, which cases the message to be bounced back.

The patch is attached.
- sendmail_fix.patch 614 Bytes
September 4th, 2010 @ 02:48 AM
Yar Dmitriev
Of course, I meant double angular brackets - the parser shows them as singular here.

September 4th, 2010 @ 02:50 AM