Replace Punycode by Symfony polyfill #358
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
Please cherry-pick 06394db, it does more validation. |
yguedidi
force-pushed
the
replace-punycode
branch
from
4b7bd5a to
3f47d22
Compare
Author
|
@jtojnar done |
jtojnar
force-pushed
the
replace-punycode
branch
from
3f47d22 to
ec6de9b
Compare
jtojnar
force-pushed
the
replace-punycode
branch
from
ec6de9b to
4405826
Compare
Collaborator
Because it's no more maintained. (cherry picked from commit 06394db)
As per <https://www.php.net/manual/en/function.idn-to-ascii.php>, PHP 7.2 deprecated the `INTL_IDNA_VARIANT_2003` constant. But until PHP 7.4, it was still used as the default value of the `$variant` argument and will trigger a deprecation warning (https://github.com/php/php-src/blob/php-7.3.0/ext/intl/idn/idn.c#L318-L320). This in turn broke tests on PHP 7.2 and 7.3: 1x: idn_to_ascii(): INTL_IDNA_VARIANT_2003 is deprecated 1x in GrabyTest::testUrlWithAccent from Tests\Graby This is not an issue on `master` since we require PHP ≥ 7.4 there. One option would be passing the `INTL_IDNA_VARIANT_UTS46` explicitly to the `$variant` argument so that it is consistent across all supported PHP versions. Unfortunately, this would require raising the requirements to ICU ≥ 4.6, which would break systems that have `intl` extension linked against ancient ICU (allowed on PHP before 7.4: https://github.com/php/php-src/blob/php-7.2.0/ext/intl/idn/idn.c#L85-L87). Polyfill does support emulating `INTL_IDNA_VARIANT_UTS46` but it will not do anything if the presence of any `intl` extension is detected (https://github.com/symfony/polyfill-intl-idn/blob/v1.26.0/bootstrap.php#L14-L16). So it will not help us when the constant we want does not actually exist. We could silence the warning with `@` but that is always iffy. For now, let’s use the polyfill explicitly on PHP 7.2 or 7.3 built with `intl` extension linked against ICU < 4.6. The PHP implementation will be a bit slower but the situation is pretty unlikely in my opinion.
jtojnar
force-pushed
the
replace-punycode
branch
from
daabde2 to
da1941b
Compare
j0k3r
approved these changes
Feb 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Targeting 2.x as this is the version used by wallabag for now, and I want to remove an abandoned package.