diff --git a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs index 14a87a46ef..bff337cdaa 100644 --- a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs @@ -618,7 +618,7 @@ public async Task ReturnsRenamedRepository() repository = await github.Repository.Get("fsprojects", "FSharp.Atom"); - Assert.Equal("https://github.com/ionide/ionide-fsharp.git", repository.CloneUrl); + Assert.Equal("https://github.com/ionide/ionide-atom-fsharp.git", repository.CloneUrl); Assert.False(repository.Private); Assert.False(repository.Fork); diff --git a/Octokit.Tests.Integration/RedirectTests.cs b/Octokit.Tests.Integration/RedirectTests.cs index b61416c2c0..7b4210b260 100644 --- a/Octokit.Tests.Integration/RedirectTests.cs +++ b/Octokit.Tests.Integration/RedirectTests.cs @@ -18,7 +18,7 @@ public async Task ReturnsRedirectedRepository() Assert.Equal(AccountType.User, repository.Owner.Type); } - [IntegrationTest(Skip = "This test is super-unreliable right now - see https://github.com/octokit/octokit.net/issues/874 for discussion")] + [IntegrationTest] public async Task CanCreateIssueOnRedirectedRepository() { var client = Helper.GetAuthenticatedClient(); diff --git a/Octokit.Tests/Http/RedirectHandlerTests.cs b/Octokit.Tests/Http/RedirectHandlerTests.cs index 581fe48317..860f1cb01a 100644 --- a/Octokit.Tests/Http/RedirectHandlerTests.cs +++ b/Octokit.Tests/Http/RedirectHandlerTests.cs @@ -122,8 +122,6 @@ public async Task Status301ShouldRedirectPOSTWithBody(HttpStatusCode statusCode) Assert.Equal(response.RequestMessage.Method, httpRequestMessage.Method); Assert.NotSame(response.RequestMessage, httpRequestMessage); - //TODO: response.RequestMessage.Content has been disposed so cant access it here - //Assert.Equal("Hello World", await response.RequestMessage.Content.ReadAsStringAsync()); } // POST see other with content diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 5cd231b434..d97092f6ec 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -173,26 +173,17 @@ protected virtual void Dispose(bool disposing) public async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - StreamContent savedContent = null; - bool unbuffered = false; - if (request.Content != null && request.Content.Headers.ContentLength != 0) - { - var stream = await request.Content.ReadAsStreamAsync().ConfigureAwait(false); - if (stream.CanSeek) - { - stream.Position = 0; - savedContent = new StreamContent(stream); - } - else - { - unbuffered = true; - } - } + // Clone the request/content incase we get a redirect + var clonedRequest = await CloneHttpRequestMessageAsync(request); + // Send initial response var response = await _http.SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); - // Can't redirect without somewhere to redirect to. Throw? - if (response.Headers.Location == null) return response; + // 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; @@ -204,7 +195,6 @@ public async Task SendAsync(HttpRequestMessage request, Can { 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 @@ -213,42 +203,60 @@ public async Task SendAsync(HttpRequestMessage request, Can || 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 - { - if (unbuffered) - { - throw new Exception("Cannot redirect a request with an unbuffered body"); - } - newRequest.Content = savedContent; - } - newRequest.RequestUri = response.Headers.Location; - if (string.Compare(newRequest.RequestUri.Host, request.RequestUri.Host, StringComparison.OrdinalIgnoreCase) != 0) + + // 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) { - newRequest.Headers.Authorization = null; + clonedRequest.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 CloneHttpRequestMessageAsync(HttpRequestMessage oldRequest) { 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); @@ -259,6 +267,6 @@ private static HttpRequestMessage CopyRequest(HttpRequestMessage oldRequest) } internal class RedirectHandler : DelegatingHandler - { + { } } \ No newline at end of file