Headers/body are not getting separated properly

[milo]

Headers/body are not getting separated properly

[milo]

Hi @mike503 . Thank you for the bug report. Unfortunately I cannot reproduce it by:

use Milo\Github;

$api = new Github\Api(new Github\Http\CurlClient);
$response = $api->get('/repos/milo/github-api/commits/master');
var_dump($response->getContent());

the result is:

string(6797) "{"sha":"4e61e272ac71cf0420cd17de5b43cf2c04819be2","commit"............"

which is plain response body, the valid JSON.

May I ask you to send me a dump of buggy content? From begin and whole headers. Dumped in CurlClient class, before line:

var_dump($result);
list($headersStr, $content) = explode("\r\n\r\n", $result, 2) + ['', ''];

After that, could you try to replace code in Http\CurlClient.php?:

# remove
list($headersStr, $content) = explode("\r\n\r\n", $result, 2) + ['', ''];

# replace by
$size = curl_getinfo($this->curl, CURLINFO_HEADER_SIZE);
$headersStr = rtrim(substr($result, 0, $size));
$content = substr($result, $size);

[mike503]

I think there's a typo now... :)

I think this would fix?

- list($name, $value) = explode(': ', $header);
+ list($name, $value) = explode(': ', $headersStr);

[mike503]

Also was just told about this, there is a CURLOPT_HEADERFUNCTION (not sure in which version it is introduced) that allows for you to receive the headers and process them in a callback, WITHOUT having to use CURLOPT_HEADER that would spit it out in the return.

http://ontodevelopment.blogspot.com/2011/04/curloptheaderfunction-tutorial-with.html

[milo]

@mike503 About the typo... could you send link to the source?

[milo]

And about CURLOPT_HEADERFUNCTION... Current solution works fine. I have no reason to complicate it for now.

[mike503]

Whoops. Ignore that last comment. I missed a line when I was reporting that.

Yes, changing to that header callback would complicate a little bit, I suppose. But you would never have to worry about wonky header stuff messing things up again. :)

I am grabbing your project via Composer, which is grabbing it (I believe) via packagist. Do you know how long it takes for your git commits to get out there? I ran composer update and it said there was nothing new.

[milo]

@mike503 I didn't tagged version, so you must use @dev. Packagist already has current master. But I don't know, sometimes it takes 10 or 15 minutes.

[milo]

But you would never have to worry about wonky header stuff messing things up again

I'll think about that. Must check the compatibility and so on... Thank's for the tip anyway!

[milo]

@mike503 Implemented 2de95e7

[mike503]

yeah I noticed. that's awesome! I will need to get composer to grab the dev from you properly tomorrow. :)

[mike503]

I think I see part of the problem :)

I am behind a corporate proxy (basic Squid-style) and I get multiple lines of HTTP status code output.

I am using the latest including your ec5a8ae with trim() and still get:

Notice: Undefined offset: 1 in /home/mike503/web/site/vendor/milo/github-api/src/Github/Http/CurlClient.php on line 83

This is actually the raw stuff I get back (and this is probably why the original way you were parsing it was breaking - because it issues TWO different "\r\n\r\n" style delimiters)

This is the HTTP response I get for a simple $response = $api->get("/repos/my-repo/repo/commits?since=timestamp");

HTTP/1.1 200 Connection established

HTTP/1.1 200 OK
Server: GitHub.com
Date: Wed, 01 Oct 2014 08:54:07 GMT
Content-Type: application/json; charset=utf-8
Status: 200 OK
X-RateLimit-Limit: 5000
X-RateLimit-Remaining: 4996
X-RateLimit-Reset: 1412157027
Cache-Control: private, max-age=60, s-maxage=60
Last-Modified: Wed, 01 Oct 2014 08:44:02 GMT
ETag: "878f29d52dddd01ccf16658b376e64f1"
X-OAuth-Scopes: gist, notifications, read:org, read:public_key, read:repo_hook, repo, user
X-Accepted-OAuth-Scopes: 
Vary: Accept, Authorization, Cookie, X-GitHub-OTP
X-GitHub-Media-Type: github.v3; format=json
X-XSS-Protection: 1; mode=block
X-Frame-Options: deny
Content-Security-Policy: default-src 'none'
Content-Length: 31331
Access-Control-Allow-Credentials: true
Access-Control-Expose-Headers: ETag, Link, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval
Access-Control-Allow-Origin: *
X-GitHub-Request-Id: 86868147:13AF:246E8960:542BC22F
Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
X-Content-Type-Options: nosniff
Vary: Accept-Encoding
X-Served-By: 6d7de9f6458146ac34ea2a8d72ba3141

[mike503]

Before you explode(':') you should probably check if the line has a colon?

if (strstr($line, ':')) {
  list($name, $value) = explode(':', $line, 2);
  $responseHeaders[trim($name)] = trim($value);
}

no desireable HTTP header should leave that out anyway.

someday I will get my git-fu up and send a PR, but that's such a minor change... I just tried it and viola, all of a sudden I get expected results. :)

[milo]

@mike503 I see! Thank you for debugging.

[milo]

@mike503 Fixed by 89dd5ff

[mike503]

I believe the first part is the connection from client -> proxy server using SSL, and curl reporting the connection was successful there. Then the next hop is the actual request. Kinda funky and not many people seem to parse for that in all the curl header splitting I've seen. I'm actually going to adopt this myself for my PHP areas that need headers in a more sane, structured way. :)

[milo]

As I read docs, client to proxy is HTTP. Than client calls CONNECT and proxy returns HTTP/1.1 Connection Established (+ my proxy returns Proxy-Agent: Apache 2.2 header). Then SSL connection is tunneled untouched through proxy. And cURL returns response headers and body.

[mike503]

Yeah, now that I think about it, it does seem to be HTTP -> PROXY -> HTTPS -> GITHUB.

Either way, curl handles it well, we just needed to process the headers better. I'm surprised I am not bitten by this more, I must not use many things which care about headers. Thanks for the quick turnaround though, I can now use this for my project. :)