diff --git a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs index 6f65dac2ec..bff337cdaa 100644 --- a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs @@ -604,6 +604,31 @@ public async Task ReturnsOrganizationRepository() Assert.Equal(AccountType.Organization, repository.Owner.Type); } + [IntegrationTest] + public async Task ReturnsRenamedRepository() + { + var github = Helper.GetAuthenticatedClient(); + + var repository = await github.Repository.Get("michael-wolfenden", "Polly"); + + Assert.Equal("https://github.com/App-vNext/Polly.git", repository.CloneUrl); + Assert.False(repository.Private); + Assert.False(repository.Fork); + //Assert.Equal(AccountType.User, repository.Owner.Type); + + repository = await github.Repository.Get("fsprojects", "FSharp.Atom"); + + Assert.Equal("https://github.com/ionide/ionide-atom-fsharp.git", repository.CloneUrl); + Assert.False(repository.Private); + Assert.False(repository.Fork); + + repository = await github.Repository.Get("cabbage89", "Orchard.Weixin"); + + Assert.Equal("https://github.com/cabbage89/Orchard.WeChat.git", repository.CloneUrl); + Assert.False(repository.Private); + Assert.False(repository.Fork); + } + [IntegrationTest] public async Task ReturnsOrganizationRepositoryWithRepositoryId() { 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 0d68b0ff02..860f1cb01a 100644 --- a/Octokit.Tests/Http/RedirectHandlerTests.cs +++ b/Octokit.Tests/Http/RedirectHandlerTests.cs @@ -14,10 +14,12 @@ public class RedirectHandlerTests [Fact] public async Task OkStatusShouldPassThrough() { - var invoker = CreateInvoker(new HttpResponseMessage(HttpStatusCode.OK)); + var handler = CreateMockHttpHandler(new HttpResponseMessage(HttpStatusCode.OK)); + var adapter = new HttpClientAdapter(handler); + var httpRequestMessage = CreateRequest(HttpMethod.Get); - var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken()); + var response = await adapter.SendAsync(httpRequestMessage, new CancellationToken()); Assert.Equal(response.StatusCode, HttpStatusCode.OK); Assert.Same(response.RequestMessage, httpRequestMessage); @@ -32,11 +34,12 @@ public async Task ShouldRedirectSameMethod(HttpStatusCode statusCode) var redirectResponse = new HttpResponseMessage(statusCode); redirectResponse.Headers.Location = new Uri("http://example.org/bar"); - var invoker = CreateInvoker(redirectResponse, - new HttpResponseMessage(HttpStatusCode.OK)); + var handler = CreateMockHttpHandler(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK)); + var adapter = new HttpClientAdapter(handler); + var httpRequestMessage = CreateRequest(HttpMethod.Post); - var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken()); + var response = await adapter.SendAsync(httpRequestMessage, new CancellationToken()); Assert.Equal(response.RequestMessage.Method, httpRequestMessage.Method); Assert.NotSame(response.RequestMessage, httpRequestMessage); @@ -48,11 +51,12 @@ public async Task Status303ShouldRedirectChangeMethod() var redirectResponse = new HttpResponseMessage(HttpStatusCode.SeeOther); redirectResponse.Headers.Location = new Uri("http://example.org/bar"); - var invoker = CreateInvoker(redirectResponse, - new HttpResponseMessage(HttpStatusCode.OK)); + var handler = CreateMockHttpHandler(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK)); + var adapter = new HttpClientAdapter(handler); + var httpRequestMessage = CreateRequest(HttpMethod.Post); - var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken()); + var response = await adapter.SendAsync(httpRequestMessage, new CancellationToken()); Assert.Equal(HttpMethod.Get, response.RequestMessage.Method); Assert.NotSame(response.RequestMessage, httpRequestMessage); @@ -64,12 +68,13 @@ public async Task RedirectWithSameHostShouldKeepAuthHeader() var redirectResponse = new HttpResponseMessage(HttpStatusCode.Redirect); redirectResponse.Headers.Location = new Uri("http://example.org/bar"); - var invoker = CreateInvoker(redirectResponse, - new HttpResponseMessage(HttpStatusCode.OK)); + var handler = CreateMockHttpHandler(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK)); + var adapter = new HttpClientAdapter(handler); + var httpRequestMessage = CreateRequest(HttpMethod.Get); httpRequestMessage.Headers.Authorization = new AuthenticationHeaderValue("fooAuth", "aparam"); - var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken()); + var response = await adapter.SendAsync(httpRequestMessage, new CancellationToken()); Assert.NotSame(response.RequestMessage, httpRequestMessage); Assert.Equal("fooAuth", response.RequestMessage.Headers.Authorization.Scheme); @@ -86,12 +91,13 @@ public async Task RedirectWithDifferentHostShouldLoseAuthHeader(HttpStatusCode s var redirectResponse = new HttpResponseMessage(statusCode); redirectResponse.Headers.Location = new Uri("http://example.net/bar"); - var invoker = CreateInvoker(redirectResponse, - new HttpResponseMessage(HttpStatusCode.OK)); + var handler = CreateMockHttpHandler(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK)); + var adapter = new HttpClientAdapter(handler); + var httpRequestMessage = CreateRequest(HttpMethod.Get); httpRequestMessage.Headers.Authorization = new AuthenticationHeaderValue("fooAuth", "aparam"); - var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken()); + var response = await adapter.SendAsync(httpRequestMessage, new CancellationToken()); Assert.NotSame(response.RequestMessage, httpRequestMessage); Assert.Null(response.RequestMessage.Headers.Authorization); @@ -106,16 +112,16 @@ public async Task Status301ShouldRedirectPOSTWithBody(HttpStatusCode statusCode) var redirectResponse = new HttpResponseMessage(statusCode); redirectResponse.Headers.Location = new Uri("http://example.org/bar"); - var invoker = CreateInvoker(redirectResponse, - new HttpResponseMessage(HttpStatusCode.OK)); + var handler = CreateMockHttpHandler(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK)); + var adapter = new HttpClientAdapter(handler); + var httpRequestMessage = CreateRequest(HttpMethod.Post); httpRequestMessage.Content = new StringContent("Hello World"); - var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken()); - + var response = await adapter.SendAsync(httpRequestMessage, new CancellationToken()); + Assert.Equal(response.RequestMessage.Method, httpRequestMessage.Method); Assert.NotSame(response.RequestMessage, httpRequestMessage); - Assert.Equal("Hello World", await response.RequestMessage.Content.ReadAsStringAsync()); } // POST see other with content @@ -125,12 +131,13 @@ public async Task Status303ShouldRedirectToGETWithoutBody() var redirectResponse = new HttpResponseMessage(HttpStatusCode.SeeOther); redirectResponse.Headers.Location = new Uri("http://example.org/bar"); - var invoker = CreateInvoker(redirectResponse, - new HttpResponseMessage(HttpStatusCode.OK)); + var handler = CreateMockHttpHandler(redirectResponse, new HttpResponseMessage(HttpStatusCode.OK)); + var adapter = new HttpClientAdapter(handler); + var httpRequestMessage = CreateRequest(HttpMethod.Post); httpRequestMessage.Content = new StringContent("Hello World"); - - var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken()); + + var response = await adapter.SendAsync(httpRequestMessage, new CancellationToken()); Assert.Equal(HttpMethod.Get, response.RequestMessage.Method); Assert.NotSame(response.RequestMessage, httpRequestMessage); @@ -145,14 +152,14 @@ public async Task Exceed3RedirectsShouldReturn() var redirectResponse2 = new HttpResponseMessage(HttpStatusCode.Found); redirectResponse2.Headers.Location = new Uri("http://example.org/foo"); - - - var invoker = CreateInvoker(redirectResponse, redirectResponse2); + + var handler = CreateMockHttpHandler(redirectResponse, redirectResponse2); + var adapter = new HttpClientAdapter(handler); var httpRequestMessage = CreateRequest(HttpMethod.Get); Assert.ThrowsAsync( - () => invoker.SendAsync(httpRequestMessage, new CancellationToken())); + () => adapter.SendAsync(httpRequestMessage, new CancellationToken())); } static HttpRequestMessage CreateRequest(HttpMethod method) @@ -163,14 +170,9 @@ static HttpRequestMessage CreateRequest(HttpMethod method) return httpRequestMessage; } - static HttpMessageInvoker CreateInvoker(HttpResponseMessage httpResponseMessage1, HttpResponseMessage httpResponseMessage2 = null) + static Func CreateMockHttpHandler(HttpResponseMessage httpResponseMessage1, HttpResponseMessage httpResponseMessage2 = null) { - var redirectHandler = new RedirectHandler - { - InnerHandler = new MockRedirectHandler(httpResponseMessage1, httpResponseMessage2) - }; - var invoker = new HttpMessageInvoker(redirectHandler); - return invoker; + return () => new MockRedirectHandler(httpResponseMessage1, httpResponseMessage2); } } diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 16bb88682d..d97092f6ec 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -20,6 +20,8 @@ public class HttpClientAdapter : IHttpClient { readonly HttpClient _http; + public const string RedirectCountKey = "RedirectCount"; + [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] public HttpClientAdapter(Func getHandler) { @@ -42,8 +44,8 @@ public async Task Send(IRequest request, CancellationToken cancellati 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 @@ protected virtual void Dispose(bool disposing) if (_http != null) _http.Dispose(); } } - } - internal class RedirectHandler : DelegatingHandler - { - public const string RedirectCountKey = "RedirectCount"; - public bool Enabled { get; set; } - - protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + public async Task 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); + + // Send initial response + var response = await _http.SendAsync(request, HttpCompletionOption.ResponseContentRead, cancellationToken).ConfigureAwait(false); - // Can't redirect without somewhere to redirect too. 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; @@ -192,7 +195,6 @@ protected override async Task SendAsync(HttpRequestMessage { 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 @@ protected override async Task SendAsync(HttpRequestMessage || 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 (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); - } - } - 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); + 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); + newRequest.Headers.TryAddWithoutValidation(header.Key, header.Value); } + foreach (var property in oldRequest.Properties) { - newrequest.Properties.Add(property); + newRequest.Properties.Add(property); } - return newrequest; + return newRequest; } } -} + + internal class RedirectHandler : DelegatingHandler + { + } +} \ No newline at end of file