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

idn_to_ascii() argument error #2459

Closed
jonnott opened this issue Dec 13, 2019 · 11 comments
Closed

idn_to_ascii() argument error #2459

jonnott opened this issue Dec 13, 2019 · 11 comments

Comments

@jonnott
Copy link
Contributor

jonnott commented Dec 13, 2019

Guzzle version(s) affected: 6.5.0

Description
Since composer update to 6.5.0 (from 6.4.x) now getting this error:

Warning: idn_to_ascii() expects parameter 3 to be long, string given in /home/*****/public_html/vendor/guzzlehttp/guzzle/src/Client.php on line 221

3rd argument is INTL_IDNA_VARIANT_UTS46 constant, it would seem.

How to reproduce
Very simple requests .. GET/POST/PUT passing JSON to Salesforce REST API endpoints.

Additional context
PHP 5.6 in production

@jonnott
Copy link
Contributor Author

jonnott commented Dec 13, 2019

Could it be related to this ? ..

https://forge.typo3.org/issues/87953

"Our Server run with CPanel on CentOS 6, and only an old version of Intl is installed, and the constant INTL_IDNA_VARIANT_UTS46 not exist."

The production server I'm on is running CPanel on CentOS 6 :(

Any work-around?

@jonnott
Copy link
Contributor Author

jonnott commented Dec 13, 2019

I've looked at phpinfo() on the server under 'intl' (internationalisation support) and ICU version is 4.2.1

PHP docs for idn_to_ascii() for argument 3 are:

variant
Either INTL_IDNA_VARIANT_2003 (deprecated as of PHP 7.2.0) for IDNA 2003 or INTL_IDNA_VARIANT_UTS46 (only available as of ICU 4.6) for UTS #46.

What do I do?

@jonnott
Copy link
Contributor Author

jonnott commented Dec 13, 2019

Guess I need to update the ICU RPM on my server.

In the meantime, how do I use the new idn_conversion (false) Request Option to disable this for my requests?

@jonnott
Copy link
Contributor Author

jonnott commented Dec 14, 2019

I tried replacing

			$client = new GuzzleHttp\Client();

with

			$client = new GuzzleHttp\Client([
				'defaults' => [
					'idn_conversion' => false
				]
			]);

..but that doesn't seem to have worked :(

@jonnott
Copy link
Contributor Author

jonnott commented Dec 14, 2019

Oops, realised with the above I was using the Guzzle 5.x method for setting defaults, so I fixed by just using 'idn_conversion' => false not inside a 'defaults' array.

Anyway, the bigger problem here seems to be that when the server's ICU version is < 4.6, Guzzle must detect this and not assume INTL_IDNA_VARIANT_UTS46 constant is defined (which causes the original error above), and fall back to INTL_IDNA_VARIANT_2003 instead...

There is an undocumented PHP constant INTL_ICU_VERSION for this:
https://stackoverflow.com/questions/53996249/php-get-icu-version
http://svn.php.net/viewvc/?view=revision&revision=311714

@jonnott
Copy link
Contributor Author

jonnott commented Dec 14, 2019

Proof the constant works!

<? exit(INTL_ICU_VERSION);

@jonnott
Copy link
Contributor Author

jonnott commented Dec 14, 2019

I guess it's all irrelevant, and Guzzle requires PHP ^7.2.5, but INTL_IDNA_VARIANT_2003 was deprecated in PHP 7.2 :(

@alexeyshockov
Copy link
Collaborator

Please also take a look at #2454

@jonnott
Copy link
Contributor Author

jonnott commented Dec 14, 2019

Please also take a look at #2454

Great! Also, do you think the INTL_ICU_VERSION constant would be of any use to simplify the patch that's already been done? Would you perhaps not need the PHP 7.2 version comparison? I guess you might as the 2003 variant constant is deprecated from PHP >7.2 ..?

@jonnott
Copy link
Contributor Author

jonnott commented Dec 14, 2019

Feel free to close this issue - it's a duplicate of #2448 and other .. I was just externally processing as I was working out what was going wrong ;)

@jonnott jonnott closed this as completed Dec 14, 2019
@alexeyshockov
Copy link
Collaborator

Also, do you think the INTL_ICU_VERSION constant would be of any use to simplify the patch that's already been done?

Don't think so... We need PHP version check anyway because of the deprecation, yes.

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

No branches or pull requests

3 participants