Fixed curl adapter to properly set http version based on $http_ver argument

[davidalger]

Description (*)

The curl HTTP adapter currently does not respect the value of the $http_ver argument to the write method causing \Magento\Framework\HTTP\ZendClient to throw an exception causing a fatal error when making a request to an HTTP/2 capable server from a client server that has HTTP/2 curl libraries loaded.

Fatal error: Uncaught Zend_Http_Exception: Invalid header line detected in /Volumes/Server/sites/vaporbeast.vbst/vendor/magento/zendframework1/library/Zend/Http/Response.php on line 572

Fixed Issues (if relevant)

1. Curl - allow setting HTTP version #19845 (this was attempting to fix same thing, but was reverted in commit 55b8bf8 because it was done incorrectly)
2. [Backport] Curl - allow setting HTTP version #21251 (backport of Curl - allow setting HTTP version #19845; should not be merged due to issues with original PR)

Manual testing scenarios (*)

Use \Magento\Framework\HTTP\ZendClient to make a request to an HTTP/2 capable endpoint from a system with HTTP/2 enabled curl libraries. Without this change in place, the client will fail to parse the response headers because the regex found in \Zend_Http_Response::extractHeaders is specifically expecting HTTP 1.1 or HTTP 1.0 in the response.

To reproduce the issue, the following code snippet may be used:

$client = new \Magento\Framework\HTTP\ZendClient();
$client->setUri('https://davidalger.com/robots.txt');
$result = $client->request();
var_dump($result->getHeaders());

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[magento-engcom-team]

Hi @davidalger. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[rogyar]

Hi @davidalger. Have just checked a few HTTP/2 implementations using write method of the curl adapter. I found no problem with headers parsing. Got a correct set of HTTP/2 headers and a proper body. Maybe the issue occurs in some special circumstances, not sure.
However, if you send a request using 1.0 or 1.1 protocol for HTTP/2 based resource, the system does not respect the protocol version passed to the write method. Your change perfectly fixes the issue.
Good job.

[magento-engcom-team]

Hi @rogyar, thank you for the review.
ENGCOM-4424 has been created to process this Pull Request

[davidalger]

@rogyar Thanks for looking at this. My bad on the testing scenario…the exception only occurs when this adapter is used by the zend client, so to reproduce the issue described, run the following code from a system with HTTP/2 curl libraries present.

$client = new \Magento\Framework\HTTP\ZendClient();
$client->setUri('https://davidalger.com/robots.txt');
$result = $client->request();
var_dump($result->getHeaders());

Before the fix on this PR, the above will result in the following error:

Fatal error: Uncaught Zend_Http_Exception: Invalid header line detected in /Volumes/Server/sites/vaporbeast.vbst/vendor/magento/zendframework1/library/Zend/Http/Response.php on line 572

As long as curl is loaded, \Magento\Framework\HTTP\ZendClient::_trySetCurlAdapter will create a new instance of \Magento\Framework\HTTP\Adapter\Curl and set it as the adapter on the ZendClient before making the request. Ultimately after the request is made, \Zend_Http_Response::extractHeaders is called, and it iterates over lines in the response header, skipping lines that match the expression '#^HTTP/\d+(?:\.\d+) [1-5]\d+#' (this does not match the HTTP/2 200 line that is in a response from an HTTP/2 capable server) causing an error when it asserts that line is a valid header.

I'll update my steps to test the issue to reflect the above code snippet and more clearly state that reproducing the issue requires using the adapter from within the zend client.

[p-bystritsky]

QA passed.

[m2-assistant]

Hi @davidalger, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[djamps]

Can we merge this into 2.2.x? Same issue there and the patch works for me on 2.2.9.

[davidalger]

@djamps This was back ported and accepted already in #21550. Based on the milestone/tags applied, it appears 2.2.10 is when it will be published on the 2.2.x line.

[djamps]

Thank you!