Maniphest T207541

php mail() to argument is MailAddress Object instead of string
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	DonPaolo
	Oct 20 2018, 12:19 PM

Description

Mediawiki 1.31.

When php's mail() function in includes/mail/UserMailer.php line ~433 is invoked, the first argument it receives is a MailAddress Object instead of a string.

This way the mail in sent, but the recipients are not treated correctly, and the mail produces an error in the mail agent.

I'm getting that through a command like:

UserMailer::send( $targetAddress, $submitterAddress, $subject, $this->text, $options );

I've checked that the UserMailer::send's parameters have their correct object type.

Details

Subject	Repo	Branch	Lines +/-
Don't pass a MailAddress pass the email to mail()	mediawiki/core	REL1_30	+2 -1
Don't pass a MailAddress pass the email to mail()	mediawiki/core	REL1_27	+2 -1
Don't pass a MailAddress pass the email to mail()	mediawiki/core	REL1_31	+2 -1
Don't pass a MailAddress pass the email to mail()	mediawiki/core	REL1_32	+1 -1
Don't pass a MailAddress pass the email to mail()	mediawiki/core	master	+1 -1

Customize query in gerrit

Event Timeline

DonPaolo created this task.Oct 20 2018, 12:19 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 20 2018, 12:19 PM

DonPaolo updated the task description. (Show Details)Oct 20 2018, 12:28 PM

Reedy edited projects, added MediaWiki-Email; removed MediaWiki-General.Oct 20 2018, 12:32 PM

Change 468699 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468699

gerritbot added a project: Patch-For-Review.Oct 20 2018, 12:39 PM

In a similar way, in line 312:

$headers['Reply-To'] = $replyto->address;

line 4101 too:

$headers['To'] = $to[0]->address;

line 291:

$headers['From'] = $from->address;

In T207541#4682592, @DonPaolo wrote:
In a similar way, in line 312:
$headers['Reply-To'] = $replyto->address;

In master, there is already

		if ( $replyto ) {
			$headers['Reply-To'] = $replyto->toString();
		}

In T207541#4682594, @DonPaolo wrote:
line 4101 too:
$headers['To'] = $to[0]->address;

Line 4101?

Line 400 on master has

				$headers['To'] = $to[0]->toString();

In T207541#4682594, @DonPaolo wrote:
line 291:
$headers['From'] = $from->address;

Already fixed in master

		$headers['From'] = $from->toString();

Legoktm added a project: MW-1.31-release.Oct 20 2018, 3:42 PM

Change 468699 merged by jenkins-bot:
[mediawiki/core@master] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468699

Change 468774 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_32] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468774

Change 468775 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_31] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468775

Change 468776 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_30] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468776

Change 468777 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_27] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468777

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.1; 2018-10-23).Oct 20 2018, 9:00 PM

I'd like to be wrong, but the toString() method doesn't return the email address but the sender name

	/**
	 * Return formatted and quoted address to insert into SMTP headers
	 * @return string
	 */
	function toString() {
		# PHP's mail() implementation under Windows is somewhat shite, and
		# can't handle "Joe Bloggs <joe@bloggs.com>" format email addresses,
		# so don't bother generating them
		if ( $this->address ) {
			if ( $this->name != '' && !wfIsWindows() ) {
				global $wgEnotifUseRealName;
				$name = ( $wgEnotifUseRealName && $this->realName !== '' ) ? $this->realName : $this->name;
				$quoted = UserMailer::quotedPrintable( $name );
				// Must only be quoted if string does not use =? encoding (T191931)
				if ( $quoted === $name ) {
					$quoted = '"' . addslashes( $quoted ) . '"';
				}
				return "$quoted <{$this->address}>";
			} else {
				return $this->address;
			}
		} else {
			return "";
		}
	}

I'm a bit confused now...MailAddress implements __toString...is that no longer working?

Change 468774 merged by jenkins-bot:
[mediawiki/core@REL1_32] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468774

ReleaseTaggerBot added a project: MW-1.32-notes.Oct 21 2018, 4:01 PM

Change 468775 merged by jenkins-bot:
[mediawiki/core@REL1_31] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468775

ReleaseTaggerBot added a project: MW-1.31-release-notes.Oct 21 2018, 6:00 PM

Change 468777 merged by jenkins-bot:
[mediawiki/core@REL1_27] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468777

Change 468776 merged by jenkins-bot:
[mediawiki/core@REL1_30] Don't pass a MailAddress pass the email to mail()

https://gerrit.wikimedia.org/r/468776

ReleaseTaggerBot added projects: MW-1.30-release-notes, MW-1.27-release-notes.Oct 25 2018, 6:00 PM

@Reedy Can we mark resolved?

Reedy closed this task as Resolved.Oct 31 2018, 12:35 PM

Reedy claimed this task.

Reedy removed a project: Patch-For-Review.

php mail() to argument is MailAddress Object instead of stringClosed, ResolvedPublicActions

Description

Details

Event Timeline

php mail() to argument is MailAddress Object instead of string
Closed, ResolvedPublic
Actions