Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle case insensitive headers when parsing for API rate limiting #2175

Merged
merged 7 commits into from
Apr 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions Octokit.Tests/Clients/Enterprise/EnterpriseProbeTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using System.Reactive.Linq;
Expand All @@ -19,10 +20,15 @@ public class TheProbeMethod
[InlineData(HttpStatusCode.NotFound)]
public async Task ReturnsExistsForResponseWithCorrectHeadersRegardlessOfResponse(HttpStatusCode httpStatusCode)
{
var headers = new Dictionary<string, string>()
{
{ "Server", "REVERSE-PROXY" },
{ "X-GitHub-Request-Id", Guid.NewGuid().ToString() }
};
var response = Substitute.For<IResponse>();
response.StatusCode.Returns(httpStatusCode);
response.Headers["Server"].Returns("REVERSE-PROXY");
response.Headers.ContainsKey("X-GitHub-Request-Id").Returns(true);
response.Headers.Returns(headers);

var productHeader = new ProductHeaderValue("GHfW", "99");
var httpClient = Substitute.For<IHttpClient>();
httpClient.Send(Args.Request, CancellationToken.None).Returns(Task.FromResult(response));
Expand All @@ -43,10 +49,16 @@ public async Task ReturnsExistsForResponseWithCorrectHeadersRegardlessOfResponse
[Fact]
public async Task ReturnsExistsForApiExceptionWithCorrectHeaders()
{

var headers = new Dictionary<string, string>()
{
{ "Server", "GitHub.com" },
{ "X-GitHub-Request-Id", Guid.NewGuid().ToString() }
};
var httpClient = Substitute.For<IHttpClient>();
var response = Substitute.For<IResponse>();
response.Headers["Server"].Returns("GitHub.com");
response.Headers.ContainsKey("X-GitHub-Request-Id").Returns(true);
response.Headers.Returns(headers);

var apiException = new ApiException(response);
httpClient.Send(Args.Request, CancellationToken.None).ThrowsAsync(apiException);
var productHeader = new ProductHeaderValue("GHfW", "99");
Expand Down
11 changes: 11 additions & 0 deletions Octokit.Tests/Exceptions/AbuseExceptionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,17 @@ public void WithRetryAfterHeader_PopulatesRetryAfterSeconds()
Assert.Equal(30, abuseException.RetryAfterSeconds);
}

[Fact]
public void WithRetryAfterCaseInsensitiveHeader_PopulatesRetryAfterSeconds()
{
var headerDictionary = new Dictionary<string, string> { { "retry-after", "20" } };

var response = new Response(HttpStatusCode.Forbidden, null, headerDictionary, "application/json");
var abuseException = new AbuseException(response);

Assert.Equal(20, abuseException.RetryAfterSeconds);
}

[Fact]
public void WithZeroHeaderValue_RetryAfterSecondsIsZero()
{
Expand Down
22 changes: 22 additions & 0 deletions Octokit.Tests/Http/ApiInfoParserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,28 @@ public void ParsesApiInfoFromHeaders()
Assert.Equal("5634b0b187fd2e91e3126a75006cc4fa", apiInfo.Etag);
}

[Fact]
public void ParsesApiInfoFromCaseInsensitiveHeaders()
{
var headers = new Dictionary<string, string>
{
{ "x-accepted-oauth-scopes", "user" },
{ "x-oauth-scopes", "user, public_repo, repo, gist" },
{ "x-ratelimit-limit", "5000" },
{ "x-ratelimit-remaining", "4997" },
{ "etag", "5634b0b187fd2e91e3126a75006cc4fa" }
};

var apiInfo = ApiInfoParser.ParseResponseHeaders(headers);

Assert.NotNull(apiInfo);
Assert.Equal(new[] { "user" }, apiInfo.AcceptedOauthScopes.ToArray());
Assert.Equal(new[] { "user", "public_repo", "repo", "gist" }, apiInfo.OauthScopes.ToArray());
Assert.Equal(5000, apiInfo.RateLimit.Limit);
Assert.Equal(4997, apiInfo.RateLimit.Remaining);
Assert.Equal("5634b0b187fd2e91e3126a75006cc4fa", apiInfo.Etag);
}

[Fact]
public void BadHeadersAreIgnored()
{
Expand Down
3 changes: 2 additions & 1 deletion Octokit/Clients/Enterprise/EnterpriseProbe.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Text.RegularExpressions;
Expand Down Expand Up @@ -103,7 +104,7 @@ public async Task<EnterpriseProbeResult> Probe(Uri enterpriseBaseUrl)

static bool IsEnterpriseResponse(IResponse response)
{
return response.Headers.ContainsKey("X-GitHub-Request-Id");
return response.Headers.Any(h => string.Equals(h.Key, "X-GitHub-Request-Id", StringComparison.OrdinalIgnoreCase));
}
}

Expand Down
8 changes: 5 additions & 3 deletions Octokit/Exceptions/AbuseException.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Net;
#if !NO_SERIALIZABLE
using System.Runtime.Serialization;
Expand Down Expand Up @@ -44,11 +46,11 @@ public AbuseException(IResponse response, Exception innerException)

private static int? ParseRetryAfterSeconds(IResponse response)
{
string secondsValue;
if (!response.Headers.TryGetValue("Retry-After", out secondsValue)) { return null; }
var header = response.Headers.FirstOrDefault(h => string.Equals(h.Key, "Retry-After", StringComparison.OrdinalIgnoreCase));
if (header.Equals(default(KeyValuePair<string, string>))) { return null; }

int retrySeconds;
if (!int.TryParse(secondsValue, out retrySeconds)) { return null; }
if (!int.TryParse(header.Value, out retrySeconds)) { return null; }
if (retrySeconds < 0) { return null; }

return retrySeconds;
Expand Down
1 change: 1 addition & 0 deletions Octokit/Helpers/ConcurrentCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ namespace Octokit
/// </summary>
/// <typeparam name="TKey"></typeparam>
/// <typeparam name="TValue"></typeparam>
[Obsolete("This component is no longer used, and will be removed in a future update")]
public class ConcurrentCache<TKey, TValue>
{
Dictionary<TKey, TValue> _cache = new Dictionary<TKey, TValue>();
Expand Down
30 changes: 22 additions & 8 deletions Octokit/Http/ApiInfoParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ internal static class ApiInfoParser
static readonly Regex _linkRelRegex = new Regex("rel=\"(next|prev|first|last)\"", regexOptions);
static readonly Regex _linkUriRegex = new Regex("<(.+)>", regexOptions);

static KeyValuePair<string, string> LookupHeader(IDictionary<string, string> headers, string key)
{
return headers.FirstOrDefault(h => string.Equals(h.Key, key, StringComparison.OrdinalIgnoreCase));
}

static bool Exists(KeyValuePair<string, string> kvp)
{
return !kvp.Equals(default(KeyValuePair<string, string>));
}

public static ApiInfo ParseResponseHeaders(IDictionary<string, string> responseHeaders)
{
Ensure.ArgumentNotNull(responseHeaders, nameof(responseHeaders));
Expand All @@ -25,28 +35,32 @@ public static ApiInfo ParseResponseHeaders(IDictionary<string, string> responseH
var acceptedOauthScopes = new List<string>();
string etag = null;

if (responseHeaders.ContainsKey("X-Accepted-OAuth-Scopes"))
var acceptedOauthScopesKey = LookupHeader(responseHeaders, "X-Accepted-OAuth-Scopes");
if (Exists(acceptedOauthScopesKey))
{
acceptedOauthScopes.AddRange(responseHeaders["X-Accepted-OAuth-Scopes"]
acceptedOauthScopes.AddRange(acceptedOauthScopesKey.Value
.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)
.Select(x => x.Trim()));
}

if (responseHeaders.ContainsKey("X-OAuth-Scopes"))
var oauthScopesKey = LookupHeader(responseHeaders, "X-OAuth-Scopes");
if (Exists(oauthScopesKey))
{
oauthScopes.AddRange(responseHeaders["X-OAuth-Scopes"]
oauthScopes.AddRange(oauthScopesKey.Value
.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries)
.Select(x => x.Trim()));
}

if (responseHeaders.ContainsKey("ETag"))
var etagKey = LookupHeader(responseHeaders, "ETag");
if (Exists(etagKey))
{
etag = responseHeaders["ETag"];
etag = etagKey.Value;
}

if (responseHeaders.ContainsKey("Link"))
var linkKey = LookupHeader(responseHeaders, "Link");
if (Exists(linkKey))
{
var links = responseHeaders["Link"].Split(',');
var links = linkKey.Value.Split(',');
foreach (var link in links)
{
var relMatch = _linkRelRegex.Match(link);
Expand Down
26 changes: 24 additions & 2 deletions Octokit/Http/RateLimit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
#if !NO_SERIALIZABLE
using System.Runtime.Serialization;
#endif
Expand Down Expand Up @@ -65,11 +66,32 @@ public RateLimit(int limit, int remaining, long resetAsUtcEpochSeconds)
[Parameter(Key = "reset")]
public long ResetAsUtcEpochSeconds { get; private set; }

static KeyValuePair<string, string> LookupHeader(IDictionary<string, string> headers, string key)
{
return headers.FirstOrDefault(h => string.Equals(h.Key, key, StringComparison.OrdinalIgnoreCase));
}

static bool Exists(KeyValuePair<string, string> kvp)
{
return !kvp.Equals(default(KeyValuePair<string, string>));
}

static long GetHeaderValueAsInt32Safe(IDictionary<string, string> responseHeaders, string key)
{
string value;
long result;
return !responseHeaders.TryGetValue(key, out value) || value == null || !long.TryParse(value, out result)

var foundKey = LookupHeader(responseHeaders, key);
if (!Exists(foundKey))
{
return 0;
}

if (string.IsNullOrWhiteSpace(foundKey.Value))
{
return 0;
}

return !long.TryParse(foundKey.Value, out result)
? 0
: result;
}
Expand Down