Sendmail "from" header can not be configured in one config file, From header broken in SMTP

RESOLVED FIXED in Bugzilla 2.22

Status

()

Bugzilla
Email Notifications
--
major
RESOLVED FIXED
17 years ago
12 years ago

People

(Reporter: Gerhard Wiesinger, Assigned: Jochen Wiedmann)

Tracking

2.13
Bugzilla 2.22
Dependency tree / graph
Bug Flags:
approval +
blocking2.22 +

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

17 years ago
When you want to set the "from" item for the automatically sent emails one has 
to change in different perl scripts:

e.g. in CGI.pl:
Change
 open SENDMAIL, "|/usr/lib/sendmail -t";
to
 open SENDMAIL, "|/usr/lib/sendmail -t -f myemail@mydomain.com";

It would be nice to configure this globally.

Also the sendmail path (/usr/lib/sendmail) should be configured globally.
You don't have to change any perl code to set the from header.  This is all in 
the preferences in editparams.cgi from the web.  Look for "passwordmail" and 
"newchangedmail".  Edit the headers to suit.

As for the sendmail location, that's bug 57798
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 2

17 years ago
Yes, thats is valid for the email itself (e.g. the headers) but not for the SMTP 
envelope:
Example:
Webserver runs under "wwwrun@mydomain.com", configured is as "from": 
user@mydomain.com. The receipient is touser@yourdomain.com

Then the following happens:
The envelope contains:
From: wwwrun@mydomain.com
To: touser@yourdomain.com

So the following SMTP session is established:
HELO ...
MAIL FROM: <wwwrun@mydomain.com>

When wwwrun@mydomain.com is in an intranet and mydomain.com is not valid outside  
the intranet mail sending does not work. (The other e.g. sendmail says: sender 
domain must exist).

So it must be possible to set the envelope with the -f option of sendmail.

The Email itself is correct:
From: user@mydomain.com
To: touser@yourdomain.com

Hope everything is clear => If not just mail me.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
In that case, I think this will be fixed by the patch on bug 84876.  Do you 
agree?
(Reporter)

Comment 4

17 years ago
When ALL!! sendmail related things are abstracted into another API this is OK.

It must also be verified that the mail address from field (for the envelope) can 
be changed.

BTW: When will the patch be included in the next official version?
That's a good question.  Right now it's targetted at 2.16, which is the next 
release after the one we're working on currently.  2.14 should be out any time 
now, and once it's out, we have over 90 patches waiting for review and checkin 
for 2.16 (that one among them).  It'd probably be safe to say within a few 
months.
Blocks: 84876
Severity: normal → enhancement
Target Milestone: --- → Bugzilla 2.16
Mass moving to new product Bugzilla...
Assignee: justdave → jake
Status: REOPENED → NEW
Component: Bugzilla → Email
Product: Webtools → Bugzilla
Version: Bugzilla 2.12 → 2.13
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18

Comment 8

16 years ago
Compare bug 126151, which is an alternative suggestion about the From header.
Changing default owner of Email Notifications component to JayPee, a.k.a.
J. Paul Reed (preed@sigkill.com).  Jake will be offline for a few months.
Assignee: jake → preed
Enhancements which don't currently have patches on them which are targetted at
2.18 are being retargetted to 2.20 because we're about to freeze for 2.18. 
Consideration will be taken for moving items back to 2.18 on a case-by-case
basis (but is unlikely for enhancements)
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
The patch on bug 49893 fixes this.
Depends on: 49893
No longer depends on: 84876
Whiteboard: [blocker will fix]
Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
(Assignee)

Comment 13

13 years ago
Created attachment 201118 [details] [diff] [review]
Patch for setting the "from" address in the envelope

The attached patch searches for a "from" header in the mail being sent. If such a header is present, it adds an option "-f<fromaddr>" (sendmail delivery) or sets the environment address MAILADDRESS (smtp delivery). I haven't checked other delivery methods, because I have no possibility to verify them.
Attachment #201118 - Flags: review?
(Assignee)

Updated

13 years ago
Attachment #201118 - Flags: review?(jake)
Attachment #201118 - Flags: review?(gerv)
Attachment #201118 - Flags: review?
There is bug 135812, too. Are the two bugs as identical as they seem to me?

Comment 15

13 years ago
this won't go into 2.22
Assignee: preed → email-notifications
OS: Windows NT → All
QA Contact: mattyt-bugzilla → default-qa
Hardware: PC → All
Whiteboard: [blocker will fix]
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
(Assignee)

Comment 16

13 years ago
(In reply to comment #14)

> There is bug 135812, too. Are the two bugs as identical as they seem to me?

IMO, the two bugs are not identical, but related.

This bug adresses the fact, that setting the "from" address is not sufficient. Bug 135812 adresses the problem, that the "from" address should be configurable, preferrably at a central location.
(Assignee)

Comment 17

13 years ago
(In reply to comment #15)

> this won't go into 2.22

LpSolit: Would you please be so kind and explain the reasons? The suggested patch is relatively small and without side effects. (The latter point is, of course, subject to a review, but you didn't seem to review the patch.)

If my impression is right, then I can't think of a reason to defer landing this change.

Comment 18

13 years ago
(In reply to comment #17)
> LpSolit: Would you please be so kind and explain the reasons?

Because the trunk is frozen for two weeks already and no new enhancement bug will be accepted for 2.22 (we are only accepting bug fixes to stabilize the code). This bug is retargetted to 2.24 as it has a patch ready for review. Else this bug would have been retargetted to ---.

You know, I have some patches for enhancement bugs which have already been reviewed (r+) but which have now to wait till we unfreeze before landing. So I don't do this because it's you, I'm in the same situation than you! ;)

I also reassign the bug to you as you are actively working on it.
Assignee: email-notifications → jochen.wiedmann
(Assignee)

Comment 19

13 years ago
(In reply to comment #18)

> I also reassign the bug to you as you are actively working on it.

Thanks, accepting.

Status: NEW → ASSIGNED
Comment on attachment 201118 [details] [diff] [review]
Patch for setting the "from" address in the envelope

I'll review this as soon as  2.22 freeze is over on the trunk.
Attachment #201118 - Flags: review?(wicked)
It seems to me that this might fix bug 318731. If so, then we should land this for 2.22. I'll take a closer look as soon as possible.
(Assignee)

Comment 22

13 years ago
Marc, I do indeed believe, that Bug 318731 is a duplicate. My patch is, IMO, a little bit more complete than yours, because it does also handle the "sendmail" method. On the other hand, you are carefully avoiding to override an existing MAILADDRESS environment variable. Whether that is good, seems to me to be a discussable subject. Personally, I think it is better to ignore the existing variable. If people have specified a "From" header, it is what they want to see.
*** Bug 318731 has been marked as a duplicate of this bug. ***

Updated

13 years ago
Target Milestone: Bugzilla 2.24 → Bugzilla 2.22

Updated

13 years ago
Flags: blocking2.22?
Comment on attachment 201118 [details] [diff] [review]
Patch for setting the "from" address in the envelope

while the patch has bitrot, manually applying (most of) the changes looks good for me, except there's now a different call for sendmail if running on windows, which should also pick up the -f flag.
Attachment #201118 - Flags: review-
(Assignee)

Comment 25

13 years ago
Created attachment 206570 [details] [diff] [review]
Updated version of the patch

Updated patch, this time including the -f flag even for Windows.
Attachment #201118 - Attachment is obsolete: true
Attachment #201118 - Flags: review?(wicked)
Attachment #201118 - Flags: review?(jake)
Attachment #201118 - Flags: review?(gerv)
Attachment #206570 - Flags: review?(bugzilla)
Comment on attachment 206570 [details] [diff] [review]
Updated version of the patch

r-

>+        if ($] >= 5.008) {
>+            # Use modern version to invoke sendmail. This avoids
>+            # command injection via $from.

we know it's windows, just quote it.

>+            my @args = (SENDMAIL_EXE, '-t', '-i');
>+            if ($from) {
>+                push(@args, '-f$from');
>+            }
>+            open(SENDMAIL, '|' . SENDMAIL_EXE . ' -t -i') ||
>+                die 'Failed to execute ' . SENDMAIL_EXE . ": $!\n";

you're not actually adding -f here ;)

open(SENDMAIL, '|' . SENDMAIL_EXE . qq# -t -i "-f$from"#) ||

>+            # If we do not know, we'd better assume it doesn't.
>+            # Perhaps, the best option would be to use Mail::Mailer
>+            # anyways, even on Unix/Linux? Saves about 20 lines of code.

mail::mailer's sendmail method won't work on windows, which is why it's treated differently.
Attachment #206570 - Flags: review?(bugzilla) → review-
(Assignee)

Comment 27

13 years ago
Created attachment 206575 [details] [diff] [review]
Updated version of the updated version of the patch (this is getting SCO like renewed version of ... :-)


> we know it's windows, just quote it.

Understood. Changed.

> you're not actually adding -f here  ;) 

Ye gods ...

> mail::mailer's sendmail method won't work on windows, which is
> why it's treated differently.

Understood.
Attachment #206570 - Attachment is obsolete: true
Attachment #206575 - Flags: review?(bugzilla)
Comment on attachment 206575 [details] [diff] [review]
Updated version of the updated version of the patch (this is getting SCO like renewed version of ... :-)

> this is getting SCO like renewed version of ...  

hehe :)

sorry to do this, but..

>+        if ($from) {
>+            # We're on Windows, thus no danger of command injection
>+            # via $from. In other words, it is safe to embed $from.
>+            $cmd .= " -f$from";
>+        }

i should have been clearer when i said "just quote it".

you still need to put quotes around $from as it may contain spaces.

  $cmd .= qq# -f"$from"#;
Attachment #206575 - Flags: review?(bugzilla) → review-
(Assignee)

Comment 29

13 years ago
Created attachment 206576 [details] [diff] [review]
(Updated version of the)^3 patch

If you can, please feel free to apply required changes and pull the patch in. The thing that matters is to get it in.
Attachment #206576 - Flags: review?(bugzilla)
Comment on attachment 206576 [details] [diff] [review]
(Updated version of the)^3 patch

r=glob

:)
Attachment #206576 - Flags: review?(bugzilla) → review+

Updated

13 years ago
Flags: approval?
Flags: blocking2.22? → blocking2.22-
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
er, I was basing that on the summary and severity of the bug.  Then someone pointed out the dupe, which has a much more severe issue, which isn't an enhancement, but a bugfix which takes the enhancement this bug was originally for along as a side-effect.  Yeehah.
Severity: enhancement → major
Flags: blocking2.22-
Flags: blocking2.22+
Flags: approval?
Flags: approval+
Summary: Sendmail "from" header can not be configured in one config file → Sendmail "from" header can not be configured in one config file, From header broken in SMTP
Target Milestone: Bugzilla 2.24 → Bugzilla 2.22
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.60; previous revision: 1.59
done
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago13 years ago
Resolution: --- → FIXED
Attachment #206575 - Attachment is obsolete: true

Comment 33

12 years ago
Created attachment 218821 [details] [diff] [review]
Patch for From address with trailing newline

The attached patch fixes Jochen Wiedmann's patch "Patch for setting From address in the envelope" in attachment 201118 [details] [diff] [review]. 

On my Windows system with default BUGZILLA-2_22rc1 installation $from variable contains "bugzilla-daemon\n" that leads to following SMTP conversation when $ENV{'MAILADDRESS'} is set to $from. 

The proposed patch removes traling newlines.

Tue 2006-04-18 15:43:13: <-- EHLO localhost.localdomain
...
Tue 2006-04-18 15:43:17: <-- MAIL FROM:<bugzilla-daemon @MYCOMP.MYDOMAIN>
Tue 2006-04-18 15:43:17: --> 553 This server does not accept routed mail
Tue 2006-04-18 15:43:17: <-- RCPT TO:<islobodin@yandex.ru>
Tue 2006-04-18 15:43:17: --> 503 Unexpected command or sequence of commands
Tue 2006-04-18 15:43:17: <-- DATA
Tue 2006-04-18 15:43:17: --> 503 Unexpected command or sequence of commands
...
(Assignee)

Comment 34

12 years ago
I have no idea where the \n comes from? It would be interesting to investigate the source.

Besides, I have no problem with the patch, only that I would change the regexp to

    s/\s+//s

This should be both faster and safer.
Blocks: 331365
Comment on attachment 218821 [details] [diff] [review]
Patch for From address with trailing newline

Thanks for the patch! As we usually only deal with one patch per bug, I've moved this patch to bug 331365, which was opened to track this regression.
Attachment #218821 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.