Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#7009 closed defect (bug) (fixed)

New Email Feature Over SMTP failed to sent.

Reported by: singhleo's profile singhleo Owned by: djpaul's profile DJPaul
Milestone: 2.5.3 Priority: high
Severity: blocker Version:
Component: Emails Keywords: needs-testing has-patch
Cc:

Description

File Location :- buddypress/bp-core/classes/class-bp-phpmailer.php
Line no :- 57
Version :- 2.5.1>

Code
$phpmailer->Hostname = self::get_hostname();

Bug :- On Some Servers Mail are not being sent over smpt. when this setting is on. maybe because the host doesnt match with the smpt server.

When i have commented this line. all is working well as it should be.

I am sure this line is added for something ..
can anyone tell me why that line of code is needed. as we don't have this on wp_mail() source.

I apologize for not being word-perfect in English

Attachments (2)

0001-Mail-host-fix.patch (927 bytes) - added by singhleo 9 years ago.
Patch
0001-Hostname-remove.patch (1.4 KB) - added by singhleo 9 years ago.
Final Patch.

Download all attachments as: .zip

Change History (16)

#1 @DJPaul
9 years ago

  • Keywords reporter-feedback added

@singhleo Hi, thanks for opening a ticket! Your English is great. 👍

Look at the fix_phpmailer_messageid() function in WordPress; hostname is set if the site is multisite. The original idea with multisite was to build wordpress.com, with different domain names for each site, so I can understand why this had that property set there.

The question is why is setting the property for not-multisite harmful.

@singhleo Are you able to do some debugging and find out what the value of $phpmailer->Hostname is for a regular WordPress email (that gets sent okay), and what the value is for a BuddyPress email (that does not get sent)?

#2 follow-up: @needle
9 years ago

@singhleo Are the servers where SMTP fails running PHP 5.6? If so, it could be related to this:
http://php.net/manual/en/migration56.openssl.php

#3 @singhleo
9 years ago

  • Milestone changed from Awaiting Review to 2.5.3

@DJPaul
Hi Paul,

Thanks for letting me know that this is taken from wordpress which is added for multisite. thats make sense now.

after doing more deep debug. i found the victim.

multisite code is fine it has nothing wrong in it, which is.

<?php
function fix_phpmailer_messageid( $phpmailer ) {
        $phpmailer->Hostname = get_current_site()->domain;
}

So what it doing it setting the sub site subdomain to the host. and phpmailer uses it for message id header info right hand part.
eg. message-id@host

and what we are doing in BuddyPress for mail functionality is.

<?php
public static function get_hostname() {
                if ( is_multisite() ) {
                        return get_current_site()->domain;  // From fix_phpmailer_messageid()
                }

                return preg_replace( '#^https?://#i', '', bp_get_option( 'home' ) );
        }

So from above code.

return get_current_site()->domain; From fix_phpmailer_messageid() <-- this line is correct which is taken from multisite setup.

But problem is with this line.
return preg_replace( '#https?:#i', , bp_get_option( 'home' ) );

which is returning sub site url with sub paths (eg. domain.com/subsite1) instead of host (eg, domain.com).
as host name cannot contain the subsite directory and http://. and in most cases its subsite.

on wordpress they used get_current_site()->domain which returns the domain url eg. www.domain.com always even client is using multisite on subdir instead of subdomains.

so it returns
www.domain.com

even if
sub site url is :-
www.domain.com/subsite1
subsite1.domain.com

So in BuddyPress mail api that function code should be changed to something..

<?php
public static function get_hostname() {
                if ( is_multisite() ) {
                        return get_current_site()->domain;  // From fix_phpmailer_messageid()
                }

                $home = bp_get_option( 'home' );
                $home = parse_url($home);
                return $home["host"];
        }
Last edited 9 years ago by singhleo (previous) (diff)

#4 in reply to: ↑ 2 @singhleo
9 years ago

Replying to needle:

@singhleo Are the servers where SMTP fails running PHP 5.6? If so, it could be related to this:
http://php.net/manual/en/migration56.openssl.php

Where i tested is using PHP Version 5.4.44

#5 follow-up: @DJPaul
9 years ago

  • Keywords dev-feedback reporter-feedback removed
  • Owner set to DJPaul
  • Status changed from new to reviewing

@singhleo Very nice investigation! Well done, and big thanks. :)

#6 @DJPaul
9 years ago

  • Status changed from reviewing to accepted

#7 in reply to: ↑ 5 @singhleo
9 years ago

Replying to DJPaul:

@singhleo Very nice investigation! Well done, and big thanks. :)

Gr8 thanks paul. :)

@singhleo
9 years ago

Patch

#8 @singhleo
9 years ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #buddypress by djpaul. View the logs.


9 years ago

#10 @singhleo
9 years ago

@DJPaul
Hi Paul after discussion on slack with jjj i have noticed that wordpress is using phpmailer_init to add hostname.
since after 2.0.5 we have added phpmailer_init hook so we no longer need get_hostname() method and $phpmailer->Hostname = self::get_hostname();

i am updating the patch.

Last edited 9 years ago by singhleo (previous) (diff)

@singhleo
9 years ago

Final Patch.

#11 @djpaul
9 years ago

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

In 10701:

Emails: don't set PHPMailer hostname property

Prior to 2.5.1, this was needed to support multisite. Now that we
support the phpmailer_init action, this isn't needed.

Removing it also fixes a problem where - on some servers - emails were
not being sent if non-multisite WordPress was running from a subfolder.

Fixes #7009 (2.5 branch)

Props singhleo

#12 @djpaul
9 years ago

In 10702:

Emails: don't set PHPMailer hostname property

Prior to 2.5.1, this was needed to support multisite. Now that we
support the phpmailer_init action, this isn't needed.

Removing it also fixes a problem where - on some servers - emails were
not being sent if non-multisite WordPress was running from a subfolder.

Fixes #7009

Props singhleo

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


9 years ago

This ticket was mentioned in Slack in #buddypress by codeart. View the logs.


9 years ago

Note: See TracTickets for help on using tickets.