Skip to content
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

Warning: Use of undefined constant INTL_IDNA_VARIANT_UTS46 #1439

Closed
kraftbj opened this issue Sep 17, 2018 · 2 comments
Closed

Warning: Use of undefined constant INTL_IDNA_VARIANT_UTS46 #1439

kraftbj opened this issue Sep 17, 2018 · 2 comments
Labels
Milestone

Comments

@kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Sep 17, 2018

( ! ) Warning: Use of undefined constant INTL_IDNA_VARIANT_UTS46 - assumed
'INTL_IDNA_VARIANT_UTS46' (this will throw an Error in a future version of PHP) in
/home/public_html/wp-content/plugins/amp/includes/class-amp-http.php on line 205

From looking online, this may be due to a really old version of ICU running on the machine, but something that we may want to account for?

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

@westonruter westonruter commented Sep 17, 2018

Thank you. The relevant code is:

https://github.com/Automattic/amp-wp/blob/64cad605ee112175f2144df2518ca5a4f11cdfbb/includes/class-amp-http.php#L203-L209

So it seems what we should do is:

- if ( version_compare( PHP_VERSION, '5.4', '>=' ) ) { 
+ if ( version_compare( PHP_VERSION, '5.4', '>=' ) && defined( 'INTL_IDNA_VARIANT_UTS46' ) ) { 

@kraftbj
Copy link
Contributor Author

@kraftbj kraftbj commented Sep 17, 2018

I'll PR that after testing.

In the meantime, I confirmed that INTL_IDNA_VARIANT_UTS46 was shipped in http://site.icu-project.org/download/46 ICU 4.6 back in ~2010/2011, but while this particular machine has PHP 7.2.9 installed, ICU is 4.2.1.

@swissspidy swissspidy added the Bug label Jun 6, 2019
kienstra added a commit that referenced this issue Dec 18, 2019
Thanks to Brandon Kraft's comment,
it looks like this version has teh
INTL_IDNA_VARIANTE_UTS46 constant:
#1439 (comment)
And as long as that constant is defined, it will be passed to idn_to_utf8():
https://github.com/ampproject/amp-wp/blob/39336b78be7a4ba8b3a09a6f43d9f66e3c201e05/includes/class-amp-http.php#L233
...and that won't trigger this deprecation warning:
https://github.com/php/php-src/pull/2355/files#diff-24931a05f9e656989a55c44cf8a72f9eR294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants