fixed removing handled header parts from response header #6691
Conversation
|
Perhaps you can create testcases for the issues you describe to prevent this bug from returning due to future changes? |
|
I tried to do it the way you did your tests, however I don't think it is a good way to do actually requests to a webserver. In my case I tried it by creating a php response with header("Transfer-Encoding", " chunked"), but php trims the whitespace before chunked and removes the "test problem". I also saw that there were other "dynamic-tests" (that's how you call remote tests, I guess) failed after setting things up. An idea would be to wrap curl in a class to mock the reponses. Then one could create tests that work even offline. But that would be a bigger change in code, so I would like to talk to someone before. I tried to contact some of you in your channel on irc, but nobody answers and I'm a bit busy. |
|
We may startup a small web server in travis in order to let all the http tests run also there... |
|
@kasitmp As @Ocramius noted, we can modify our Travis configuration to startup a web server using the built-in PHP webserver with versions 5.4+, and then test against that (we actually have tests already that only test against a web server that runs against a document root in the test hierarchy, and we use these for integration spot-checking). Alternately, you can create mock responses to test against (we also do this in a number of places). Considering this is cURL specific, you likely need to go with the first option. We do need a test, however, to validate that this works. If you think you can work with us to make that happen in the next two weeks, I'll mark this for 2.4. |
|
I'm really busy at the moment, but I will try. This is a while back so I will need some time to get back into my code, I guess :) |
| // separating header from body because it is dangerous to accidentially replace strings in the body | ||
| $responseHeaderSize = curl_getinfo($this->curl, CURLINFO_HEADER_SIZE); | ||
| $responseHeaders = substr($this->response, 0, $responseHeaderSize); | ||
|
|
||
| // cURL automatically decodes chunked-messages, this means we have to disallow the Zend\Http\Response to do it again |
There was a problem hiding this comment.
I separated header from body, to be sure that i don't manipulate the body. Possible test:
Textfile with Transfer-Encoding: chunked\r\n inside AND the webserver must not make use of chunking.
In the old version the assert that the file is the same would probably fail.
|
I'm sorry, but I can't find time to do the tests at the moment. But I describe what I would test, if I would know how to do it with your infrastructure. Just look at the annotations https://github.com/zendframework/zf2/pull/6691/files |
…pter fixed removing handled header parts from response header
|
Merged to develop for release with 2.4. |
…header-handling-in-curl-adapter fixed removing handled header parts from response header
This fix should solve two potential issues:
$this->response = str_ireplace("Transfer-Encoding: chunked\r\n", '', $this->response);is able to remove content, also from a response body. To solve this I separate the header from the body first before manipulating the it.I coded this to fix my issue #6689