Page MenuHomePhabricator

D8320.diff
No OneTemporary

D8320.diff

Index: src/__phutil_library_map__.php
===================================================================
--- src/__phutil_library_map__.php
+++ src/__phutil_library_map__.php
@@ -2177,6 +2177,7 @@
'PhabricatorUserEditor' => 'applications/people/editor/PhabricatorUserEditor.php',
'PhabricatorUserEditorTestCase' => 'applications/people/editor/__tests__/PhabricatorUserEditorTestCase.php',
'PhabricatorUserEmail' => 'applications/people/storage/PhabricatorUserEmail.php',
+ 'PhabricatorUserEmailTestCase' => 'applications/people/storage/__tests__/PhabricatorUserEmailTestCase.php',
'PhabricatorUserLog' => 'applications/people/storage/PhabricatorUserLog.php',
'PhabricatorUserPreferences' => 'applications/settings/storage/PhabricatorUserPreferences.php',
'PhabricatorUserProfile' => 'applications/people/storage/PhabricatorUserProfile.php',
@@ -5000,6 +5001,7 @@
'PhabricatorUserEditor' => 'PhabricatorEditor',
'PhabricatorUserEditorTestCase' => 'PhabricatorTestCase',
'PhabricatorUserEmail' => 'PhabricatorUserDAO',
+ 'PhabricatorUserEmailTestCase' => 'PhabricatorTestCase',
'PhabricatorUserLog' => 'PhabricatorUserDAO',
'PhabricatorUserPreferences' => 'PhabricatorUserDAO',
'PhabricatorUserProfile' => 'PhabricatorUserDAO',
Index: src/applications/people/storage/PhabricatorUserEmail.php
===================================================================
--- src/applications/people/storage/PhabricatorUserEmail.php
+++ src/applications/people/storage/PhabricatorUserEmail.php
@@ -37,6 +37,23 @@
return false;
}
+ // Very roughly validate that this address isn't so mangled that a
+ // reasonable piece of code might completely misparse it. In particular,
+ // the major risks are:
+ //
+ // - `PhutilEmailAddress` needs to be able to extract the domain portion
+ // from it.
+ // - Reasonable mail adapters should be hard-pressed to interpret one
+ // address as several addresses.
+ //
+ // To this end, we're roughly verifying that there's some normal text, an
+ // "@" symbol, and then some more normal text.
+
+ $email_regex = '(^[a-z0-9_+.!-]+@[a-z0-9_+:.-]+$)i';
+ if (!preg_match($email_regex, $address)) {
+ return false;
+ }
+
return true;
}
@@ -46,7 +63,8 @@
*/
public static function describeValidAddresses() {
return pht(
- 'The maximum length of an email address is %d character(s).',
+ "Email addresses should be in the form 'user@domain.com'. The maximum ".
+ "length of an email address is %d character(s).",
new PhutilNumber(self::MAX_ADDRESS_LENGTH));
}
Index: src/applications/people/storage/__tests__/PhabricatorUserEmailTestCase.php
===================================================================
--- /dev/null
+++ src/applications/people/storage/__tests__/PhabricatorUserEmailTestCase.php
@@ -0,0 +1,41 @@
+<?php
+
+final class PhabricatorUserEmailTestCase extends PhabricatorTestCase {
+
+ public function testEmailValidation() {
+ $tests = array(
+ 'alincoln@whitehouse.gov' => true,
+ '_-.@.-_' => true,
+ '.@.com' => true,
+ 'user+suffix@gmail.com' => true,
+ 'IAMIMPORTANT@BUSINESS.COM' => true,
+ '1@22.33.44.55' => true,
+ '999@999.999' => true,
+ 'user@2001:0db8:85a3:0042:1000:8a2e:0370:7334' => true,
+ '!..!@o.O' => true,
+
+ '' => false,
+ str_repeat('a', 256).'@example.com' => false,
+ 'quack' => false,
+ '@gmail.com' => false,
+ 'usergmail.com' => false,
+ '"user" user@gmail.com' => false,
+ 'a,b@evil.com' => false,
+ 'a;b@evil.com' => false,
+ 'ab@evil.com;cd@evil.com' => false,
+ 'x@y@z.com' => false,
+ '@@' => false,
+ '@' => false,
+ 'user@' => false,
+ );
+
+ foreach ($tests as $input => $expect) {
+ $actual = PhabricatorUserEmail::isValidAddress($input);
+ $this->assertEqual(
+ $expect,
+ $actual,
+ $input);
+ }
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Wed, Oct 23, 1:21 PM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6718290
Default Alt Text
D8320.diff (3 KB)

Event Timeline