WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 10 years ago

Last modified 22 months ago

#5007 closed defect (bug) (fixed)

Email notifications fail on hosted sites that check sender address

Reported by: jlwarlow Owned by: pishmishy
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.8.4
Component: Mail Keywords: needs-patch
Focuses: Cc:
PR Number:

Description

I had wordpress 2.2.2 hosted on FastHosts and wasn't getting any email notifications, everything was landing in dead.letter. After some investigation I found it was because the phpmailer->Sender wasn't being set. Fasthosts check the sender email address to make sure it's a valid email address for an account in your domain as part of their spam filtering rules.

If you add the line

$phpmailer->Sender = "wordpress@" . preg_replace('#^www\.#', '', strtolower($_SERVER['SERVER_NAME']));

to pluggable.php on wp-includes before the line

$phpmailer->FromName = "WordPress";

I found this fixed the issue.

Attachments (4)

patch.diff (489 bytes) - added by mattyrob 12 years ago.
Patch
patch.2.diff (489 bytes) - added by mattyrob 12 years ago.
5007.patch (975 bytes) - added by pishmishy 12 years ago.
Standardizes sender address to admin_email
5007.2.patch (1.6 KB) - added by pishmishy 12 years ago.
Document the problems in the code

Download all attachments as: .zip

Change History (49)

#1 @Viper007Bond
12 years ago

  • Keywords has-patch 2nd-opinion added; phpmailer removed
  • Milestone changed from 2.2.3 to 2.3
  • Version changed from 2.2.3 to 2.2.2

@mattyrob
12 years ago

Patch

@mattyrob
12 years ago

#2 @mattyrob
12 years ago

Working from revision 6159 it still looks like $phpmailer->Sender is not being used. I've attached (hopefully) a possible patch that makes use of the existing filters and variables instead of using a preg_replace as suggested above.

#3 @mattyrob
12 years ago

  • Milestone changed from 2.4 to 2.3.1

Can't we get this in for 2.3.1?

#4 follow-up: @ryan
12 years ago

I'm not sure how that would help unless you happen to have a wordpress@ address for your domain. Regardless, the patch makes Sender and From consistent, which seems a good thing to do. I'll +1.

#5 @westi
12 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [6265]) Set the Sender on emails as well as from. Fixes #5007 for trunk props mattyrob

#6 @westi
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#7 @westi
12 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [6266]) Set the Sender on emails as well as from. Fixes #5007 for 2.3.1 props mattyrob

#8 in reply to: ↑ 4 ; follow-up: @foolswisdom
12 years ago

  • Keywords regression added; has-patch 2nd-opinion removed
  • Milestone changed from 2.3.1 to 2.3.2
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to ryan:

I'm not sure how that would help unless you happen to have a wordpress@ address for your domain. Regardless, the patch makes Sender and From consistent, which seems a good thing to do. I'll +1.

Who would have known, turns out this is bad mojo, and doesn't play nice with other configurations, causing regressions #5273 and 5294 . Seems that in some environments if you are smart enough to set the Sender, you are dumb to be a spammer, and so they run it through "callout verification".

#9 in reply to: ↑ 8 ; follow-up: @mattyrob
12 years ago

Who would have known, turns out this is bad mojo, and doesn't play nice with other configurations, causing regressions #5273 and 5294 . Seems that in some environments if you are smart enough to set the Sender, you are dumb to be a spammer, and so they run it through "callout verification".

That's bad news - seems we're damned if we do set Sender and we're damend if we don't depending on the server config :-(

I think we should set it to be honest as it's consistent and valid. Any host dropping email that sets Sender is being overly strict IMO.

#10 in reply to: ↑ 9 @lloydbudd
12 years ago

Replying to mattyrob:

consistent and valid.

That's the problem, for most members it isn't valid. wordpress@[domain] doesn't exist.

#11 @westi
12 years ago

I guess the solution is going to be:

#12 follow-up: @lloydbudd
12 years ago

westi, sounds good. A longer term project may be to evaluate using the admin email address or helping set and ensuring a valid email is used (maybe using this same "callout verification" technique, hehe) -- its own ticket, for another day.

#13 in reply to: ↑ 12 ; follow-up: @westi
12 years ago

Replying to lloydbudd:

westi, sounds good. A longer term project may be to evaluate using the admin email address or helping set and ensuring a valid email is used (maybe using this same "callout verification" technique, hehe) -- its own ticket, for another day.

I'm pretty sure we used to use the admin address at one point - I can't remember exactly why it changed to the current method.

#14 in reply to: ↑ 13 @mattyrob
12 years ago

Replying to westi:

I'm pretty sure we used to use the admin address at one point - I can't remember exactly why it changed to the current method.

My plugin tries to pull the admin details including the address - I often run into support issues because users have deleted the admin entry (ID = 1) from the database. This may be why that method is no longer used. It may also be because the supplied email could be on a different domain than the blog.

I think future work on using a valid email would be good. In the meantime I guess we'll have to reverse this change. :-(

#15 @torbens
12 years ago

  • Priority changed from low to normal

I'm also running into this issue. No mails due to nonexistent wordpress@... address. No documentation on this either in the upgrade guides. Occurs on a major German hoster. Therefore I'm boosting the priority.

I don't quite understand why you guys not simple use the email address specified at '/wp-admin/options-general.php'. It even says "This address is used only for admin purposes."

#16 @pishmishy
12 years ago

  • Owner changed from anonymous to pishmishy
  • Status changed from reopened to new

There doesn't appear to be any consistency as to when WordPress uses get_option('admin_email') or "wordpress@" . $_SERVER['SERVER_NAME'] to send mail.

Since the second case is only used twice I suggest standardizing on the first form. We can't guarantee it's a deliverable address, but it is almost certain to be. Simple patch attached.

@pishmishy
12 years ago

Standardizes sender address to admin_email

#17 @pishmishy
12 years ago

  • Keywords has-patch added

#18 @pishmishy
12 years ago

  • Status changed from new to assigned

#20 @lloydbudd
12 years ago

  • Milestone changed from 2.3.2 to 2.6

#21 @lloydbudd
12 years ago

  • Keywords regression has-patch removed

#22 @pishmishy
12 years ago

Why did the has-patch keyword get deleted? Is there a problem with the patch?

#23 @ryan
12 years ago

  • Keywords has-patch added

Added back has-patch, although I don't think we can use the patch because of #2053 and #1532. Using admin_email has its own problems.

#24 follow-up: @westi
12 years ago

(In [6599]) Revert #5007 as it causes more trouble than it solves. Fixes #5273 for trunk.

I have reverted this for 2.5

#25 @pishmishy
12 years ago

westi, despite owning the ticket I've not really been following it.

I'm a little confused by the relationship between this bug and #5273. Both seem to be problems caused by sending e-mail from "wordpress@" . $_SERVERSERVER_NAME? when $_SERVERSERVER_NAME? isn't necessarily a valid domain for sending mail. I guess there are also circumstances where it's not valid to use admin_email either. I think that makes solving these two tickets mutually exclusive.

What I might be tempted to suggest is standardizing on one address or the other and pointing out in the install process that the address must be valid for that server. Perhaps I'm just confused though =)

#26 in reply to: ↑ 24 @lloydbudd
12 years ago

Replying to westi:

(In [6599]) Revert #5007 as it causes more trouble than it solves. Fixes #5273 for trunk.

I have reverted this for 2.5

Ryan reverted in branch/2.3 today in preparation for a security release:
(In [6706]) Revert #5007 as it causes more trouble than it solves. Fixes #5273

#27 @pishmishy
12 years ago

I'd like to be able to close this particular ticket. It'd be nice to standardize on using get_option('admin_email') or "wordpress@" . $_SERVERSERVER_NAME? throughout but there'll be difficulty getting this right.

People's hosts might refuse to send mail from the first because it's not a domain they're aware of, hosts may refuse to send mail from the second because the wordpress@ address doesn't exist.

Could we document this in the code? If the issue appears again then this ticket can be found from the code. I'd be happy to close this ticket then.

#28 @Viper007Bond
12 years ago

Let's just pick something and then filter it. Plugins can take care of fringe cases.

#29 @pishmishy
12 years ago

I thought we did that, with my patch? It caused far more problems than the current situation does ;-)

#30 @pishmishy
12 years ago

  • Keywords has-patch removed

I think we could safely remove line 118 of wp-admin/includes/upgrade.php which would simplify any change.

#31 @Viper007Bond
12 years ago

Oh, well I guess I should pay attention then, huh? lol

@pishmishy
12 years ago

Document the problems in the code

#32 @janbrasna
11 years ago

  • Cc janbrasna added
  • Keywords e-mail notification sender spam added
  • Milestone changed from 2.9 to 2.8

Since both admin_email and wordpress@SERVER_NAME won't do for 100% of installations, another configuration field might be appropriate to set just the e–mail sender for notifications and other system e–mails.

Maybe just at the time of installation as not to clutter the blog settings?

#33 @jacobsantos
11 years ago

  • Milestone changed from 2.8 to 2.9

Documentation patch should be committed.

#34 @westi
11 years ago

(In [10575]) Add documentation to detail reasoning for default from address. See #5007 props pishmishy

#35 follow-up: @westi
11 years ago

I think we should close this ticket now we have that documentaion and there is a filter available for the % of sites where the current method doesn't work.

#36 in reply to: ↑ 35 @tigertech
11 years ago

Replying to westi:

I think we should close this ticket now we have that documentaion and there is a filter available for the % of sites where the current method doesn't work.

I'd like to object to closing this.

Having WordPress make up a fake address to send from, and then documenting that it doesn't work some of the time, is not a reasonable resolution to this problem.

Other scripts don't work this way. They either ask the user for a working address that the script is allowed to send from, or they just use the default setting without specifying any address, like WordPress originally did.

I run the mail system for a Web hosting company, and I can report that many extremely popular scripts send automated mail with no particular address set, allowing the system to fill in a valid address. The authors of those scripts expect it to work properly on properly configured servers, and it does; our mail logs show over 50,000 successful outgoing messages per day like this.

Because of that, I find it hard to believe that the original FastHosts problem (where they were blocking mail from scripts that didn't set a specific address) is widespread. The original bug here was not a WordPress problem, but rather a spam filtering bug at FastHosts that would prevent mail from being sent from many scripts. Heck, that system would prevent 3 of the 4 documentation examples for PHP's mail() function from working.

If the default address set on a server for script sending isn't allowed to send mail, that's a serious misconfiguration on the part of the hosting company, not something that WordPress should try to "fix" by making up a (probably nonexistent) address to send from.

The right solution is either to either ask the user for a working address that the script is allowed to send from, or to revert the original change so that WordPress uses the default address.

#37 @Denis-de-Bernardy
11 years ago

  • Keywords needs-patch added; e-mail notification sender spam removed
  • Severity changed from minor to normal

#38 @Denis-de-Bernardy
11 years ago

  • Component changed from General to Mail

#39 @aiusepsi
10 years ago

  • Version changed from 2.2.2 to 2.8.4

I'd also like to object to closing this. I just ran into this problem. The domain which the majority of things like my users' self-registration emails are going to be sent to is automatically spam-blocking the fake address Wordpress makes up here.

The way things work right now, the symptom from the user's side is that Wordpress inexplicably makes up an address, even when there's a setting in the admin interface. A filter makes it relatively simple to patch yourself, but you've got to dig though PHP to find the problem first.

#40 @westi
10 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

I closing this as fixed.

Please open a new ticket for any outstanding issue that you see.

#41 follow-up: @hakre
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I reopened this. The arguements given are totally resonable and should not be canceled that way. It's a serious issue.

#42 in reply to: ↑ 41 @westi
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to hakre:

I reopened this. The arguements given are totally resonable and should not be canceled that way. It's a serious issue.

Please open a new ticket for any outstanding issue that you see.

This ticket was mentioned in Slack in #cli by ipstenu. View the logs.


3 years ago

This ticket was mentioned in Slack in #core-editor by alexander-botteram. View the logs.


22 months ago

Note: See TracTickets for help on using tickets.