Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support redirect requests natively #808

Merged
merged 29 commits into from
Jul 17, 2015
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
14fe8bc
First attempt to reuse HttpClient
darrelmiller May 13, 2015
7f95f55
massaging and fighting with fxcop
shiftkey May 22, 2015
4d77f0e
suppressing a bunch of stuff because murtaugh's law
shiftkey May 22, 2015
f0011cc
missed an overload
shiftkey May 22, 2015
894609e
missed a supresssion
shiftkey May 22, 2015
eb7e14b
test hacks for great good
shiftkey May 22, 2015
ea9f3a4
Fixed issues with RedirectHandler and added tests
darrelmiller May 22, 2015
5d0c8d6
get the portable tests back running by muting all these methods that …
shiftkey May 31, 2015
e5e4b4c
extracting all the cross-platform setup of HttpMessageHandler into
shiftkey May 31, 2015
549ff92
a bit more cleanup
shiftkey May 31, 2015
d551eb2
internalize this for now
shiftkey May 31, 2015
3c32814
:lipstick:
shiftkey May 31, 2015
be57a70
wrote some docs
shiftkey May 31, 2015
d7d7efd
renaming things is hard
shiftkey May 31, 2015
74ac314
review and cleanup suppressions
shiftkey May 31, 2015
3859ff3
the test, it passes
shiftkey May 31, 2015
3acdd10
deprecate the optional AllowAutoRedirect parameter
shiftkey May 31, 2015
54e0865
introduce GetArchive overloads for RepositoryContentClients which han…
shiftkey May 31, 2015
51947ad
added another exclusion for detecting binary content
shiftkey May 31, 2015
9a634e4
docs docs docs
shiftkey May 31, 2015
fe36783
and the other docs
shiftkey May 31, 2015
ea48040
Merge pull request #810 from octokit/fix-redirect-with-archive-link
shiftkey Jun 1, 2015
72339c8
added tests specific to redirects
shiftkey Jun 5, 2015
442bbb4
throw an error if the redirect count has been exceeded
shiftkey Jun 5, 2015
eafd63d
oops, corrected the test
shiftkey Jun 5, 2015
982dca5
specify the timeout interval when downloading the archive
shiftkey Jun 15, 2015
0acdb8c
Merge branch 'master' into redirect-requests
shiftkey Jun 15, 2015
5be1cae
:art: Really anal fix-ups of extra newlines
haacked Jul 17, 2015
8e77f00
:art: Minor formatting changes
haacked Jul 17, 2015
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Octokit.Tests.Integration/HttpClientAdapterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class TheSendAsyncMethod
[IntegrationTest]
public async Task CanDownloadImage()
{
var httpClient = new HttpClientAdapter();
var httpClient = new HttpClientAdapter(HttpMessageHandlerFactory.GetHandler);
var request = new Request
{
BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute),
Expand All @@ -36,7 +36,7 @@ public async Task CanDownloadImage()
[IntegrationTest]
public async Task CanCancelARequest()
{
var httpClient = new HttpClientAdapter();
var httpClient = new HttpClientAdapter(HttpMessageHandlerFactory.GetHandler);
var request = new Request
{
BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute),
Expand Down
5 changes: 5 additions & 0 deletions Octokit.Tests/Http/HttpClientAdapterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ public async Task SetsContentType()

sealed class HttpClientAdapterTester : HttpClientAdapter
{
public HttpClientAdapterTester()
: base (HttpMessageHandlerFactory.GetHandler)
{
}

public HttpRequestMessage BuildRequestMessageTester(IRequest request)
{
return BuildRequestMessage(request);
Expand Down
225 changes: 225 additions & 0 deletions Octokit.Tests/Http/RedirectHandlerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Octokit.Internal;
using Xunit;

namespace Octokit.Tests.Http
{
public class RedirectHandlerTests
{

[Fact]
public async Task OkStatusShouldPassThrough()
{
var invoker = CreateInvoker(new HttpResponseMessage(HttpStatusCode.OK));
var httpRequestMessage = CreateRequest(HttpMethod.Get);

var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.Equal(response.StatusCode, HttpStatusCode.OK);
Assert.Same(response.RequestMessage, httpRequestMessage);
}


[Theory]
[InlineData(HttpStatusCode.MovedPermanently)] // 301
[InlineData(HttpStatusCode.Found)] // 302
[InlineData(HttpStatusCode.TemporaryRedirect)] // 307
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 httpRequestMessage = CreateRequest(HttpMethod.Post);

var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.Equal(response.RequestMessage.Method, httpRequestMessage.Method);
Assert.NotSame(response.RequestMessage, httpRequestMessage);
}

[Fact]
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 httpRequestMessage = CreateRequest(HttpMethod.Post);

var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.Equal(HttpMethod.Get, response.RequestMessage.Method);
Assert.NotSame(response.RequestMessage, httpRequestMessage);
}

[Fact]
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 httpRequestMessage = CreateRequest(HttpMethod.Get);
httpRequestMessage.Headers.Authorization = new AuthenticationHeaderValue("fooAuth", "aparam");

var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.NotSame(response.RequestMessage, httpRequestMessage);
Assert.Equal("fooAuth", response.RequestMessage.Headers.Authorization.Scheme);
}


[Theory]
[InlineData(HttpStatusCode.MovedPermanently)] // 301
[InlineData(HttpStatusCode.Found)] // 302
[InlineData(HttpStatusCode.SeeOther)] // 303
[InlineData(HttpStatusCode.TemporaryRedirect)] // 307
public async Task RedirectWithDifferentHostShouldLoseAuthHeader(HttpStatusCode statusCode)
{
var redirectResponse = new HttpResponseMessage(statusCode);
redirectResponse.Headers.Location = new Uri("http://example.net/bar");

var invoker = CreateInvoker(redirectResponse,
new HttpResponseMessage(HttpStatusCode.OK));
var httpRequestMessage = CreateRequest(HttpMethod.Get);
httpRequestMessage.Headers.Authorization = new AuthenticationHeaderValue("fooAuth", "aparam");

var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.NotSame(response.RequestMessage, httpRequestMessage);
Assert.Null(response.RequestMessage.Headers.Authorization);

}

[Fact]
public async Task DisabledRedirectShouldPassThrough()
{
var invoker = CreateInvoker(new HttpResponseMessage(HttpStatusCode.Found));
var httpRequestMessage = CreateRequest(HttpMethod.Get);
httpRequestMessage.Properties[RedirectHandler.AllowAutoRedirectKey] = false;
var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.Equal(response.StatusCode, HttpStatusCode.Redirect);
Assert.Same(response.RequestMessage, httpRequestMessage);
}

[Theory]
[InlineData(HttpStatusCode.MovedPermanently)] // 301
[InlineData(HttpStatusCode.Found)] // 302
[InlineData(HttpStatusCode.TemporaryRedirect)] // 307
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 httpRequestMessage = CreateRequest(HttpMethod.Post);
httpRequestMessage.Content = new StringContent("Hello World");

var response = await invoker.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
[Fact]
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 httpRequestMessage = CreateRequest(HttpMethod.Post);
httpRequestMessage.Content = new StringContent("Hello World");

var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.Equal(HttpMethod.Get, response.RequestMessage.Method);
Assert.NotSame(response.RequestMessage, httpRequestMessage);
Assert.Null(response.RequestMessage.Content);
}

[Fact]
public async Task Exceed3RedirectsShouldReturn()
{
var redirectResponse = new HttpResponseMessage(HttpStatusCode.Found);
redirectResponse.Headers.Location = new Uri("http://example.org/bar");

var redirectResponse2 = new HttpResponseMessage(HttpStatusCode.Found);
redirectResponse2.Headers.Location = new Uri("http://example.org/foo");


var invoker = CreateInvoker(redirectResponse, redirectResponse2);

var httpRequestMessage = CreateRequest(HttpMethod.Get);

var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());

Assert.NotSame(response.RequestMessage, httpRequestMessage);
Assert.Equal(4, (int)response.RequestMessage.Properties[RedirectHandler.RedirectCountKey]);
}

static HttpRequestMessage CreateRequest(HttpMethod method)
{
var httpRequestMessage = new HttpRequestMessage();
httpRequestMessage.RequestUri = new Uri("http://example.org/foo");
httpRequestMessage.Properties[RedirectHandler.AllowAutoRedirectKey] = true;
httpRequestMessage.Method = method;
return httpRequestMessage;
}

static HttpMessageInvoker CreateInvoker(HttpResponseMessage httpResponseMessage1, HttpResponseMessage httpResponseMessage2 = null)
{

var redirectHandler = new RedirectHandler()
{
InnerHandler = new MockRedirectHandler(httpResponseMessage1, httpResponseMessage2)
};
var invoker = new HttpMessageInvoker(redirectHandler);
return invoker;
}
}

public class MockRedirectHandler : HttpMessageHandler
{
readonly HttpResponseMessage _response1;
readonly HttpResponseMessage _response2;
private bool _Response1Sent = false;
public MockRedirectHandler(HttpResponseMessage response1, HttpResponseMessage response2 = null)
{
_response1 = response1;
_response2 = response2;
}

protected async override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
if (!_Response1Sent)
{
_Response1Sent = true;
_response1.RequestMessage = request;
return _response1;
}
else
{
_response2.RequestMessage = request;
return _response2;
}
}
}
}
1 change: 1 addition & 0 deletions Octokit.Tests/Octokit.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@
<Compile Include="Authentication\TokenAuthenticatorTests.cs" />
<Compile Include="Http\ConnectionTests.cs" />
<Compile Include="Http\RateLimitTests.cs" />
<Compile Include="Http\RedirectHandlerTests.cs" />
<Compile Include="Http\ResponseTests.cs" />
<Compile Include="Http\RequestTests.cs" />
<Compile Include="Models\DeploymentStatusTests.cs" />
Expand Down
Binary file added Octokit/GlobalSuppressions.cs
Binary file not shown.
3 changes: 2 additions & 1 deletion Octokit/Http/Connection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,9 @@ public Connection(ProductHeaderValue productInformation, ICredentialStore creden
/// The address to point this client to such as https://api.github.com or the URL to a GitHub Enterprise
/// instance</param>
/// <param name="credentialStore">Provides credentials to the client when making requests</param>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")]
public Connection(ProductHeaderValue productInformation, Uri baseAddress, ICredentialStore credentialStore)
: this(productInformation, baseAddress, credentialStore, new HttpClientAdapter(), new SimpleJsonSerializer())
: this(productInformation, baseAddress, credentialStore, new HttpClientAdapter(HttpMessageHandlerFactory.GetHandler), new SimpleJsonSerializer())
{
}

Expand Down
Loading