Skip to content

Fix idn emails not working in shares - #30600

Merged
CarlSchwan merged 3 commits into
masterfrom
fix/30595/idn-email-share
Jan 13, 2022
Merged

Fix idn emails not working in shares#30600
CarlSchwan merged 3 commits into
masterfrom
fix/30595/idn-email-share

Conversation

@CarlSchwan

Copy link
Copy Markdown
Member

Fix#30595

Signed-off-by: Carl Schwan carl@carlschwan.eu

Fix#30595
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@CarlSchwanCarlSchwan added the 3. to review Waiting for reviews label Jan 11, 2022
@CarlSchwanCarlSchwan added this to the Nextcloud 24 milestone Jan 11, 2022
@CarlSchwan
CarlSchwan requested review from a team and PVince81January 11, 2022 14:45
@CarlSchwanCarlSchwan self-assigned this Jan 11, 2022
@CarlSchwan
CarlSchwan requested review from ArtificialOwl, come-nc and skjnldsv and removed request for a teamJanuary 11, 2022 14:45

@tcitworldtcitworld left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can use OCP\Mail\Mailer::validateMailAddress that already has the IDN check.

/**
* Checks if an e-mail address is valid
*
* @param string $email Email address to be validated
* @return bool True if the mail address is valid, false otherwise
*/
publicfunctionvalidateMailAddress(string$email): bool {
if ($email === '') {
// Shortcut: empty addresses are never valid
returnfalse;
}
$validator = newEmailValidator();
$validation = newRFCValidation();
return$validator->isValid($this->convertEmail($email), $validation);
}
/**
* SwiftMailer does currently not work with IDN domains, this function therefore converts the domains
*
* FIXME: Remove this once SwiftMailer supports IDN
*
* @param string $email
* @return string Converted mail address if `idn_to_ascii` exists
*/
protectedfunctionconvertEmail(string$email): string {
if (!function_exists('idn_to_ascii') || !defined('INTL_IDNA_VARIANT_UTS46') || strpos($email, '@') === false) {
return$email;
}
[$name, $domain] = explode('@', $email, 2);
$domain = idn_to_ascii($domain, 0,INTL_IDNA_VARIANT_UTS46);
return$name.'@'.$domain;
}

@PVince81PVince81 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@PVince81

Copy link
Copy Markdown
Member

if possible for this code path would be good to add unit tests with an IDN address

@CarlSchwan
CarlSchwanforce-pushed the fix/30595/idn-email-share branch from a666594 to 50da844CompareJanuary 11, 2022 19:56
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@CarlSchwan
CarlSchwanforce-pushed the fix/30595/idn-email-share branch from 50da844 to 7ecb65fCompareJanuary 11, 2022 19:59
Comment threadapps/sharebymail/lib/ShareByMailProvider.php Outdated
@CarlSchwan
CarlSchwanforce-pushed the fix/30595/idn-email-share branch from 7711339 to acf3becCompareJanuary 12, 2022 14:17
Comment threadapps/sharebymail/lib/ShareByMailProvider.php Outdated
@CarlSchwan
CarlSchwanforce-pushed the fix/30595/idn-email-share branch from acf3bec to ed80527CompareJanuary 12, 2022 15:36
@CarlSchwan

Copy link
Copy Markdown
MemberAuthor

I'm looking into the php unit failures, strange that is works locally for me :/

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@CarlSchwan
CarlSchwanforce-pushed the fix/30595/idn-email-share branch from ed80527 to fffc19fCompareJanuary 12, 2022 19:33
@CarlSchwan

Copy link
Copy Markdown
MemberAuthor

I found why the tests seemed to work locally and fixed them :)

@PVince81PVince81 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@CarlSchwan
CarlSchwan merged commit b104886 into masterJan 13, 2022
@CarlSchwan
CarlSchwan deleted the fix/30595/idn-email-share branch January 13, 2022 08:24
@CarlSchwan

Copy link
Copy Markdown
MemberAuthor

/backport to stable23

@CarlSchwan

Copy link
Copy Markdown
MemberAuthor

/backport to stable22

@CarlSchwan

Copy link
Copy Markdown
MemberAuthor

/backport to stable21

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No email suggestion when sharing with IDN email address

3 participants

@CarlSchwan@PVince81@tcitworld