[Github] getRateLimit throws false negative when rate limiting not applicable

[eddieajau]

The \Joomla\Github\Package\Authorization::getRateLimit method throws a false negative exception if users are on a white list. See:

• http://developer.github.com/v3/rate_limit/
• http://developer.github.com/v3/#rate-limiting

Unfortunately Github is silent on this aspect, possibly because it may only relate to github Enterprise installations.

The response object is below:

object(Joomla\Http\Response)#29 (3) {
  ["code"]=>
  int(404)
  ["headers"]=>
  array(12) {
    ["Server"]=>
    string(10) "GitHub.com"
    ["Date"]=>
    string(29) "Mon, 18 Nov 2013 01:01:10 GMT"
    ["Content-Type"]=>
    string(31) "application/json; charset=utf-8"
    ["Connection"]=>
    string(10) "keep-alive"
    ["Status"]=>
    string(13) "404 Not Found"
    ["X-GitHub-Media-Type"]=>
    string(36) "github.beta; param=html; format=json"
    ["X-Content-Type-Options"]=>
    string(7) "nosniff"
    ["Content-Length"]=>
    string(2) "50"
    ["Access-Control-Allow-Credentials"]=>
    string(4) "true"
    ["Access-Control-Expose-Headers"]=>
    string(112) "ETag, Link, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes"
    ["Access-Control-Allow-Origin"]=>
    string(1) "*"
    ["X-GitHub-Request-Id"]=>
    string(36) "28e422b4-774c-4264-bcaf-81c0a6705fd8"
  }
  ["body"]=>
  string(50) "{"message":"No rate limit for white listed users"}"
}

A possible fix is to ignore the 404 response and treat it as unlimited.

[mbabker]

My suggestion is if the object will always contain the "No rate limit" text in the body, check if the response is a 404 and has that text. Something basically to make sure the 404 is actually because of the white list and not just because something else is messed up.

[eddieajau]

Yeah I thought of that, but it would be brittle because Github might change the text. Thinking about it, the 404 is probably enough (the rate limit was not found) and I think we could rely on them keeping that b/c.