New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checks for INTL_IDNA_VARIANT_UTS46 #1440

Merged
merged 1 commit into from Sep 17, 2018

Conversation

Projects
None yet
2 participants
@kraftbj
Contributor

kraftbj commented Sep 17, 2018

Fixes #1439

Since ICU, which provides the INTL_IDNA_VARIANT_UTS46 may not be kept up to date the same as PHP, a separate check here will prevent the undeclared constant warning.

The downside is idn_to_utf8 function defaults to using INTL_IDNA_VARIANT_2003 which would then throw a deprecated warning in PHP 7.2+. If PHP 7.4, it is planned that the default will be INTL_IDNA_VARIANT_UTS46 which would likely return the original issue.

@westonruter What would you prefer? Checking for UTS46 and taking the deprecated in PHP 7.2 for sites running ancient ICU or check for the UTS46 variant along with the idn_to_utf8 exists check? Or a third option?

@kraftbj

This comment has been minimized.

Contributor

kraftbj commented Sep 17, 2018

After a bit more research and touching base with sysadmin, the setup of the testing box is a default CentOS6/cPanel-powered box with PHP updated. So, I think we'll need to account for both lacking INTL_IDNA_VARIANT_UTS46 and a deprecated INTL_IDNA_VARIANT_2003.

@westonruter westonruter added this to the v1.0 milestone Sep 17, 2018

@westonruter

This comment has been minimized.

Member

westonruter commented Sep 17, 2018

The downside is idn_to_utf8 function defaults to using INTL_IDNA_VARIANT_2003 which would then throw a deprecated warning in PHP 7.2+. If PHP 7.4, it is planned that the default will be INTL_IDNA_VARIANT_UTS46 which would likely return the original issue.

So if ICU is at an old version as before PHP 5.4 but the current version of PHP is 7.2+?

What would you prefer? Checking for UTS46 and taking the deprecated in PHP 7.2 for sites running ancient ICU or check for the UTS46 variant along with the idn_to_utf8 exists check? Or a third option?

@kraftbj Humm. I don't really know. I'll go with whatever you think is best.

@kraftbj

This comment has been minimized.

Contributor

kraftbj commented Sep 17, 2018

So if ICU is at an old version as before PHP 5.4 but the current version of PHP is 7.2+?

Yup, ICU is independent of PHP, apparently. 5.4+ integrated the "wrapper", but isn't an indicator of what version of ICU is available to a given system. So yes, PHP 7.2+ with older ICU <4.6, we're stuck between a rock and a hard place.

For now, I think what I have is decent. I would expect people with 7.2+ with outdated ICU would be fewer than people on 5.4+ with outdated ICU. Long-term, I need to chew on it more.

@westonruter westonruter merged commit a4d41ca into develop Sep 17, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@westonruter westonruter deleted the fix/idna-1 branch Sep 17, 2018

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