Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7572 (closed defect: fixed)

Opened 2 years ago

Last modified 5 months ago

[PATCH] allow return-path header to work

Reported by: joost Assigned to: jamis@37signals.com
Priority: normal Milestone: 2.0.3
Component: ActionMailer Version: edge
Severity: normal Keywords: tiny verified
Cc:

Description

Attached is a very useful patch, based on various blogs and mailing list posts I've read today.

The smtp Return-Path header SHOULD be used by MTAs to send bounce messages, quota warnings and delivery faillures. Most do, so this is a great header to use when you run mailing lists. Or when you want to automate bounce messages.

ActionMailer allows the Return-Path header to be changed just fine. But when passing the message for delivery via SMTP, it is lost since the From: header is used there.

This patch fixes that, in that it will prefer the Return-Path header as the envelope from. If it doesn't exist, From: is used.

The change is tiny.

Oddly, the unit tests I wrote to test this did NOT fail anyway. I suspect the TestMailer isn't actually behaving like a real one.

I would ask this patch to be included in the upcoming rails release.

Attachments

working_return_path.diff (2.0 kB) - added by joost on 03/18/08 17:18:13.
allows headersreturn-path? to set envelope-from
sendmail_just_one_return_path.diff (0.8 kB) - added by billkirtley on 04/02/08 15:10:43.
This patch fixes the problem of accumulating return-path specifications

Change History

03/14/07 23:09:48 changed by joost

  • milestone changed from 1.x to 1.2.4.

Will this patch be applied to actionmailer? It's not in 1.3.3 If it won't, please let me know so I can rebuild it as a plugin.

09/22/07 11:56:32 changed by roderickvd

I am all for such functionality, but the Sendmail bit isn't working correctly yet. At least not on Postfix. It outputs a command like (paraphrased) sendmail -f<user@domain.tld> which doesn't work correctly because of the < and > pipes.

I'll look into it the upcoming business week and tweak your patch when I've found a workaround. Might just be putting quotes around that output...

09/22/07 14:14:20 changed by joost

quotes around the email address is indeed an improvement and works here. Admittedly also on Postfix. I also added a space between -f and the output, per the Sendmail and Postfix man page.

I am uploading the improved version of the patch.

09/22/07 18:38:22 changed by david

  • owner changed from core to jamis@37signals.com.

09/27/07 12:05:47 changed by roderickvd

Works on Postfix now.

+1

10/29/07 09:00:43 changed by jacobat

Works for me on Postfix. It would be nice with a little documentation though. I'm not sure how one would go about testing something like this with unit tests, yet some tests would be nice too. Finally I'm not sure how other MTA's (Exim f.ex) will handle this patch.

01/15/08 05:43:24 changed by pthomsen

Ticket #7697 has a patch for this that also patches SMTP delivery. Will either of these patches ever make it into mainline code?

01/15/08 08:48:48 changed by joost

  • keywords changed from tiny to tiny verified.
  • milestone changed from 1.2.7 to 2.0.3.

Seeing ticket #7697 I can see there is a real demand for this patch. I am now moving this one to 'verified'. I have just verified the patch against current edge, which works as expected. This can now be moved into trunk.

01/15/08 08:52:23 changed by joost

@jacobat: i had already supplied new tests in a separate ticket #7571 (closed and accepted).

The test in #7571 applies to the patch here.

01/15/08 09:01:55 changed by pthomsen

Question: The patch in #7697 also includes envelope-from support in smtp. Is that included in what is being moved into trunk?

AFAICT, #7572 (this patch) does not have changes for smtp, only smtp.

01/15/08 09:02:59 changed by pthomsen

Argh! So of course I can't type. #7572 doesn't seem to have smtp support, only sendmail.

Sorry...

01/15/08 09:51:13 changed by joost

This one, #7572 most certainly has support for both smtp and postfix. Both tested.

From the patch: 

smtp.sendmail(mail.encoded, sender, destinations)

01/27/08 11:40:02 changed by lifofifo

  • keywords changed from tiny verified to tiny.

You need three plus ones for "verified" keyword.

03/09/08 14:43:09 changed by knut

+1

03/18/08 17:14:05 changed by pthomsen

+1

03/18/08 17:18:13 changed by joost

  • attachment working_return_path.diff added.

allows headersreturn-path? to set envelope-from

03/18/08 17:19:40 changed by joost

  • keywords changed from tiny to tiny verified.

I re-applied my patch to current trunk and added documentation.

03/27/08 17:54:05 changed by david

  • status changed from new to closed.
  • resolution set to fixed.

(In [9099]) Fixed that a return-path header would be ignored (closes #7572) [joost]

04/02/08 14:47:33 changed by billkirtley

  • status changed from closed to reopened.
  • resolution deleted.

Change [9099] causes the sendmail command to get longer with every invocation:

sendmail_settings[:arguments] += " -f \"#{mailreturn-path?}\"" if mailreturn-path?

On the second message to get sent either: (a) the message does not specify a return-path, and the return-path from the last one gets reused (b) the message does specify a return-path and the sendmail command gets another -f clause.

The longer and longer sendmail command is bound to cause problems eventually.

04/02/08 15:10:43 changed by billkirtley

  • attachment sendmail_just_one_return_path.diff added.

This patch fixes the problem of accumulating return-path specifications

04/05/08 11:28:18 changed by nzkoz

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [9228]) Avoid modifying the sendmail_settings hash when using the return path. Closes #7572 [billkirtley]