handle case insensitive headers when parsing for API rate limiting (#…

…2175)
octokit · Apr 12, 2020 · 8d7bda9 · 8d7bda9
1 parent 2bfd101
commit 8d7bda9
Show file tree

Hide file tree

Showing 8 changed files with 103 additions and 18 deletions.
diff --git 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<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));
@@ -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");

diff --git 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<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()
                     {

diff --git 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<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()
             {

diff --git 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<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));
         }
     }
 

diff --git 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<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;

diff --git a/Octokit/Helpers/ConcurrentCache.cs b/Octokit/Helpers/ConcurrentCache.cs
@@ -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>();

diff --git 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<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));
@@ -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);

diff --git 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<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;
         }