[4.0] AlreadyPunycodeException exception in email test by conseilgouz · Pull Request #36303 · joomla/joomla-cms

conseilgouz · 2021-12-13T16:33:24Z

Don't convert domain to punycode if domain is already a Punycode string

Pull Request for Issue #36301 .

Summary of Changes

Testing Instructions

In Global Configuration, Server tab, in mail section, update "from email" parameter so that domain name contains one or more accented character. Click on "Send TestMail".

Actual result BEFORE applying this Pull Request

Nothing happens : no error, no email sent

Expected result AFTER applying this Pull Request

a message "The email was sent to ..." is displayed and, if domain exists, you should receive an email.

Documentation Changes Required

Don't convert domain to punycode if domain is already a Punycode string fix #36301

infograf768 · 2021-12-14T07:53:19Z

Tested with
infograf@avélaccent.fr

Before patch, we get the error
An error has occurred while fetching the JSON data: HTTP 0 status code.

After patch, the mail is sent.

Still remains a problem:

After patch: I can't save such an email as I get a warning
The email address you entered is invalid. Please enter another email address.
I guess the regex may be wrong

infograf768 · 2021-12-14T08:29:28Z

Maybe

if (preg_match('/[^\x20-\x7e]/', $utfString))

HLeithner · 2021-12-15T12:17:50Z

the regex will always match because it doesn't except . right?

HLeithner · 2021-12-15T12:21:59Z

and it would be easier to test for xn--

conseilgouz · 2021-12-15T16:20:59Z

and it would be easier to test for xn--

I was not sure about uniqueness of the prefix.

Ignore AlreadyPunycodeException

conseilgouz · 2021-12-16T03:41:32Z

Updated code to ignore AlreadyPunycodeException instead of testing domain value.

fontanil · 2021-12-16T14:49:47Z

The PR works fine for me: test email sent and well received, no more Punycode error.
Thanks!

FrankReisenhofer · 2022-05-25T08:54:32Z

@conseilgouz : Any idea when this PR will be merged. So the fix would be available for consumers :-)
Thanks for an update.

conseilgouz · 2022-05-25T10:33:41Z

@conseilgouz : Any idea when this PR will be merged. So the fix would be available for consumers :-) Thanks for an update.

This PR has been marked "closed", and is not in 4.1.

No idea who did that, but, I think that as nobody tested it, it's normal PR life.....

FrankReisenhofer · 2022-05-25T10:52:33Z

@conseilgouz : Thanks for your quick reply.

At the moment I can confirm that the issue is still reproducible in 4.1.4

So how to proceed? What do you suggest?

Above I saw: [1]. So the PR was tested

[1]

fontanil commented on Dec 16, 2021
The PR works fine for me: test email sent and well received, no more Punycode error.
Thanks!

conseilgouz · 2022-05-26T08:37:00Z

@conseilgouz : Thanks for your quick reply.

At the moment I can confirm that the issue is still reproducible in 4.1.4

So how to proceed? What do you suggest?

Above I saw: [1]. So the PR was tested

[1]

fontanil commented on Dec 16, 2021 The PR works fine for me: test email sent and well received, no more Punycode error. Thanks!

A PR has to be "human tested" twice, following the right procedure.
fontanil commented on Dec 16, 2021 just said it was ok, but did not respect the procedure, so no human test has be recorded for this PR => dead PR

To go RTC, it has to be reopened and tested.

FrankReisenhofer · 2022-06-01T07:14:34Z

@conseilgouz : Thanks for your update. Are you going to reopen the PR? If not: Who can do this?

conseilgouz · 2022-06-01T07:57:47Z

I don't have the rights to do it. I just asked it in issue #36301 .
Let's hope this will reopen this PR.

alikon · 2022-06-01T08:56:23Z

pr opened as rquested #36301 (comment)

conseilgouz · 2022-06-01T09:28:29Z

@alikon : what is missing in this PR to go RTC ?

alikon · 2022-06-01T10:28:58Z

2 human succesfull tests

conseilgouz · 2022-06-01T11:28:12Z

@FrankReisenhofer @fontanil : please test this PR and send your result via https://issues.joomla.org/tracker/joomla-cms/36303

conseilgouz · 2022-06-01T11:31:21Z

@alikon : thank you.

Next question : I have "This branch is out-of-date with the base branch" message and no way to fix this. Any idea ?

FrankReisenhofer · 2022-06-01T11:46:13Z

I have tested this item ✅ successfully on 5d16f6e

With the fix the reported issue is no longer occurring. My human manual tests are green

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36303.}

fontanil · 2022-06-02T08:39:51Z

I have tested this item ✅ successfully on 5d16f6e

Tested after manual changes in the file (Patch tester doesn't change the code) : no more error for me.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36303.}

alikon · 2022-06-02T08:49:10Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36303.}

bembelimen · 2022-06-05T19:05:21Z

Thx

Update PunycodeHelper.php

294e1f4

Don't convert domain to punycode if domain is already a Punycode string fix #36301

joomla-cms-bot added the PR-4.0-dev label Dec 13, 2021

conseilgouz changed the title ~~[4.0] Update PunycodeHelper.php~~ [4.0] AlreadyPunycodeException exception in email test Dec 13, 2021

conseilgouz added 3 commits December 13, 2021 17:39

Update PunycodeHelper.php

082af01

Update PunycodeHelper.php

03ad438

Update PunycodeHelper.php

fcd5dfa

alikon mentioned this pull request Dec 13, 2021

[4.0]"This is already a Punycode string" when email domain name contains accented characters #36301

Closed

This comment was marked as abuse.

Sign in to view

conseilgouz added 2 commits December 15, 2021 17:33

Update PunycodeHelper.php

16cde53

Update PunycodeHelper.php

4686049

Ignore AlreadyPunycodeException

conseilgouz added 3 commits December 16, 2021 04:43

Update PunycodeHelper.php

15f194b

Update PunycodeHelper.php

92abf20

Update PunycodeHelper.php

b764286

chmst changed the base branch from 4.0-dev to 4.1-dev January 31, 2022 17:31

Merge branch '4.1-dev' into 4.0-dev

5d16f6e

joomla-cms-bot added the PR-4.1-dev label Jan 31, 2022

chmst removed the PR-4.0-dev label Jan 31, 2022

alikon reopened this Jun 1, 2022

joomla-cms-bot changed the title ~~[4.0] AlreadyPunycodeException exception in email test~~ [4.0] AlreadyPunycodeException exception in email test Jun 2, 2022

Quy added the RTC This Pull Request is Ready To Commit label Jun 2, 2022

bembelimen merged commit 2187874 into joomla:4.1-dev Jun 5, 2022

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 5, 2022

bembelimen added this to the Joomla 4.1.5 milestone Jun 5, 2022

Uh oh!

Conversation

conseilgouz commented Dec 13, 2021

Summary of Changes

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

Uh oh!

infograf768 commented Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

infograf768 commented Dec 14, 2021

Uh oh!

This comment was marked as abuse.

HLeithner commented Dec 15, 2021

Uh oh!

HLeithner commented Dec 15, 2021

Uh oh!

conseilgouz commented Dec 15, 2021

Uh oh!

conseilgouz commented Dec 16, 2021

Uh oh!

fontanil commented Dec 16, 2021

Uh oh!

FrankReisenhofer commented May 25, 2022

Uh oh!

conseilgouz commented May 25, 2022

Uh oh!

FrankReisenhofer commented May 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

conseilgouz commented May 26, 2022

Uh oh!

FrankReisenhofer commented Jun 1, 2022

Uh oh!

conseilgouz commented Jun 1, 2022

Uh oh!

alikon commented Jun 1, 2022

Uh oh!

conseilgouz commented Jun 1, 2022

Uh oh!

alikon commented Jun 1, 2022

Uh oh!

conseilgouz commented Jun 1, 2022

Uh oh!

conseilgouz commented Jun 1, 2022

Uh oh!

FrankReisenhofer commented Jun 1, 2022

Uh oh!

fontanil commented Jun 2, 2022

Uh oh!

alikon commented Jun 2, 2022

Uh oh!

bembelimen commented Jun 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

infograf768 commented Dec 14, 2021 •

edited

Loading

FrankReisenhofer commented May 25, 2022 •

edited

Loading