Better defaults for PHP installations with old ICU lib by alexeyshockov · Pull Request #2454 · guzzle/guzzle

alexeyshockov · 2019-12-12T08:00:25Z

GrahamCampbell · 2019-12-12T09:42:03Z

But what if the function was implemented by a pollyfill?

alexeyshockov · 2019-12-12T11:03:59Z

@GrahamCampbell, don't see any issues with a polyfill. If it implements idn_to_ascii(), then all should be fine. symfony/polyfill implements both the function and INTL_IDNA_VARIANT_UTS46 constant, so idn_conversion option will be true by default.

GrahamCampbell · 2019-12-12T12:49:01Z

Maybe guzzle should always load the polyfill?

alexeyshockov · 2019-12-12T13:04:03Z

I think it will not help here. The original issue with 6.5.0 is about old version of ICU library (intl extension is available, just "partially"). So symfony/polyfill will not work in this case, because it discriminates things by just checking for idn_to_ascii() existence.

Creating a polyfill for exactly this case seems to be very challenging, and IMO Guzzle should not force any polyfills usage to the end developer.

Co-Authored-By: Brandon Kelly <[email protected]>

johncarlson21 · 2019-12-12T18:22:12Z

guys this needs to be merged like now.. after my update, now I have a non working script. I'm going to manually implement this fix.

Due to the INTL/ICU bug, which we have seen on various places, Guzzle, which does not cover our edge cases yet, ran in the same issue as our Core versions earlier in 2019. See guzzle/guzzle#2448 guzzle/guzzle#2454 For the time being, lets mark guzzle as incompatible until Guzzle has solved the issue and released a new version, so we can loosen the conflict constraint. Related: #87953 Resolves: #89904 Releases: master, 9.5, 8.7 Change-Id: If64fb9472d046f020c850cd0551beeaf78796b60 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62606 Tested-by: TYPO3com <[email protected]> Tested-by: Oliver Hader <[email protected]> Reviewed-by: Oliver Hader <[email protected]>

Due to the INTL/ICU bug, which we have seen on various places, Guzzle, which does not cover our edge cases yet, ran in the same issue as our Core versions earlier in 2019. See guzzle/guzzle#2448 guzzle/guzzle#2454 For the time being, lets mark guzzle as incompatible until Guzzle has solved the issue and released a new version, so we can loosen the conflict constraint. Related: #87953 Resolves: #89904 Releases: master, 9.5, 8.7 Change-Id: If64fb9472d046f020c850cd0551beeaf78796b60 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62613 Tested-by: TYPO3com <[email protected]> Tested-by: Oliver Hader <[email protected]> Reviewed-by: Oliver Hader <[email protected]>

Due to the INTL/ICU bug, which we have seen on various places, Guzzle, which does not cover our edge cases yet, ran in the same issue as our Core versions earlier in 2019. See guzzle/guzzle#2448 guzzle/guzzle#2454 For the time being, lets mark guzzle as incompatible until Guzzle has solved the issue and released a new version, so we can loosen the conflict constraint. Related: #87953 Resolves: #89904 Releases: master, 9.5, 8.7 Change-Id: If64fb9472d046f020c850cd0551beeaf78796b60 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62612 Tested-by: TYPO3com <[email protected]> Tested-by: Oliver Hader <[email protected]> Reviewed-by: Oliver Hader <[email protected]>

Due to the INTL/ICU bug, which we have seen on various places, Guzzle, which does not cover our edge cases yet, ran in the same issue as our Core versions earlier in 2019. See guzzle/guzzle#2448 guzzle/guzzle#2454 For the time being, lets mark guzzle as incompatible until Guzzle has solved the issue and released a new version, so we can loosen the conflict constraint. Related: #87953 Resolves: #89904 Releases: master, 9.5, 8.7 Change-Id: If64fb9472d046f020c850cd0551beeaf78796b60 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62613 Tested-by: TYPO3com <[email protected]> Tested-by: Oliver Hader <[email protected]> Reviewed-by: Oliver Hader <[email protected]>

Due to the INTL/ICU bug, which we have seen on various places, Guzzle, which does not cover our edge cases yet, ran in the same issue as our Core versions earlier in 2019. See guzzle/guzzle#2448 guzzle/guzzle#2454 For the time being, lets mark guzzle as incompatible until Guzzle has solved the issue and released a new version, so we can loosen the conflict constraint. Related: #87953 Resolves: #89904 Releases: master, 9.5, 8.7 Change-Id: If64fb9472d046f020c850cd0551beeaf78796b60 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62612 Tested-by: TYPO3com <[email protected]> Tested-by: Oliver Hader <[email protected]> Reviewed-by: Oliver Hader <[email protected]>

Mark-H · 2019-12-13T21:18:04Z

This PR indeed fixes the issue I reported in #2458.

sagikazarmark · 2019-12-13T22:04:52Z

I assume this has to be forward ported to master as well, right?

alexeyshockov · 2019-12-14T06:45:26Z

@sagikazarmark, correct.

In master we already depend on PHP 7.2+, so the check can be simplified to function_exists('idn_to_ascii') && defined('INTL_IDNA_VARIANT_UTS46').

jonnott · 2019-12-14T09:42:52Z

...so when is 6.5.1 shipping? ;)

ohader

Looks good +1

jonnott · 2019-12-16T13:45:56Z

Can 6.5.1 be expedited ... ? Currently anyone running CentOS 6 has broken code if using Guzzle :(

sagikazarmark · 2019-12-18T13:21:51Z

@jonnott you can downgrade to 6.4 for the time being. 6.5.1 will be released today or tomorrow.

sagikazarmark · 2019-12-18T13:23:20Z

@alexeyshockov can you send this (or the appropriate version) patch to master as well?

sagikazarmark

LGTM, thanks!

jonnott · 2019-12-18T17:03:40Z

@jonnott you can downgrade to 6.4 for the time being. 6.5.1 will be released today or tomorrow.

I temporarily added 'idn_conversion'=>false to all my Guzzle\Client instances instead!

This was referenced Dec 12, 2019

Better defaults for PHP installations with old ICU lib #2453

Closed

INTL_IDNA_VARIANT_UTS46 not defined having ICU < 4.6 #2448

Closed

Better defaults for PHP installations with old ICU lib

c63379e

alexeyshockov force-pushed the idn-conversion-fix branch from 82f4b76 to c63379e Compare December 12, 2019 11:05

brandonkelly reviewed Dec 12, 2019

View reviewed changes

Comment thread src/Client.php Outdated

brandonkelly mentioned this pull request Dec 12, 2019

Avoid Guzzle 6.5.0 craftcms/cms#5326

Closed

Faster PHP version comparison

6a8a624

Co-Authored-By: Brandon Kelly <[email protected]>

This was referenced Dec 13, 2019

Use of undefined constant INTL_IDNA_VARIANT_UTS46 - assumed 'INTL_IDNA_VARIANT_UTS46' (this will throw an Error in a future version of PHP) #2457

Closed

Exception: Host must be a string #2458

Closed

sagikazarmark added this to the 6.5.1 milestone Dec 13, 2019

alexeyshockov mentioned this pull request Dec 14, 2019

idn_to_ascii() argument error #2459

Closed

ohader approved these changes Dec 14, 2019

View reviewed changes

sagikazarmark mentioned this pull request Dec 14, 2019

Add IDN support for redirects also #2424

Merged

gmponos approved these changes Dec 16, 2019

View reviewed changes

sagikazarmark approved these changes Dec 18, 2019

View reviewed changes

sagikazarmark merged commit 2eda2b9 into guzzle:6.5 Dec 18, 2019

DaPoHou mentioned this pull request Feb 6, 2020

IDN conversion failed fisharebest/webtrees#3064

Closed

Uh oh!

Conversation

alexeyshockov commented Dec 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GrahamCampbell commented Dec 12, 2019

Uh oh!

alexeyshockov commented Dec 12, 2019

Uh oh!

GrahamCampbell commented Dec 12, 2019

Uh oh!

alexeyshockov commented Dec 12, 2019

Uh oh!

Uh oh!

johncarlson21 commented Dec 12, 2019

Uh oh!

Mark-H commented Dec 13, 2019

Uh oh!

sagikazarmark commented Dec 13, 2019

Uh oh!

alexeyshockov commented Dec 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonnott commented Dec 14, 2019

Uh oh!

ohader left a comment

Choose a reason for hiding this comment

Uh oh!

jonnott commented Dec 16, 2019

Uh oh!

sagikazarmark commented Dec 18, 2019

Uh oh!

sagikazarmark commented Dec 18, 2019

Uh oh!

sagikazarmark left a comment

Choose a reason for hiding this comment

Uh oh!

jonnott commented Dec 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

alexeyshockov commented Dec 12, 2019 •

edited

Loading

alexeyshockov commented Dec 14, 2019 •

edited

Loading