Fix timeout getting multiple repositories (#1411)

* add test

* [WIP]

* put logic for redirects outside of delegating handler

* change send method

* format code

* reorganized http client adapter

* change HttpClientAdapter

* rework http redirect tests - still an issue with accessing the response.RequestMessage.Content property as it is disposed

* remove some unused lines in httpclientadapter

* Reworked redirect implementation to fully clone http request and re-use it later
Now the skipped test from #874 works!
Also had to fix the new ReturnsRenamedRepository test as the ionide repo was renamed again
This commit is contained in:
Martin Scholz
2016-08-14 22:57:50 +02:00
committed by Ryan Gribble
parent ef0da2f84d
commit 5b9e23c2fb
4 changed files with 126 additions and 82 deletions
+64 -47
View File
@@ -20,6 +20,8 @@ namespace Octokit.Internal
{
readonly HttpClient _http;
public const string RedirectCountKey = "RedirectCount";
[SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
public HttpClientAdapter(Func<HttpMessageHandler> getHandler)
{
@@ -42,8 +44,8 @@ namespace Octokit.Internal
using (var requestMessage = BuildRequestMessage(request))
{
// Make the request
var responseMessage = await _http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationTokenForRequest).ConfigureAwait(false);
var responseMessage = await SendAsync(requestMessage, cancellationTokenForRequest).ConfigureAwait(false);
return await BuildResponse(responseMessage).ConfigureAwait(false);
}
}
@@ -168,19 +170,20 @@ namespace Octokit.Internal
if (_http != null) _http.Dispose();
}
}
}
internal class RedirectHandler : DelegatingHandler
{
public const string RedirectCountKey = "RedirectCount";
public bool Enabled { get; set; }
protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);
// Clone the request/content incase we get a redirect
var clonedRequest = await CloneHttpRequestMessageAsync(request);
// Can't redirect without somewhere to redirect too. Throw?
if (response.Headers.Location == null) return response;
// Send initial response
var response = await _http.SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false);
// Can't redirect without somewhere to redirect to.
if (response.Headers.Location == null)
{
return response;
}
// Don't redirect if we exceed max number of redirects
var redirectCount = 0;
@@ -192,7 +195,6 @@ namespace Octokit.Internal
{
throw new InvalidOperationException("The redirect count for this request has been exceeded. Aborting.");
}
request.Properties[RedirectCountKey] = ++redirectCount;
if (response.StatusCode == HttpStatusCode.MovedPermanently
|| response.StatusCode == HttpStatusCode.Redirect
@@ -201,55 +203,70 @@ namespace Octokit.Internal
|| response.StatusCode == HttpStatusCode.TemporaryRedirect
|| (int)response.StatusCode == 308)
{
var newRequest = CopyRequest(response.RequestMessage);
if (response.StatusCode == HttpStatusCode.SeeOther)
{
newRequest.Content = null;
newRequest.Method = HttpMethod.Get;
clonedRequest.Content = null;
clonedRequest.Method = HttpMethod.Get;
}
else
// Increment the redirect count
clonedRequest.Properties[RedirectCountKey] = ++redirectCount;
// Set the new Uri based on location header
clonedRequest.RequestUri = response.Headers.Location;
// Clear authentication if redirected to a different host
if (string.Compare(clonedRequest.RequestUri.Host, request.RequestUri.Host, StringComparison.OrdinalIgnoreCase) != 0)
{
if (request.Content != null && request.Content.Headers.ContentLength != 0)
{
var stream = await request.Content.ReadAsStreamAsync().ConfigureAwait(false);
if (stream.CanSeek)
{
stream.Position = 0;
}
else
{
throw new Exception("Cannot redirect a request with an unbuffered body");
}
newRequest.Content = new StreamContent(stream);
}
clonedRequest.Headers.Authorization = null;
}
newRequest.RequestUri = response.Headers.Location;
if (string.Compare(newRequest.RequestUri.Host, request.RequestUri.Host, StringComparison.OrdinalIgnoreCase) != 0)
{
newRequest.Headers.Authorization = null;
}
response = await SendAsync(newRequest, cancellationToken).ConfigureAwait(false);
// Send redirected request
response = await SendAsync(clonedRequest, cancellationToken).ConfigureAwait(false);
}
return response;
}
[SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
private static HttpRequestMessage CopyRequest(HttpRequestMessage oldRequest)
public static async Task<HttpRequestMessage> CloneHttpRequestMessageAsync(HttpRequestMessage oldRequest)
{
var newrequest = new HttpRequestMessage(oldRequest.Method, oldRequest.RequestUri);
var newRequest = new HttpRequestMessage(oldRequest.Method, oldRequest.RequestUri);
// Copy the request's content (via a MemoryStream) into the cloned object
var ms = new MemoryStream();
if (oldRequest.Content != null)
{
await oldRequest.Content.CopyToAsync(ms).ConfigureAwait(false);
ms.Position = 0;
newRequest.Content = new StreamContent(ms);
// Copy the content headers
if (oldRequest.Content.Headers != null)
{
foreach (var h in oldRequest.Content.Headers)
{
newRequest.Content.Headers.Add(h.Key, h.Value);
}
}
}
newRequest.Version = oldRequest.Version;
foreach (var header in oldRequest.Headers)
{
newrequest.Headers.TryAddWithoutValidation(header.Key, header.Value);
}
foreach (var property in oldRequest.Properties)
{
newrequest.Properties.Add(property);
newRequest.Headers.TryAddWithoutValidation(header.Key, header.Value);
}
return newrequest;
foreach (var property in oldRequest.Properties)
{
newRequest.Properties.Add(property);
}
return newRequest;
}
}
}
internal class RedirectHandler : DelegatingHandler
{
}
}