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

Fix HTTP/2 errors with cURL post #589

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

wpdevteam
Copy link
Contributor

Latest cURL update defaults to HTTP/2, but Magento's HTTP CURL Adapter is incompatible. This patch is necessary for newer server environments.

@colinmollenhour
Copy link
Member

Can you describe how it is incompatible? Perhaps it could be made compatible rather than disabling it?

@wpdevteam
Copy link
Contributor Author

For cURL requests, there's no need to use HTTP/2, which would cost a lot more for a single request.
And this function call does not use input param $http_ver, which is a bug.
To make it better, perhaps can base on input param to set proper protocol.

@LeeSaferite
Copy link
Contributor

@wpdevteam How is Magento's HTTP curl adapter incompatible with curl selecting HTTP/2?

@wpdevteam
Copy link
Contributor Author

@LeeSaferite I think the main issue is that input param '$http_ver' in 'public function write' of 'lib/Varien/Http/Adapter/Curl.php' does nothing and 'CURL_HTTP_VERSION_1_1' should be the default header. It has better performance and compatibility if defaulted this way.

Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving since Varien_Http will never support multiplexing or server push which would be the only reasons to support HTTP/2 over HTTP/1.1.

@colinmollenhour colinmollenhour merged commit 7c372a9 into OpenMage:1.9.3.x Jun 4, 2020
colinmollenhour pushed a commit that referenced this pull request Jun 4, 2020
@colinmollenhour
Copy link
Member

Oops, merged into 1.9.3.x.. Just cherry picked into 1.9.4.x.

Thanks for the PR @wpdevteam!

@simbus82
Copy link
Contributor

simbus82 commented Jun 4, 2020

Nice to see that i have solved this same problem in the same manner in a plugin that connect Magento to Joomla :-)
simbus82/MageBridgeCore@f95b3fb

@sreichel sreichel added the Component: lib/Varien Relates to lib/Varien label Jun 4, 2020
@sreichel sreichel added this to the Release 19.4.4 milestone Jun 26, 2020
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 20, 2020
…_VERSION

Fixes HTTP/2 errors with cURL post as it is default now
edannenberg pushed a commit to edannenberg/magento-lts that referenced this pull request Aug 24, 2020
…_VERSION

Fixes HTTP/2 errors with cURL post as it is default now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants