diff --git a/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs b/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs index 4b56ed1b4f..a5221a6bd8 100644 --- a/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs +++ b/Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs @@ -34,8 +34,18 @@ public interface IObservableRepositoryContentsClient /// The owner of the repository /// The name of the repository /// + [Obsolete("Use GetArchive to download the archive instead")] IObservable GetArchiveLink(string owner, string name); + /// + /// Get an archive of a given repository's contents + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// A promise, containing the binary contents of the archive + IObservable GetArchive(string owner, string name); + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -47,8 +57,19 @@ public interface IObservableRepositoryContentsClient /// The name of the repository /// The format of the archive. Can be either tarball or zipball /// + [Obsolete("Use GetArchive to download the archive instead")] IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat); + /// + /// Get an archive of a given repository's contents, in a specific format + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A promise, containing the binary contents of the archive + IObservable GetArchive(string owner, string name, ArchiveFormat archiveFormat); + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -61,8 +82,20 @@ public interface IObservableRepositoryContentsClient /// The format of the archive. Can be either tarball or zipball /// A valid Git reference. /// + [Obsolete("Use GetArchive to download the archive instead")] IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference); + /// + /// Get an archive of a given repository's contents, using a specific format and reference + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// A promise, containing the binary contents of the archive + IObservable GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference); + /// /// Returns the contents of a file or directory in a repository. /// diff --git a/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs b/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs index ca2889e478..a4a5e4b256 100644 --- a/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs +++ b/Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs @@ -59,11 +59,24 @@ public IObservable GetReadmeHtml(string owner, string name) /// The owner of the repository /// The name of the repository /// + [Obsolete("Use GetArchive to download the archive instead")] public IObservable GetArchiveLink(string owner, string name) { return GetArchiveLink(owner, name, ArchiveFormat.Tarball, string.Empty); } + /// + /// Get an archive of a given repository's contents + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// A promise, containing the binary contents of the archive + public IObservable GetArchive(string owner, string name) + { + return _client.Repository.Content.GetArchive(owner, name).ToObservable(); + } + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -75,11 +88,25 @@ public IObservable GetArchiveLink(string owner, string name) /// The name of the repository /// The format of the archive. Can be either tarball or zipball /// + [Obsolete("Use GetArchive to download the archive instead")] public IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat) { return GetArchiveLink(owner, name, archiveFormat, String.Empty); } + /// + /// Get an archive of a given repository's contents, in a specific format + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A promise, containing the binary contents of the archive + public IObservable GetArchive(string owner, string name, ArchiveFormat archiveFormat) + { + return _client.Repository.Content.GetArchive(owner, name, archiveFormat).ToObservable(); + } + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -92,6 +119,7 @@ public IObservable GetArchiveLink(string owner, string name, ArchiveForm /// The format of the archive. Can be either tarball or zipball /// A valid Git reference. /// + [Obsolete("Use GetArchive to download the archive instead")] public IObservable GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference) { Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); @@ -100,6 +128,20 @@ public IObservable GetArchiveLink(string owner, string name, ArchiveForm return _client.Repository.Content.GetArchiveLink(owner, name, archiveFormat, reference).ToObservable(); } + /// + /// Get an archive of a given repository's contents, using a specific format and reference + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// A promise, containing the binary contents of the archive + public IObservable GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference) + { + return _client.Repository.Content.GetArchive(owner, name, archiveFormat, reference).ToObservable(); + } + /// /// Returns the contents of a file or directory in a repository. /// diff --git a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs index 12fc90db63..0c6786b74d 100644 --- a/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoriesClientTests.cs @@ -3,7 +3,6 @@ using System.Linq; using System.Threading.Tasks; using Octokit; -using Octokit.Tests.Helpers; using Octokit.Tests.Integration; using Xunit; @@ -540,7 +539,6 @@ public async Task ReturnsForkedRepository() } } - public class TheGetAllPublicMethod { [IntegrationTest(Skip = "Takes too long to run.")] diff --git a/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs b/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs index 707c4c474f..d2d0bf9d5a 100644 --- a/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs +++ b/Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs @@ -115,43 +115,43 @@ await Assert.ThrowsAsync( } } - [IntegrationTest] - public async Task GetsArchiveLinkAsTarball() + [IntegrationTest(Skip = "this will probably take too long")] + public async Task GetsArchiveAsTarball() { var github = Helper.GetAuthenticatedClient(); - var archiveLink = await github + var archive = await github .Repository .Content - .GetArchiveLink("octokit", "octokit.net"); + .GetArchive("octokit", "octokit.net"); - Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.tar.gz/master", archiveLink); + Assert.NotEmpty(archive); } [IntegrationTest] - public async Task GetsArchiveLinkAsZipball() + public async Task GetsArchiveAsZipball() { var github = Helper.GetAuthenticatedClient(); - var archiveLink = await github + var archive = await github .Repository .Content - .GetArchiveLink("octokit", "octokit.net", ArchiveFormat.Zipball, ""); + .GetArchive("octokit", "octokit.net", ArchiveFormat.Zipball, ""); - Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.zip/master", archiveLink); + Assert.NotEmpty(archive); } [IntegrationTest] - public async Task GetsArchiveLinkForReleaseBranchAsTarball() + public async Task GetsArchiveForReleaseBranchAsTarball() { var github = Helper.GetAuthenticatedClient(); - var archiveLink = await github + var archive = await github .Repository .Content - .GetArchiveLink("alfhenrik", "ScriptCs.OctoKit", ArchiveFormat.Tarball, "dev"); + .GetArchive("alfhenrik", "ScriptCs.OctoKit", ArchiveFormat.Tarball, "dev"); - Assert.Equal("https://codeload.github.com/alfhenrik/ScriptCs.OctoKit/legacy.tar.gz/dev", archiveLink); + Assert.NotEmpty(archive); } } } \ No newline at end of file diff --git a/Octokit.Tests.Integration/HttpClientAdapterTests.cs b/Octokit.Tests.Integration/HttpClientAdapterTests.cs index bf4a0fb41e..72087350f8 100644 --- a/Octokit.Tests.Integration/HttpClientAdapterTests.cs +++ b/Octokit.Tests.Integration/HttpClientAdapterTests.cs @@ -14,7 +14,7 @@ public class TheSendAsyncMethod [IntegrationTest] public async Task CanDownloadImage() { - var httpClient = new HttpClientAdapter(); + var httpClient = new HttpClientAdapter(HttpMessageHandlerFactory.CreateDefault); var request = new Request { BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute), @@ -36,7 +36,7 @@ public async Task CanDownloadImage() [IntegrationTest] public async Task CanCancelARequest() { - var httpClient = new HttpClientAdapter(); + var httpClient = new HttpClientAdapter(HttpMessageHandlerFactory.CreateDefault); var request = new Request { BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute), diff --git a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj index 58c045a7af..b587eb7ddf 100644 --- a/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj +++ b/Octokit.Tests.Integration/Octokit.Tests.Integration.csproj @@ -120,6 +120,7 @@ + diff --git a/Octokit.Tests.Integration/RedirectTests.cs b/Octokit.Tests.Integration/RedirectTests.cs new file mode 100644 index 0000000000..7b4210b260 --- /dev/null +++ b/Octokit.Tests.Integration/RedirectTests.cs @@ -0,0 +1,45 @@ +using System.Threading.Tasks; +using Xunit; + +namespace Octokit.Tests.Integration +{ + public class RedirectTests + { + [IntegrationTest] + public async Task ReturnsRedirectedRepository() + { + var github = Helper.GetAuthenticatedClient(); + + var repository = await github.Repository.Get("robconery", "massive"); + + Assert.Equal("https://github.com/FransBouma/Massive.git", repository.CloneUrl); + Assert.False(repository.Private); + Assert.False(repository.Fork); + Assert.Equal(AccountType.User, repository.Owner.Type); + } + + [IntegrationTest] + public async Task CanCreateIssueOnRedirectedRepository() + { + var client = Helper.GetAuthenticatedClient(); + + var owner = "shiftkey-tester"; + var oldRepoName = "repository-before-rename"; + var newRepoName = "repository-after-rename"; + + var newIssue = new NewIssue("a test issue") { Body = "A new unassigned issue" }; + var issue = await client.Issue.Create(owner, oldRepoName, newIssue); + Assert.NotNull(issue); + + Assert.True(issue.Url.AbsoluteUri.Contains("repository-after-rename")); + + var resolvedIssue = await client.Issue.Get(owner, newRepoName, issue.Number); + + Assert.NotNull(resolvedIssue); + + var update = resolvedIssue.ToUpdate(); + update.State = ItemState.Closed; + await client.Issue.Update(owner, oldRepoName, issue.Number, update); + } + } +} diff --git a/Octokit.Tests/Http/ConnectionTests.cs b/Octokit.Tests/Http/ConnectionTests.cs index d0bfd7cae3..02f1601ee7 100644 --- a/Octokit.Tests/Http/ConnectionTests.cs +++ b/Octokit.Tests/Http/ConnectionTests.cs @@ -439,7 +439,7 @@ await connection.Post( httpClient.Received().Send(Arg.Is(req => req.BaseAddress == _exampleUri && req.Body == body && - req.Headers["Accept"] == "application/vnd.github.v3+json; charset=utf-8" && + req.Headers["Accept"] == "application/vnd.github.quicksilver-preview+json; charset=utf-8, application/vnd.github.v3+json; charset=utf-8" && req.ContentType == "application/arbitrary" && req.Method == HttpMethod.Post && req.Endpoint == new Uri("https://other.host.com/path?query=val")), Args.CancellationToken); diff --git a/Octokit.Tests/Http/HttpClientAdapterTests.cs b/Octokit.Tests/Http/HttpClientAdapterTests.cs index 029b705236..32cf42acc0 100644 --- a/Octokit.Tests/Http/HttpClientAdapterTests.cs +++ b/Octokit.Tests/Http/HttpClientAdapterTests.cs @@ -178,6 +178,11 @@ public async Task SetsContentType() sealed class HttpClientAdapterTester : HttpClientAdapter { + public HttpClientAdapterTester() + : base(HttpMessageHandlerFactory.CreateDefault) + { + } + public HttpRequestMessage BuildRequestMessageTester(IRequest request) { return BuildRequestMessage(request); diff --git a/Octokit.Tests/Http/JsonHttpPipelineTests.cs b/Octokit.Tests/Http/JsonHttpPipelineTests.cs index be4b19eaa7..88a67be08f 100644 --- a/Octokit.Tests/Http/JsonHttpPipelineTests.cs +++ b/Octokit.Tests/Http/JsonHttpPipelineTests.cs @@ -30,7 +30,7 @@ public void SetsRequestAcceptHeader() jsonPipeline.SerializeRequest(request); Assert.Contains("Accept", request.Headers.Keys); - Assert.Equal("application/vnd.github.v3+json; charset=utf-8", request.Headers["Accept"]); + Assert.Equal("application/vnd.github.quicksilver-preview+json; charset=utf-8, application/vnd.github.v3+json; charset=utf-8", request.Headers["Accept"]); } [Fact] diff --git a/Octokit.Tests/Http/RedirectHandlerTests.cs b/Octokit.Tests/Http/RedirectHandlerTests.cs new file mode 100644 index 0000000000..ebd6fa56ad --- /dev/null +++ b/Octokit.Tests/Http/RedirectHandlerTests.cs @@ -0,0 +1,207 @@ +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); + } + + [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); + + Assert.ThrowsAsync( + () => invoker.SendAsync(httpRequestMessage, new CancellationToken())); + } + + static HttpRequestMessage CreateRequest(HttpMethod method) + { + var httpRequestMessage = new HttpRequestMessage(); + httpRequestMessage.RequestUri = new Uri("http://example.org/foo"); + 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 SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + if (!_Response1Sent) + { + _Response1Sent = true; + _response1.RequestMessage = request; + return _response1; + } + else + { + _response2.RequestMessage = request; + return _response2; + } + } + } +} diff --git a/Octokit.Tests/Octokit.Tests.csproj b/Octokit.Tests/Octokit.Tests.csproj index e1cbb6f47d..96aed6bd2e 100644 --- a/Octokit.Tests/Octokit.Tests.csproj +++ b/Octokit.Tests/Octokit.Tests.csproj @@ -141,6 +141,7 @@ + diff --git a/Octokit.sln.DotSettings b/Octokit.sln.DotSettings index 3ceaf70816..6fba030f79 100644 --- a/Octokit.sln.DotSettings +++ b/Octokit.sln.DotSettings @@ -263,6 +263,7 @@ II.2.12 <HandlesEvent /> <Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /> <Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /> True + True True True \ No newline at end of file diff --git a/Octokit/Clients/IRepositoryContentsClient.cs b/Octokit/Clients/IRepositoryContentsClient.cs index 45a217b0da..e02cd6893f 100644 --- a/Octokit/Clients/IRepositoryContentsClient.cs +++ b/Octokit/Clients/IRepositoryContentsClient.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Threading.Tasks; using Octokit.Internal; @@ -57,8 +58,18 @@ public interface IRepositoryContentsClient /// The owner of the repository /// The name of the repository /// + [Obsolete("Use GetArchive to download the archive instead")] Task GetArchiveLink(string owner, string name); + /// + /// Get an archive of a given repository's contents + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The binary contents of the archive + Task GetArchive(string owner, string name); + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -70,8 +81,19 @@ public interface IRepositoryContentsClient /// The name of the repository /// The format of the archive. Can be either tarball or zipball /// + [Obsolete("Use GetArchive to download the archive instead")] Task GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat); + /// + /// Get an archive of a given repository's contents, in a specific format + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// The binary contents of the archive + Task GetArchive(string owner, string name, ArchiveFormat archiveFormat); + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -84,8 +106,20 @@ public interface IRepositoryContentsClient /// The format of the archive. Can be either tarball or zipball /// A valid Git reference. /// + [Obsolete("Use GetArchive to download the archive instead")] Task GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference); + /// + /// Get an archive of a given repository's contents, using a specific format and reference + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// The binary contents of the archive + Task GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference); + /// /// Creates a commit that creates a new file in a repository. /// diff --git a/Octokit/Clients/RepositoryContentsClient.cs b/Octokit/Clients/RepositoryContentsClient.cs index 7b867f0198..4ad75d5ef4 100644 --- a/Octokit/Clients/RepositoryContentsClient.cs +++ b/Octokit/Clients/RepositoryContentsClient.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Threading.Tasks; namespace Octokit @@ -89,6 +90,18 @@ public Task GetArchiveLink(string owner, string name) return GetArchiveLink(owner, name, ArchiveFormat.Tarball, string.Empty); } + /// + /// Get an archive of a given repository's contents + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The binary contents of the archive + public Task GetArchive(string owner, string name) + { + return GetArchive(owner, name, ArchiveFormat.Tarball, string.Empty); + } + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -105,6 +118,19 @@ public Task GetArchiveLink(string owner, string name, ArchiveFormat arch return GetArchiveLink(owner, name, archiveFormat, string.Empty); } + /// + /// Get an archive of a given repository's contents, in a specific format + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// The binary contents of the archive + public Task GetArchive(string owner, string name, ArchiveFormat archiveFormat) + { + return GetArchive(owner, name, archiveFormat, string.Empty); + } + /// /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository. /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the @@ -125,6 +151,27 @@ public Task GetArchiveLink(string owner, string name, ArchiveFormat arch return ApiConnection.GetRedirect(ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference)); } + /// + /// Get an archive of a given repository's contents, using a specific format and reference + /// + /// https://developer.github.com/v3/repos/contents/#get-archive-link + /// The owner of the repository + /// The name of the repository + /// The format of the archive. Can be either tarball or zipball + /// A valid Git reference. + /// The binary contents of the archive + public async Task GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference) + { + Ensure.ArgumentNotNullOrEmptyString(owner, "owner"); + Ensure.ArgumentNotNullOrEmptyString(name, "name"); + + var endpoint = ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference); + + var response = await Connection.Get(endpoint, TimeSpan.FromMinutes(60)); + + return response.Body; + } + /// /// Creates a commit that creates a new file in a repository. /// diff --git a/Octokit/Http/Connection.cs b/Octokit/Http/Connection.cs index 182ac9b563..846923d3d8 100644 --- a/Octokit/Http/Connection.cs +++ b/Octokit/Http/Connection.cs @@ -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 /// Provides credentials to the client when making requests + [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.CreateDefault), new SimpleJsonSerializer()) { } @@ -152,12 +153,12 @@ public Task> Get(Uri uri, IDictionary paramet /// Specifies accepted response media types. /// To follow redirect links automatically or not /// representing the received HTTP response - + [Obsolete("allowAutoRedirect is no longer respected and will be deprecated in a future release")] public Task> Get(Uri uri, IDictionary parameters, string accepts, bool allowAutoRedirect) { Ensure.ArgumentNotNull(uri, "uri"); - return SendData(uri.ApplyParameters(parameters), HttpMethod.Get, null, accepts, null, CancellationToken.None, allowAutoRedirect: allowAutoRedirect); + return SendData(uri.ApplyParameters(parameters), HttpMethod.Get, null, accepts, null, CancellationToken.None); } public Task> Get(Uri uri, IDictionary parameters, string accepts, CancellationToken cancellationToken) @@ -167,6 +168,13 @@ public Task> Get(Uri uri, IDictionary paramet return SendData(uri.ApplyParameters(parameters), HttpMethod.Get, null, accepts, null, cancellationToken); } + public Task> Get(Uri uri, TimeSpan timeout) + { + Ensure.ArgumentNotNull(uri, "uri"); + + return SendData(uri, HttpMethod.Get, null, null, null, timeout, CancellationToken.None); + } + /// /// Performs an asynchronous HTTP GET request that expects a containing HTML. /// @@ -320,8 +328,7 @@ Task> SendData( string contentType, CancellationToken cancellationToken, string twoFactorAuthenticationCode = null, - Uri baseAddress = null, - bool allowAutoRedirect = true) + Uri baseAddress = null) { Ensure.ArgumentNotNull(uri, "uri"); @@ -330,7 +337,6 @@ Task> SendData( Method = method, BaseAddress = baseAddress ?? BaseAddress, Endpoint = uri, - AllowAutoRedirect = allowAutoRedirect, }; return SendDataInternal(body, accepts, contentType, cancellationToken, twoFactorAuthenticationCode, request); diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs index 0e9888fcfd..59dd87c025 100644 --- a/Octokit/Http/HttpClientAdapter.cs +++ b/Octokit/Http/HttpClientAdapter.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; using System.Net; @@ -17,13 +18,14 @@ namespace Octokit.Internal /// public class HttpClientAdapter : IHttpClient { - readonly IWebProxy _webProxy; + readonly HttpClient _http; - public HttpClientAdapter() { } - - public HttpClientAdapter(IWebProxy webProxy) + [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] + public HttpClientAdapter(Func getHandler) { - _webProxy = webProxy; + Ensure.ArgumentNotNull(getHandler, "getHandler"); + + _http = new HttpClient(new RedirectHandler { InnerHandler = getHandler() }); } /// @@ -36,21 +38,20 @@ public async Task Send(IRequest request, CancellationToken cancellati { Ensure.ArgumentNotNull(request, "request"); - var httpOptions = new HttpClientHandler - { - AllowAutoRedirect = request.AllowAutoRedirect - }; - if (httpOptions.SupportsAutomaticDecompression) - { - httpOptions.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; - } - if (httpOptions.SupportsProxy && _webProxy != null) + var cancellationTokenForRequest = GetCancellationTokenForRequest(request, cancellationToken); + + using (var requestMessage = BuildRequestMessage(request)) { - httpOptions.UseProxy = true; - httpOptions.Proxy = _webProxy; + // Make the request + var responseMessage = await _http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationTokenForRequest) + .ConfigureAwait(false); + return await BuildResponse(responseMessage).ConfigureAwait(false); } + } - var http = new HttpClient(httpOptions); + [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] + static CancellationToken GetCancellationTokenForRequest(IRequest request, CancellationToken cancellationToken) + { var cancellationTokenForRequest = cancellationToken; if (request.Timeout != TimeSpan.Zero) @@ -60,14 +61,7 @@ public async Task Send(IRequest request, CancellationToken cancellati cancellationTokenForRequest = unifiedCancellationToken.Token; } - - using (var requestMessage = BuildRequestMessage(request)) - { - // Make the request - var responseMessage = await http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationTokenForRequest) - .ConfigureAwait(false); - return await BuildResponse(responseMessage).ConfigureAwait(false); - } + return cancellationTokenForRequest; } protected async virtual Task BuildResponse(HttpResponseMessage responseMessage) @@ -83,7 +77,9 @@ protected async virtual Task BuildResponse(HttpResponseMessage respon contentType = GetContentMediaType(responseMessage.Content); // We added support for downloading images and zip-files. Let's constrain this appropriately. - if (contentType != null && (contentType.StartsWith("image/") || contentType.Equals("application/zip", StringComparison.OrdinalIgnoreCase))) + if (contentType != null && (contentType.StartsWith("image/") + || contentType.Equals("application/zip", StringComparison.OrdinalIgnoreCase) + || contentType.Equals("application/x-gzip", StringComparison.OrdinalIgnoreCase))) { responseBody = await responseMessage.Content.ReadAsByteArrayAsync().ConfigureAwait(false); } @@ -109,6 +105,7 @@ protected virtual HttpRequestMessage BuildRequestMessage(IRequest request) { var fullUri = new Uri(request.BaseAddress, request.Endpoint); requestMessage = new HttpRequestMessage(request.Method, fullUri); + foreach (var header in request.Headers) { requestMessage.Headers.Add(header.Key, header.Value); @@ -152,5 +149,101 @@ static string GetContentMediaType(HttpContent httpContent) } return null; } + + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + if (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) + { + var response = await base.SendAsync(request, cancellationToken); + + // Can't redirect without somewhere to redirect too. Throw? + if (response.Headers.Location == null) return response; + + // Don't redirect if we exceed max number of redirects + var redirectCount = 0; + if (request.Properties.Keys.Contains(RedirectCountKey)) + { + redirectCount = (int)request.Properties[RedirectCountKey]; + } + if (redirectCount > 3) + { + 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 + || response.StatusCode == HttpStatusCode.Found + || response.StatusCode == HttpStatusCode.SeeOther + || 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; + } + else + { + if (request.Content != null && request.Content.Headers.ContentLength != 0) { + var stream = await request.Content.ReadAsStreamAsync(); + 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) + { + newRequest.Headers.Authorization = null; + } + response = await this.SendAsync(newRequest, cancellationToken); + } + + return response; + } + + [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] + private static HttpRequestMessage CopyRequest(HttpRequestMessage oldRequest) + { + var newrequest = new HttpRequestMessage(oldRequest.Method, oldRequest.RequestUri); + + foreach (var header in oldRequest.Headers) + { + newrequest.Headers.TryAddWithoutValidation(header.Key, header.Value); + } + foreach (var property in oldRequest.Properties) + { + newrequest.Properties.Add(property); + } + + return newrequest; + } } } diff --git a/Octokit/Http/HttpMessageHandlerFactory.cs b/Octokit/Http/HttpMessageHandlerFactory.cs new file mode 100644 index 0000000000..5b9321c812 --- /dev/null +++ b/Octokit/Http/HttpMessageHandlerFactory.cs @@ -0,0 +1,36 @@ +using System.Diagnostics.CodeAnalysis; +using System.Net; +using System.Net.Http; + +namespace Octokit.Internal +{ + public static class HttpMessageHandlerFactory + { + public static HttpMessageHandler CreateDefault() + { + return CreateDefault(null); + } + + [SuppressMessage("Microsoft.Usage", "CA1801:ReviewUnusedParameters", MessageId = "proxy")] + [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] + public static HttpMessageHandler CreateDefault(IWebProxy proxy) + { + var handler = new HttpClientHandler + { + AllowAutoRedirect = false + }; +#if !PORTABLE + if (handler.SupportsAutomaticDecompression) + { + handler.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate; + } + if (handler.SupportsProxy && proxy != null) + { + handler.UseProxy = true; + handler.Proxy = proxy; + } +#endif + return handler; + } + } +} diff --git a/Octokit/Http/IConnection.cs b/Octokit/Http/IConnection.cs index 00b9c3be5e..7deec1ccb8 100644 --- a/Octokit/Http/IConnection.cs +++ b/Octokit/Http/IConnection.cs @@ -58,6 +58,17 @@ public interface IConnection [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get")] Task> Get(Uri uri, IDictionary parameters, string accepts, CancellationToken cancellationToken); + /// + /// Performs an asynchronous HTTP GET request. + /// Attempts to map the response to an object of type + /// + /// The type to map the response to + /// URI endpoint to send request to + /// Expiration time of the request + /// representing the received HTTP response + [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1716:IdentifiersShouldNotMatchKeywords", MessageId = "Get")] + Task> Get(Uri uri, TimeSpan timeout); + /// /// Performs an asynchronous HTTP PATCH request. /// diff --git a/Octokit/Http/IHttpClient.cs b/Octokit/Http/IHttpClient.cs index 0818c525ab..f554d750dc 100644 --- a/Octokit/Http/IHttpClient.cs +++ b/Octokit/Http/IHttpClient.cs @@ -1,4 +1,5 @@ -using System.Threading; +using System; +using System.Threading; using System.Threading.Tasks; namespace Octokit.Internal @@ -9,7 +10,7 @@ namespace Octokit.Internal /// /// Most folks won't ever need to swap this out. But if you're trying to run this on Windows Phone, you might. /// - public interface IHttpClient + public interface IHttpClient : IDisposable { /// /// Sends the specified request and returns a response. diff --git a/Octokit/Http/IRequest.cs b/Octokit/Http/IRequest.cs index f52b2e52b0..31fbc578fb 100644 --- a/Octokit/Http/IRequest.cs +++ b/Octokit/Http/IRequest.cs @@ -14,6 +14,7 @@ public interface IRequest Uri Endpoint { get; } TimeSpan Timeout { get; } string ContentType { get; } + [Obsolete("This value is no longer respected due to the necessary redirect work")] bool AllowAutoRedirect { get; } } } diff --git a/Octokit/Http/JsonHttpPipeline.cs b/Octokit/Http/JsonHttpPipeline.cs index 66e01c07f7..864136e4af 100644 --- a/Octokit/Http/JsonHttpPipeline.cs +++ b/Octokit/Http/JsonHttpPipeline.cs @@ -11,7 +11,7 @@ namespace Octokit.Internal /// public class JsonHttpPipeline { - private const string v3ApiVersion = "application/vnd.github.v3+json; charset=utf-8"; + private const string v3ApiVersion = "application/vnd.github.quicksilver-preview+json; charset=utf-8, application/vnd.github.v3+json; charset=utf-8"; readonly IJsonSerializer _serializer; @@ -32,7 +32,6 @@ public void SerializeRequest(IRequest request) if (!request.Headers.ContainsKey("Accept")) { - request.Headers["Accept"] = v3ApiVersion; } diff --git a/Octokit/Http/Request.cs b/Octokit/Http/Request.cs index cd6dad4d31..f9300fd49b 100644 --- a/Octokit/Http/Request.cs +++ b/Octokit/Http/Request.cs @@ -11,7 +11,6 @@ public Request() { Headers = new Dictionary(); Parameters = new Dictionary(); - AllowAutoRedirect = true; Timeout = TimeSpan.FromSeconds(100); } @@ -23,6 +22,8 @@ public Request() public Uri Endpoint { get; set; } public TimeSpan Timeout { get; set; } public string ContentType { get; set; } + + [Obsolete("This value is no longer respected due to the necessary redirect work")] public bool AllowAutoRedirect { get; set; } } } diff --git a/Octokit/Octokit-Mono.csproj b/Octokit/Octokit-Mono.csproj index 99360bc04e..4bd0ade65d 100644 --- a/Octokit/Octokit-Mono.csproj +++ b/Octokit/Octokit-Mono.csproj @@ -393,6 +393,7 @@ + diff --git a/Octokit/Octokit-MonoAndroid.csproj b/Octokit/Octokit-MonoAndroid.csproj index 40389d2fc5..9110123b2f 100644 --- a/Octokit/Octokit-MonoAndroid.csproj +++ b/Octokit/Octokit-MonoAndroid.csproj @@ -1,4 +1,4 @@ - + Debug @@ -409,6 +409,7 @@ + diff --git a/Octokit/Octokit-Monotouch.csproj b/Octokit/Octokit-Monotouch.csproj index 558570e9e2..8a98670da8 100644 --- a/Octokit/Octokit-Monotouch.csproj +++ b/Octokit/Octokit-Monotouch.csproj @@ -1,4 +1,4 @@ - + Debug @@ -402,6 +402,7 @@ + diff --git a/Octokit/Octokit-Portable.csproj b/Octokit/Octokit-Portable.csproj index 32fcf52402..4e3d1d8ebf 100644 --- a/Octokit/Octokit-Portable.csproj +++ b/Octokit/Octokit-Portable.csproj @@ -391,6 +391,7 @@ + diff --git a/Octokit/Octokit-netcore45.csproj b/Octokit/Octokit-netcore45.csproj index b0896731ae..d40ee8b895 100644 --- a/Octokit/Octokit-netcore45.csproj +++ b/Octokit/Octokit-netcore45.csproj @@ -395,6 +395,7 @@ + diff --git a/Octokit/Octokit.csproj b/Octokit/Octokit.csproj index c880cc1ad5..ca6ab92bee 100644 --- a/Octokit/Octokit.csproj +++ b/Octokit/Octokit.csproj @@ -81,6 +81,7 @@ + diff --git a/docs/http-client.md b/docs/http-client.md new file mode 100644 index 0000000000..265ec7c43c --- /dev/null +++ b/docs/http-client.md @@ -0,0 +1,53 @@ +## `HttpClient` and Octokit + +If you want to understand more of the internals of how Octokit is using +`HttpClient`, then read on. Otherwise, you should be fine with using the +defaults. + +### Redirects + +A recent feature of the GitHub API is to handle redirects automatically. +This means the client needs to handle 30* responses and redirect correctly. + +Due to the default behaviour of `HttpWebRequest`, we need to implement this +as a custom handler in `HttpClient`. + +The rules we currently follow: + +``` + - MUST follow redirect requests if HTTP status code is 301, 302, or 307. + - MUST redirect a 30x status code if the HTTP method is HEAD, OPTIONS, GET, POST, PUT, PATCH, or DELETE. + - MUST use the original request's HTTP method when following a redirect where the HTTP status code is 307. + - SHOULD use the original request's HTTP method when following a redirect where the HTTP status code is 301 or 302. + - MUST use the original request's authentication credentials when following a redirect where the original host matches the redirect host. + - MUST NOT use the original request's authentication credentials when following a redirect where the original host does not match the redirect host. + - SHOULD only follow 3 redirects. + ``` + +### Proxy Support + +There's some basic support for specify proxy credentials, but this is a work +in progress as there's a bunch of internals that need to be refactored +thoughtfully. + +Here's how you can do it today: + +```csharp +// set your proxy details here +var proxy = new WebProxy(); + +// this is the core connection +var connection = new Connection(new ProductHeaderValue("my-cool-app"), + new HttpClientAdapter(() => HttpMessageHandlerFactory.CreateDefault(proxy))); + +// and pass this connection to your client +var client = new GitHubClient(connection); +``` + +And if you're looking for GitHub Enterprise support with a custom proxy, it's +currently broken. I've opened an issue to track this, so please chime in if it's +something you need to support: https://github.com/octokit/octokit.net/issues/809 + +Another request has been for auto-wiring the proxy details at runtime. Please +leave a comment in this thread if you're interested in this: +https://github.com/octokit/octokit.net/issues/594 \ No newline at end of file