r91668 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91667‎ | r91668 | r91669 >
Date:19:11, 7 July 2011
Author:mah
Status:ok (Comments)
Tags:
Comment:
Fixes Bug#17866

Minimal check that we have more than an empty string before sending and email. NikeRabbit asked me about LQT sending email to users w/o email addresses and this looked like one good place to add a check.
Modified paths:
  • /trunk/phase3/includes/UserMailer.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/UserMailer.php
——@@ -55,16 +55,18 @@
5656 # PHP's mail() implementation under Windows is somewhat shite, and
5757 # can't handle "Joe Bloggs <joe@bloggs.com>" format email addresses,
5858 # so don't bother generating them
59 - if ( $this->name != '' && !wfIsWindows() ) {
60 - global $wgEnotifUseRealName;
61 - $name = ( $wgEnotifUseRealName && $this->realName ) ? $this->realName : $this->name;
62 - $quoted = UserMailer::quotedPrintable( $name );
63 - if ( strpos( $quoted, '.' ) !== false || strpos( $quoted, ',' ) !== false ) {
64 - $quoted = '"' . $quoted . '"';
 59+ if ( $this->address ) {
 60+ if ( $this->name != '' && !wfIsWindows() ) {
 61+ global $wgEnotifUseRealName;
 62+ $name = ( $wgEnotifUseRealName && $this->realName ) ? $this->realName : $this->name;
 63+ $quoted = UserMailer::quotedPrintable( $name );
 64+ if ( strpos( $quoted, '.' ) !== false || strpos( $quoted, ',' ) !== false ) {
 65+ $quoted = '"' . $quoted . '"';
 66+ }
 67+ return "$quoted <{$this->address}>";
 68+ } else {
 69+ return $this->address;
6570 }
66 - return "$quoted <{$this->address}>";
67 - } else {
68 - return $this->address;
6971 }
7072 }
7173

Follow-up revisions

RevisionCommit summaryAuthorDate
r93412further sanity checks for sending email … bomb out if we aren't given...mah19:15, 28 July 2011

Comments

#Comment by Bawolff (talk | contribs)   01:23, 8 July 2011

I think there is a larger issue here involving confirming email. It should be impossible to send an email if your email is not verified. You can't verify an email if you don't have one set, so it should be impossible to get to this point if you don't have an email set. (Of course checking there is an email is probably a good sanity check nonetheless)

#Comment by MarkAHershberger (talk | contribs)   19:37, 8 July 2011

I agree that there is probably something that should be fixed in LQT so that it checks to make sure there is actually an email address to send to.

In fact this doesn't solve the problem. Emails can still be sent w/o an actual address.

#Comment by Bawolff (talk | contribs)   19:43, 8 July 2011

I think the issue is more in where-ever we do verifications that the email is authenticated. Really its not LQT's job to do that, but should use the same code as special:emailuser/watchlist notifications/etc uses. (I'm assuming said code exists somewhere, I don't know enough about email stuff to say if that's actually the case or not)

Status & tagging log