Skip to content

Comments

[5.3] Major update idna converter#45140

Merged
rdeutz merged 4 commits intojoomla:5.3-devfrom
Digital-Peak:idna
Mar 19, 2025
Merged

[5.3] Major update idna converter#45140
rdeutz merged 4 commits intojoomla:5.3-devfrom
Digital-Peak:idna

Conversation

@laoneo
Copy link
Member

@laoneo laoneo commented Mar 15, 2025

Summary of Changes

When running the API tests, sometimes a implicit warning is displayed because of the outdated idna version.

Testing Instructions

  • Save an article with an url, a correct url and an invalid url like http:/google.c
  • Save a contact with a valid and invalid email

Actual result BEFORE applying this Pull Request

  • Article can be saved or an error is shown when the url is invalid.
  • Contact can be saved or an error is shown when the email is invalid.

Expected result AFTER applying this Pull Request

  • Article can be saved or an error is shown when the url is invalid.
  • Contact can be saved or an error is shown when the email is invalid.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@brianteeman
Copy link
Contributor

@richard67
Copy link
Member

richard67 commented Mar 15, 2025

Unit tests are failing with error "Class "IntlChar" not found" here: https://github.com/joomla/joomla-cms/blob/5.3-dev/libraries/src/String/PunycodeHelper.php#L44

E.g.:

1) Joomla\Tests\Unit\Libraries\Cms\Mail\MailHelperTest::testIsEmailAddress with data set #1 ('joe@home', true)
Error: Class "IntlChar" not found
C:\projects\joomla-cms\libraries\vendor\algo26-matthias\idna-convert\src\NamePrep\NamePrep.php:69
C:\projects\joomla-cms\libraries\vendor\algo26-matthias\idna-convert\src\NamePrep\NamePrep.php:58
C:\projects\joomla-cms\libraries\vendor\algo26-matthias\idna-convert\src\Punycode\ToPunycode.php:33
C:\projects\joomla-cms\libraries\vendor\algo26-matthias\idna-convert\src\ToIdn.php:55
C:\projects\joomla-cms\libraries\src\String\PunycodeHelper.php:44
C:\projects\joomla-cms\libraries\src\Mail\MailHelper.php:163
C:\projects\joomla-cms\tests\Unit\Libraries\Cms\Mail\MailHelperTest.php:287

@laoneo
Copy link
Member Author

laoneo commented Mar 15, 2025

This update would require the intl extension of PHP, which probably is installed on a default web server. But it is a requirement change which can be counted as bc break.

@laoneo
Copy link
Member Author

laoneo commented Mar 15, 2025

I'v opened an issue on upstream algo26-matthias/idna-convert#47.

@laoneo laoneo added the b/c break This item changes the behavior in an incompatible why. HEADS UP label Mar 15, 2025
@laoneo
Copy link
Member Author

laoneo commented Mar 16, 2025

Once algo26-matthias/idna-convert#49 will be merged and a new release shipped, then this is upgrade safe.

@laoneo laoneo removed the b/c break This item changes the behavior in an incompatible why. HEADS UP label Mar 16, 2025
@brianteeman
Copy link
Contributor

tested that the deprecation notice is resolved by this update.

Unable to follow your test instructions as I dont understand what you mean by " and an invalid one like http:/google.c"

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on f1ff2fd


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45140.

@laoneo
Copy link
Member Author

laoneo commented Mar 18, 2025

Updated the instructions, it should be tested with a valid and invalid url.

@brianteeman
Copy link
Contributor

i didnt spot the single slash in the url

@brianteeman
Copy link
Contributor

this is needed for php 8.4.5 or Joomla can not be installed if your admin email address is in the form abc@example

@muhme
Copy link
Contributor

muhme commented Mar 19, 2025

I have tested this item ✅ successfully on f1ff2fd

* Tested with current 5.3-dev and PHP 8.4.4 on macOS

  • Before applying the PR, after fresh apache restart once the deprecated is shown in Joomla administrator backend
Deprecated: Algo26\IdnaConvert\Punycode\PunycodeInterface::__construct(): Implicitly marking parameter $idnVersion as nullable is deprecated, the explicit nullable type must be used instead in
/Users/hlu/Desktop/no_backup/joomla-cms/53/libraries/vendor/algo26-matthias/idna-convert/src/Punycode/PunycodeInterface.php 
on line 6
  • Patch applied with gh pr checkout 45140 and checked in composer* that idna-converser version is updated from ^3.1.1 to ^4.0.4, did composer i and restarted apache, no deprecated error message anymore
  • Tested article with valid, invalid and Umlaut-URL; invalid URL is not passing, it is getting Invalid field until it is fixed
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45140.
  • Tested contact email-adress with valid address and Umlaut-domain
  • ⚠️ Invalid email adresses like heiko@bla are passing without error, same behavier as before the patch

@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45140.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 19, 2025
@rdeutz rdeutz merged commit 18be481 into joomla:5.3-dev Mar 19, 2025
4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 19, 2025
@laoneo laoneo deleted the idna branch March 19, 2025 15:43
@brianteeman
Copy link
Contributor

thank you

@bembelimen bembelimen removed this from the Joomla! 5.3.4 milestone Sep 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants