From 8d7bda96e41d7415812d821f5bb4402c0ea33d44 Mon Sep 17 00:00:00 2001 From: Brendan Forster Date: Sun, 12 Apr 2020 13:04:30 -0300 Subject: [PATCH] handle case insensitive headers when parsing for API rate limiting (#2175) --- .../Enterprise/EnterpriseProbeTests.cs | 20 ++++++++++--- .../Exceptions/AbuseExceptionTests.cs | 11 +++++++ Octokit.Tests/Http/ApiInfoParserTests.cs | 22 ++++++++++++++ Octokit/Clients/Enterprise/EnterpriseProbe.cs | 3 +- Octokit/Exceptions/AbuseException.cs | 8 +++-- Octokit/Helpers/ConcurrentCache.cs | 1 + Octokit/Http/ApiInfoParser.cs | 30 ++++++++++++++----- Octokit/Http/RateLimit.cs | 26 ++++++++++++++-- 8 files changed, 103 insertions(+), 18 deletions(-) diff --git a/Octokit.Tests/Clients/Enterprise/EnterpriseProbeTests.cs b/Octokit.Tests/Clients/Enterprise/EnterpriseProbeTests.cs index 54df7aba7b..0d225f58a5 100644 --- a/Octokit.Tests/Clients/Enterprise/EnterpriseProbeTests.cs +++ b/Octokit.Tests/Clients/Enterprise/EnterpriseProbeTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Net; using System.Net.Http; using System.Reactive.Linq; @@ -19,10 +20,15 @@ public class TheProbeMethod [InlineData(HttpStatusCode.NotFound)] public async Task ReturnsExistsForResponseWithCorrectHeadersRegardlessOfResponse(HttpStatusCode httpStatusCode) { + var headers = new Dictionary() + { + { "Server", "REVERSE-PROXY" }, + { "X-GitHub-Request-Id", Guid.NewGuid().ToString() } + }; var response = Substitute.For(); 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(); httpClient.Send(Args.Request, CancellationToken.None).Returns(Task.FromResult(response)); @@ -43,10 +49,16 @@ public async Task ReturnsExistsForResponseWithCorrectHeadersRegardlessOfResponse [Fact] public async Task ReturnsExistsForApiExceptionWithCorrectHeaders() { + + var headers = new Dictionary() + { + { "Server", "GitHub.com" }, + { "X-GitHub-Request-Id", Guid.NewGuid().ToString() } + }; var httpClient = Substitute.For(); var response = Substitute.For(); - 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"); diff --git a/Octokit.Tests/Exceptions/AbuseExceptionTests.cs b/Octokit.Tests/Exceptions/AbuseExceptionTests.cs index 17b2616c14..80285bc11f 100644 --- a/Octokit.Tests/Exceptions/AbuseExceptionTests.cs +++ b/Octokit.Tests/Exceptions/AbuseExceptionTests.cs @@ -53,6 +53,17 @@ public void WithRetryAfterHeader_PopulatesRetryAfterSeconds() Assert.Equal(30, abuseException.RetryAfterSeconds); } + [Fact] + public void WithRetryAfterCaseInsensitiveHeader_PopulatesRetryAfterSeconds() + { + var headerDictionary = new Dictionary { { "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() { diff --git a/Octokit.Tests/Http/ApiInfoParserTests.cs b/Octokit.Tests/Http/ApiInfoParserTests.cs index 00e3215b4e..ed837640ac 100644 --- a/Octokit.Tests/Http/ApiInfoParserTests.cs +++ b/Octokit.Tests/Http/ApiInfoParserTests.cs @@ -32,6 +32,28 @@ public void ParsesApiInfoFromHeaders() Assert.Equal("5634b0b187fd2e91e3126a75006cc4fa", apiInfo.Etag); } + [Fact] + public void ParsesApiInfoFromCaseInsensitiveHeaders() + { + var headers = new Dictionary + { + { "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() { diff --git a/Octokit/Clients/Enterprise/EnterpriseProbe.cs b/Octokit/Clients/Enterprise/EnterpriseProbe.cs index 3584825ea6..45173529a0 100644 --- a/Octokit/Clients/Enterprise/EnterpriseProbe.cs +++ b/Octokit/Clients/Enterprise/EnterpriseProbe.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Net; using System.Net.Http; using System.Text.RegularExpressions; @@ -103,7 +104,7 @@ public async Task 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)); } } diff --git a/Octokit/Exceptions/AbuseException.cs b/Octokit/Exceptions/AbuseException.cs index 8fceec4f3a..5e94754b95 100644 --- a/Octokit/Exceptions/AbuseException.cs +++ b/Octokit/Exceptions/AbuseException.cs @@ -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; @@ -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))) { 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; diff --git a/Octokit/Helpers/ConcurrentCache.cs b/Octokit/Helpers/ConcurrentCache.cs index 404eff0e48..55e6d3d21f 100644 --- a/Octokit/Helpers/ConcurrentCache.cs +++ b/Octokit/Helpers/ConcurrentCache.cs @@ -11,6 +11,7 @@ namespace Octokit /// /// /// + [Obsolete("This component is no longer used, and will be removed in a future update")] public class ConcurrentCache { Dictionary _cache = new Dictionary(); diff --git a/Octokit/Http/ApiInfoParser.cs b/Octokit/Http/ApiInfoParser.cs index 94a51c5649..e9cf2fb0bd 100644 --- a/Octokit/Http/ApiInfoParser.cs +++ b/Octokit/Http/ApiInfoParser.cs @@ -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 LookupHeader(IDictionary headers, string key) + { + return headers.FirstOrDefault(h => string.Equals(h.Key, key, StringComparison.OrdinalIgnoreCase)); + } + + static bool Exists(KeyValuePair kvp) + { + return !kvp.Equals(default(KeyValuePair)); + } + public static ApiInfo ParseResponseHeaders(IDictionary responseHeaders) { Ensure.ArgumentNotNull(responseHeaders, nameof(responseHeaders)); @@ -25,28 +35,32 @@ public static ApiInfo ParseResponseHeaders(IDictionary responseH var acceptedOauthScopes = new List(); 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); diff --git a/Octokit/Http/RateLimit.cs b/Octokit/Http/RateLimit.cs index a5c2f28800..016ef4944a 100644 --- a/Octokit/Http/RateLimit.cs +++ b/Octokit/Http/RateLimit.cs @@ -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 @@ -65,11 +66,32 @@ public RateLimit(int limit, int remaining, long resetAsUtcEpochSeconds) [Parameter(Key = "reset")] public long ResetAsUtcEpochSeconds { get; private set; } + static KeyValuePair LookupHeader(IDictionary headers, string key) + { + return headers.FirstOrDefault(h => string.Equals(h.Key, key, StringComparison.OrdinalIgnoreCase)); + } + + static bool Exists(KeyValuePair kvp) + { + return !kvp.Equals(default(KeyValuePair)); + } + static long GetHeaderValueAsInt32Safe(IDictionary 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; }