r94727 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r94726‎ | r94727 | r94728 >
Date:03:49, 17 August 2011
Author:nagelp
Status:ok (Comments)
Tags:
Comment:
Added support for specifying multiple (up to 10) addresses
Modified paths:
  • /trunk/extensions/Notificator/Notificator.body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Notificator/Notificator.body.php
——@@ -89,17 +89,33 @@
9090
9191 public static function receiverIsValid( $receiver ) {
9292 // Returns true if the parameter is a valid e-mail address, false if not
93 - $receiverIsValid = false;
 93+ $receiverIsValid = true;
9494
 95+ // There may be multiple e-mail addresses, divided by commas - which is valid
 96+ // for us, but not for the validation functions we use below. So get the single
 97+ // address into an array first, validate them one by one, and only if all are ok,
 98+ // return true.
 99+ $receiverArray = explode( ',', str_replace ( ', ', ',', $receiver ) );
 100+
 101+ // To make sure some joker doesn't copy in a large number of e-mail addresses
 102+ // and spams them all, lets set a (admittedly arbitrary) limit of 10.
 103+ if ( count( $receiverArray ) > 10 ) {
 104+ return false;
 105+ }
 106+
 107+ if ( method_exists( 'Sanitizer', 'validateEmail' ) ) {
95108 // User::isValidEmailAddr() has been moved to Sanitizer::validateEmail as of
96109 // MediaWiki version 1.18 (I think).
97 - if ( method_exists( 'Sanitizer', 'validateEmail' ) ) {
98 - if ( Sanitizer::validateEmail( $receiver ) ) {
99 - $receiverIsValid = true;
 110+ foreach ( $receiverArray as $singleEmailAddress ) {
 111+ if ( ! Sanitizer::validateEmail( $singleEmailAddress ) ) {
 112+ $receiverIsValid = false;
 113+ }
100114 }
101115 } else {
102 - if ( User::isValidEmailAddr( $receiver ) ) {
103 - $receiverIsValid = true;
 116+ foreach ( $receiverArray as $singleEmailAddress ) {
 117+ if ( ! User::isValidEmailAddr( $singleEmailAddress ) ) {
 118+ $receiverIsValid = false;
 119+ }
104120 }
105121 }
106122 return $receiverIsValid;

Follow-up revisions

RevisionCommit summaryAuthorDate
r94857Followup to r94727 - no space after ! in if conditionsnagelp03:02, 18 August 2011

Comments

#Comment by Nikerabbit (talk | contribs)   10:29, 17 August 2011

Style nitpick: no space after ! in if conditions.

I'd call $receiverArray just $receivers. Instead of doing replacing ', ' with ',' I'd just run trim on the exploded array.

#Comment by Patrick Nagel (talk | contribs)   03:15, 18 August 2011

I thought about using trim(), but I'm not sure how the mailer function, that will get the untrimmed string as it came in from the user, will cope with \t, \n, \r, \0 or \x0B. I only know for sure that it can handle spaces. That's why I just remove the spaces for validation, and if there are any of the above listed characters still left, the validation function will just fail, and the list of e-mail addresses will not be used.

In short - if I use trim() here, I need to go through string -> array -> trim(strings in array) -> string again somewhere else, and I prefer to keep it simple for now.

#Comment by Nikerabbit (talk | contribs)   13:13, 18 August 2011

Hmm right you are only exploding it for validation. But still, it is only two lines of code, and trim removes almost all those characters: 

$f = array_map( 'trim', explode( ',', $receivers ) );
$receivers = implode( ', ', $f );

Status & tagging log