Ticket #5007 (assigned defect)

Opened 1 year ago

Last modified 7 months ago

Email notifications fail on hosted sites that check sender address

Reported by: jlwarlow Assigned to: pishmishy (accepted)
Priority: normal Milestone: 2.9
Component: General Version: 2.2.2
Severity: minor Keywords:
Cc:

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

patch.diff (489 bytes) - added by mattyrob on 09/23/07 10:42:31.
Patch
patch.2.diff (489 bytes) - added by mattyrob on 09/23/07 10:44:48.
5007.patch (1.0 kB) - added by pishmishy on 11/30/07 09:43:57.
Standardizes sender address to admin_email
5007.2.patch (1.6 kB) - added by pishmishy on 05/07/08 14:47:10.
Document the problems in the code

Change History

09/19/07 08:07:30 changed by Viper007Bond

  • keywords changed from phpmailer to has-patch 2nd-opinion.
  • version changed from 2.2.3 to 2.2.2.
  • milestone changed from 2.2.3 to 2.3.

09/23/07 10:42:31 changed by mattyrob

  • attachment patch.diff added.

Patch

09/23/07 10:44:48 changed by mattyrob

  • attachment patch.2.diff added.

09/23/07 10:45:27 changed by mattyrob

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.

09/27/07 19:56:18 changed by mattyrob

  • milestone changed from 2.4 to 2.3.1.

Can't we get this in for 2.3.1?

(follow-up: ↓ 8 ) 10/12/07 21:50:51 changed by 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.

10/17/07 20:17:01 changed by westi

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

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

10/17/07 20:17:29 changed by westi

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

10/17/07 20:19:01 changed by westi

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

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

(in reply to: ↑ 4 ; follow-up: ↓ 9 ) 10/31/07 20:00:45 changed by foolswisdom

  • keywords changed from has-patch 2nd-opinion to regression.
  • status changed from closed to reopened.
  • resolution deleted.
  • milestone changed from 2.3.1 to 2.3.2.

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".

(in reply to: ↑ 8 ; follow-up: ↓ 10 ) 11/01/07 17:42:23 changed by mattyrob

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.

(in reply to: ↑ 9 ) 11/01/07 18:25:37 changed by lloydbudd

Replying to mattyrob:

consistent and valid.

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

11/01/07 19:19:06 changed by westi

I guess the solution is going to be:

(follow-up: ↓ 13 ) 11/01/07 20:17:23 changed by 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.

(in reply to: ↑ 12 ; follow-up: ↓ 14 ) 11/01/07 22:12:49 changed by westi

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.

(in reply to: ↑ 13 ) 11/05/07 19:52:14 changed by mattyrob

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. :-(

11/30/07 08:58:14 changed by torbens

  • 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."

11/30/07 09:42:41 changed by pishmishy

  • 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.

11/30/07 09:43:57 changed by pishmishy

  • attachment 5007.patch added.

Standardizes sender address to admin_email

12/05/07 10:16:22 changed by pishmishy

  • keywords changed from regression to regression has-patch.

12/19/07 14:24:42 changed by pishmishy

  • status changed from new to assigned.

12/24/07 18:02:37 changed by ryan

see #2053 and #1532

01/05/08 00:45:58 changed by lloydbudd

  • milestone changed from 2.3.2 to 2.6.

01/05/08 00:46:08 changed by lloydbudd

  • keywords deleted.

01/06/08 17:22:28 changed by pishmishy

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

01/06/08 19:09:52 changed by ryan

  • keywords set to has-patch.

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.

(follow-up: ↓ 26 ) 01/11/08 18:45:36 changed by westi

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

I have reverted this for 2.5

01/30/08 14:59:57 changed by pishmishy

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 =)

(in reply to: ↑ 24 ) 02/02/08 17:48:57 changed by lloydbudd

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

05/07/08 13:20:31 changed by pishmishy

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.

05/07/08 13:26:12 changed by Viper007Bond

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

05/07/08 13:36:17 changed by pishmishy

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

05/07/08 13:50:28 changed by pishmishy

  • keywords deleted.

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

05/07/08 14:17:11 changed by Viper007Bond

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

05/07/08 14:47:10 changed by pishmishy

  • attachment 5007.2.patch added.

Document the problems in the code