zendframework / zend-http

Http component from Zend Framework
BSD 3-Clause "New" or "Revised" License
134 stars 85 forks source link

ChunkedBody decoding with Curl Adapter #19

Open necromant2005 opened 9 years ago

necromant2005 commented 9 years ago

Zend\Http\Response::decodeChunkedBody fail with exception when curl adapter decode body itself , BUT it still has "Transfer-Encoding: encoded" header

$client = new \Zend\Http\Client();
$client->setOptions(array( 
   'sslverifypeer' => false,
   'adapter'         => 'Zend\Http\Client\Adapter\Curl',
));
$client->setUri('https://www.truesocialmetrics.com/');
$response = $client->send(); // Error parsing body - doesn't seem to be a chunked message

php: 5.5 , 5.6, 7

zerocrates commented 9 years ago

It looks like you're leaving off the $response->getBody() call that would actually cause the exception.

At any rate, in this case, it looks like the problem is that the server you're talking to sends the header in lowercase: transfer-encoding: chunked, while the code in the curl adapter that's supposed to deal with this (line 437) only looks for the uppercase version.

Header names are case-insensitive by definition, and the values for Transfer-Encoding are also case-insensitive, so just adding the i case-insensitivity flag to the regex being used here is probably a reasonable solution.

zerocrates commented 9 years ago

This would be fixed by the open PR #10, and this issue serves as an example of the kind of bug that currently happens and that PR would fix.

Specifically, this looks like a regression introduced by ddf5a83c60120450cc15467a886192de4f2c229f: the comparisons were case-sensitive at that point, and when adding support for varying whitespace, the insensitivity was accidentally not applied to the new regex version of the code.

sgehrig commented 8 years ago

Any idea when this would be fixed?

necromant2005 commented 8 years ago

Nope. I've patched zend to solve this issue.

sgehrig commented 8 years ago

Who is the current maintainer of Zend\Http?

necromant2005 commented 8 years ago

I suppose that's @weierophinney

weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-http; a new issue has been opened at https://github.com/laminas/laminas-http/issues/25.