Skip to content

Commit f9bfcb1

Browse files
alzotaalzota
committed
ACP2E-2125: unified validation for utf-8 email addresses
1 parent 13e54e1 commit f9bfcb1

File tree

9 files changed

+120
-64
lines changed

9 files changed

+120
-64
lines changed

‎app/code/Magento/Customer/etc/di.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,9 @@
578578
<item name="store_id" xsi:type="string">store_id</item>
579579
<item name="group_id" xsi:type="string">group_id</item>
580580
<item name="dob" xsi:type="string">dob</item>
581+
<item name="rp_token" xsi:type="string">rp_token</item>
582+
<item name="rp_token_created_at" xsi:type="string">rp_token_created_at</item>
583+
<item name="password_hash" xsi:type="string">password_hash</item>
581584
</item>
582585
<item name="customer_address" xsi:type="array">
583586
<item name="country_id" xsi:type="string">country_id</item>

‎app/code/Magento/Eav/Model/Attribute/Data/Text.php

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,32 +6,39 @@
66

77
namespace Magento\Eav\Model\Attribute\Data;
88

9+
use Magento\Eav\Model\Attribute;
910
use Magento\Framework\App\RequestInterface;
11+
use Magento\Framework\Exception\LocalizedException;
12+
use Magento\Framework\Locale\ResolverInterface;
13+
use Magento\Framework\Stdlib\DateTime\TimezoneInterface;
14+
use Magento\Framework\Stdlib\StringUtils;
15+
use Psr\Log\LoggerInterface;
1016

1117
/**
1218
* EAV Entity Attribute Text Data Model
1319
*
1420
* @author Magento Core Team <core@magentocommerce.com>
21+
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
1522
*/
1623
class Text extends \Magento\Eav\Model\Attribute\Data\AbstractData
1724
{
1825
/**
19-
* @var \Magento\Framework\Stdlib\StringUtils
26+
* @var StringUtils
2027
*/
2128
protected $_string;
2229

2330
/**
24-
* @param \Magento\Framework\Stdlib\DateTime\TimezoneInterface $localeDate
25-
* @param \Psr\Log\LoggerInterface $logger
26-
* @param \Magento\Framework\Locale\ResolverInterface $localeResolver
27-
* @param \Magento\Framework\Stdlib\StringUtils $stringHelper
31+
* @param TimezoneInterface $localeDate
32+
* @param LoggerInterface $logger
33+
* @param ResolverInterface $localeResolver
34+
* @param StringUtils $stringHelper
2835
* @codeCoverageIgnore
2936
*/
3037
public function __construct(
31-
\Magento\Framework\Stdlib\DateTime\TimezoneInterface $localeDate,
32-
\Psr\Log\LoggerInterface $logger,
33-
\Magento\Framework\Locale\ResolverInterface $localeResolver,
34-
\Magento\Framework\Stdlib\StringUtils $stringHelper
38+
TimezoneInterface $localeDate,
39+
LoggerInterface $logger,
40+
ResolverInterface $localeResolver,
41+
StringUtils $stringHelper
3542
) {
3643
parent::__construct($localeDate, $logger, $localeResolver);
3744
$this->_string = $stringHelper;
@@ -79,9 +86,6 @@ public function validateValue($value)
7986
return $errors;
8087
}
8188

82-
// if string with diacritics encode it.
83-
$value = $this->encodeDiacritics($value);
84-
8589
$validateLengthResult = $this->validateLength($attribute, $value);
8690
$errors = array_merge($errors, $validateLengthResult);
8791

@@ -100,6 +104,7 @@ public function validateValue($value)
100104
*
101105
* @param array|string $value
102106
* @return $this
107+
* @throws LocalizedException
103108
*/
104109
public function compactValue($value)
105110
{
@@ -127,6 +132,7 @@ public function restoreValue($value)
127132
* @param string $format
128133
* @return string|array
129134
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
135+
* @throws LocalizedException
130136
*/
131137
public function outputValue($format = \Magento\Eav\Model\AttributeDataFactory::OUTPUT_FORMAT_TEXT)
132138
{
@@ -139,11 +145,11 @@ public function outputValue($format = \Magento\Eav\Model\AttributeDataFactory::O
139145
/**
140146
* Validates value length by attribute rules
141147
*
142-
* @param \Magento\Eav\Model\Attribute $attribute
148+
* @param Attribute $attribute
143149
* @param string $value
144150
* @return array errors
145151
*/
146-
private function validateLength(\Magento\Eav\Model\Attribute $attribute, string $value): array
152+
private function validateLength(Attribute $attribute, string $value): array
147153
{
148154
$errors = [];
149155
$length = $this->_string->strlen(trim($value));
@@ -176,19 +182,4 @@ private function validateInputRule(string $value): array
176182
$result = $this->_validateInputRule($value);
177183
return \is_array($result) ? $result : [];
178184
}
179-
180-
/**
181-
* Encode strings with diacritics for validate.
182-
*
183-
* @param array|string $value
184-
* @return array|string
185-
*/
186-
private function encodeDiacritics($value): array|string
187-
{
188-
$encoded = $value;
189-
if (is_string($value)) {
190-
$encoded = iconv('UTF-8', 'ASCII//TRANSLIT//IGNORE', $value);
191-
}
192-
return $encoded;
193-
}
194185
}

‎app/code/Magento/Sales/Model/Order/Address/Validator.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Eav\Model\Config as EavConfig;
1212
use Magento\Framework\App\ObjectManager;
1313
use Magento\Framework\Exception\LocalizedException;
14+
use Magento\Framework\Validator\EmailAddress as EmailAddressValidator;
1415
use Magento\Sales\Model\Order\Address;
1516

1617
/**
@@ -48,20 +49,29 @@ class Validator
4849
*/
4950
protected $eavConfig;
5051

52+
/**
53+
* @var EmailAddressValidator
54+
*/
55+
private $emailAddressValidator;
56+
5157
/**
5258
* @param DirectoryHelper $directoryHelper
5359
* @param CountryFactory $countryFactory
54-
* @param EavConfig $eavConfig
60+
* @param EavConfig|null $eavConfig
61+
* @param EmailAddressValidator|null $emailAddressValidator
5562
*/
5663
public function __construct(
5764
DirectoryHelper $directoryHelper,
5865
CountryFactory $countryFactory,
59-
EavConfig $eavConfig = null
66+
EavConfig $eavConfig = null,
67+
EmailAddressValidator $emailAddressValidator = null
6068
) {
6169
$this->directoryHelper = $directoryHelper;
6270
$this->countryFactory = $countryFactory;
6371
$this->eavConfig = $eavConfig ?: ObjectManager::getInstance()
6472
->get(EavConfig::class);
73+
$this->emailAddressValidator = $emailAddressValidator ?: ObjectManager::getInstance()
74+
->get(EmailAddressValidator::class);
6575
}
6676

6777
/**
@@ -91,9 +101,13 @@ public function validate(Address $address)
91101
$warnings[] = sprintf('"%s" is required. Enter and try again.', $label);
92102
}
93103
}
94-
if (!filter_var($address->getEmail(), FILTER_VALIDATE_EMAIL)) {
104+
105+
$email = $address->getEmail();
106+
107+
if (empty($email) || $this->emailAddressValidator->isValid($email)) {
95108
$warnings[] = 'Email has a wrong format';
96109
}
110+
97111
if (!in_array($address->getAddressType(), [Address::TYPE_BILLING, Address::TYPE_SHIPPING])) {
98112
$warnings[] = 'Address type doesn\'t match required options';
99113
}

‎app/code/Magento/Sales/Test/Unit/Model/Order/Address/ValidatorTest.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use Magento\Directory\Model\CountryFactory;
1212
use Magento\Eav\Model\Config;
1313
use Magento\Eav\Model\Entity\Attribute;
14+
use Magento\Framework\Validator\EmailAddress;
1415
use Magento\Sales\Model\Order\Address;
1516
use Magento\Sales\Model\Order\Address\Validator;
1617
use PHPUnit\Framework\MockObject\MockObject;
@@ -38,6 +39,11 @@ class ValidatorTest extends TestCase
3839
*/
3940
protected $countryFactoryMock;
4041

42+
/**
43+
* @var EmailAddress|MockObject
44+
*/
45+
private $emailValidatorMock;
46+
4147
/**
4248
* Mock order address model
4349
*/
@@ -57,10 +63,12 @@ protected function setUp(): void
5763
$eavConfigMock->expects($this->any())
5864
->method('getAttribute')
5965
->willReturn($attributeMock);
66+
$this->emailValidatorMock = $this->createMock(EmailAddress::class);
6067
$this->validator = new Validator(
6168
$this->directoryHelperMock,
6269
$this->countryFactoryMock,
63-
$eavConfigMock
70+
$eavConfigMock,
71+
$this->emailValidatorMock
6472
);
6573
}
6674

@@ -84,6 +92,10 @@ public function testValidate($addressData, $email, $addressType, $expectedWarnin
8492
$this->addressMock->expects($this->once())
8593
->method('getAddressType')
8694
->willReturn($addressType);
95+
$this->emailValidatorMock->expects($this->once())
96+
->method('isValid')
97+
->with($email)
98+
->willReturn((stripos($email, '@') === false));
8799
$actualWarnings = $this->validator->validate($this->addressMock);
88100
$this->assertEquals($expectedWarnings, $actualWarnings);
89101
}

‎dev/tests/api-functional/testsuite/Magento/GraphQl/Customer/CreateCustomerTest.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,22 @@ protected function setUp(): void
3636
}
3737

3838
/**
39+
* @dataProvider validEmailAddressDataProvider
3940
* @throws \Exception
4041
*/
41-
public function testCreateCustomerAccountWithPassword()
42+
public function testCreateCustomerAccountWithPassword(string $email)
4243
{
4344
$newFirstname = 'Richard';
4445
$newLastname = 'Rowe';
4546
$currentPassword = 'test123#';
46-
$newEmail = 'new_customer@example.com';
4747

4848
$query = <<<QUERY
4949
mutation {
5050
createCustomer(
5151
input: {
5252
firstname: "{$newFirstname}"
5353
lastname: "{$newLastname}"
54-
email: "{$newEmail}"
54+
email: "{$email}"
5555
password: "{$currentPassword}"
5656
is_subscribed: true
5757
}
@@ -71,10 +71,22 @@ public function testCreateCustomerAccountWithPassword()
7171
$this->assertNull($response['createCustomer']['customer']['id']);
7272
$this->assertEquals($newFirstname, $response['createCustomer']['customer']['firstname']);
7373
$this->assertEquals($newLastname, $response['createCustomer']['customer']['lastname']);
74-
$this->assertEquals($newEmail, $response['createCustomer']['customer']['email']);
74+
$this->assertEquals($email, $response['createCustomer']['customer']['email']);
7575
$this->assertTrue($response['createCustomer']['customer']['is_subscribed']);
7676
}
7777

78+
/**
79+
* @return array
80+
*/
81+
public function validEmailAddressDataProvider(): array
82+
{
83+
return [
84+
['new_customer@example.com'],
85+
['jØrgen@somedomain.com'],
86+
['“email”@example.com']
87+
];
88+
}
89+
7890
/**
7991
* @throws \Exception
8092
*/
@@ -217,15 +229,13 @@ public function invalidEmailAddressDataProvider(): array
217229
{
218230
return [
219231
['plainaddress'],
220-
['jØrgen@somedomain.com'],
221232
['#@%^%#$@#$@#.com'],
222233
['@example.com'],
223234
['Joe Smith <email@example.com>'],
224235
['email.example.com'],
225236
['email@example@example.com'],
226237
['email@example.com (Joe Smith)'],
227-
['email@example'],
228-
['“email”@example.com'],
238+
['email@example']
229239
];
230240
}
231241

‎dev/tests/api-functional/testsuite/Magento/GraphQl/Customer/CreateCustomerV2Test.php

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,22 @@ protected function setUp(): void
3636
}
3737

3838
/**
39-
* @magentoConfigFixture default_store newsletter/general/active 1
39+
* @dataProvider validEmailAddressDataProvider
4040
* @throws \Exception
4141
*/
42-
public function testCreateCustomerAccountWithPassword()
42+
public function testCreateCustomerAccountWithPassword(string $email)
4343
{
4444
$newFirstname = 'Richard';
4545
$newLastname = 'Rowe';
4646
$currentPassword = 'test123#';
47-
$newEmail = 'new_customer@example.com';
4847

4948
$query = <<<QUERY
5049
mutation {
5150
createCustomerV2(
5251
input: {
5352
firstname: "{$newFirstname}"
5453
lastname: "{$newLastname}"
55-
email: "{$newEmail}"
54+
email: "{$email}"
5655
password: "{$currentPassword}"
5756
is_subscribed: true
5857
}
@@ -72,10 +71,22 @@ public function testCreateCustomerAccountWithPassword()
7271
$this->assertNull($response['createCustomerV2']['customer']['id']);
7372
$this->assertEquals($newFirstname, $response['createCustomerV2']['customer']['firstname']);
7473
$this->assertEquals($newLastname, $response['createCustomerV2']['customer']['lastname']);
75-
$this->assertEquals($newEmail, $response['createCustomerV2']['customer']['email']);
74+
$this->assertEquals($email, $response['createCustomerV2']['customer']['email']);
7675
$this->assertTrue($response['createCustomerV2']['customer']['is_subscribed']);
7776
}
7877

78+
/**
79+
* @return array
80+
*/
81+
public function validEmailAddressDataProvider(): array
82+
{
83+
return [
84+
['new_customer@example.com'],
85+
['jØrgenV2@somedomain.com'],
86+
['“emailV2”@example.com']
87+
];
88+
}
89+
7990
/**
8091
* @throws \Exception
8192
*/
@@ -118,7 +129,7 @@ public function testCreateCustomerAccountWithoutPassword()
118129
public function testCreateCustomerIfInputDataIsEmpty()
119130
{
120131
$this->expectException(\Exception::class);
121-
$this->expectExceptionMessageMatches('/of required type String! was not provided./');
132+
$this->expectExceptionMessage('CustomerCreateInput.email of required type String! was not provided.');
122133

123134
$query = <<<QUERY
124135
mutation {
@@ -218,15 +229,13 @@ public function invalidEmailAddressDataProvider(): array
218229
{
219230
return [
220231
['plainaddress'],
221-
['jØrgen@somedomain.com'],
222232
['#@%^%#$@#$@#.com'],
223233
['@example.com'],
224234
['Joe Smith <email@example.com>'],
225235
['email.example.com'],
226236
['email@example@example.com'],
227237
['email@example.com (Joe Smith)'],
228-
['email@example'],
229-
['“email”@example.com'],
238+
['email@example']
230239
];
231240
}
232241

0 commit comments

Comments
 (0)