Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

Remove IDNA conversion of local-part #66

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/EmailAddress.php
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ protected function validateLocalPart()
// atext: ALPHA / DIGIT / and "!", "#", "$", "%", "&", "'", "*",
// "+", "-", "/", "=", "?", "^", "_", "`", "{", "|", "}", "~"
$atext = 'a-zA-Z0-9\x21\x23\x24\x25\x26\x27\x2a\x2b\x2d\x2f\x3d\x3f\x5e\x5f\x60\x7b\x7c\x7d\x7e';
if (preg_match('/^[' . $atext . ']+(\x2e+[' . $atext . ']+)*$/', $this->idnToAscii($this->localPart))) {
if (preg_match('/^[' . $atext . ']+(\x2e+[' . $atext . ']+)*$/', $this->localPart)) {
$result = true;
} else {
// Try quoted string format (RFC 5321 Chapter 4.1.2)
Expand Down Expand Up @@ -379,7 +379,7 @@ protected function validateMXRecords()
{
$mxHosts = [];
$weight = [];
$result = getmxrr($this->idnToAscii($this->hostname), $mxHosts, $weight);
$result = getmxrr($this->hostname, $mxHosts, $weight);
if (!empty($mxHosts) && !empty($weight)) {
$this->mxRecord = array_combine($mxHosts, $weight) ?: [];
} else {
Expand Down Expand Up @@ -473,7 +473,7 @@ protected function splitEmailParts($value)
}

$this->localPart = $matches[1];
$this->hostname = $matches[2];
$this->hostname = $this->idnToAscii($matches[2]);

return true;
}
Expand All @@ -497,7 +497,7 @@ public function isValid($value)
}

$length = true;
$this->setValue($this->idnToUtf8($value));
$this->setValue($value);

// Split email address up and disallow '..'
if (!$this->splitEmailParts($this->getValue())) {
Expand Down
14 changes: 11 additions & 3 deletions test/EmailAddressTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public function testEmailDisplay()
public function validEmailAddresses()
{
// @codingStandardsIgnoreStart
return [
$return = [
'[email protected]' => ['[email protected]'],
'[email protected]' => ['[email protected]'],
'[email protected]' => ['[email protected]'],
Expand All @@ -232,6 +232,13 @@ public function validEmailAddresses()
'bob@verylongdomainsupercalifragilisticexpialidociousspoonfulofsugar.com' => ['bob@verylongdomainsupercalifragilisticexpialidociousspoonfulofsugar.com'],
"B.O'[email protected]" => ["B.O'[email protected]"],
];

if (extension_loaded('intl')) {
$return['bob@موقع.إختبار'] = ['bob@موقع.إختبار'];
$return['[email protected]'] = ['[email protected]'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've merged #67 at this point, and just attempted to merge this PR. When running tests, both of these specific cases fail, each with:

not a valid hostname for the email address
The input appears to be a DNS hostname but cannot match TLD against known list
The input appears to be a local network name but local network names are not allowed
Failed asserting that false is true.

I'm not sure if that's expected at this point, and you have more work to do, or if there's another, more subtle issue.

}

return $return;
// @codingStandardsIgnoreEnd
}

Expand Down Expand Up @@ -269,6 +276,7 @@ public function invalidEmailAddresses()
'bob @ domain.com' => ['bob @ domain.com'],
'[email protected]' => ['[email protected]'],
'"bob%[email protected]' => ['"bob%[email protected]'],
'иван@письмо.рф' => ['иван@письмо.рф'],
'multiline' => ['bob

@domain.com'],
Expand Down Expand Up @@ -741,8 +749,8 @@ public function testUseMxCheckBasicValid()
];

if (extension_loaded('intl')) {
$emailAddresses[] = 'иван@письмо.рф';
$emailAddresses[] = 'xn--@-7sbfxdyelgv5j.xn--p1ai';
$emailAddresses[] = 'test@письмо.рф';
$emailAddresses[] = 'test@xn--h1aigbl0e.xn--p1ai';
Copy link
Member

@Maks3w Maks3w Apr 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You say the issue is in the local part, so why change the domain part?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because

var_dump(
    idn_to_ascii('иван@письмо.рф'),
    'test@' . idn_to_ascii('письмо.рф')
);

prints

string '[email protected]' (length=28)

string '[email protected]' (length=27)

}

foreach ($emailAddresses as $input) {
Expand Down