From 96d97f873abce0d2d66139990db5386cf6a116a8 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Tue, 1 Aug 2017 19:31:10 +0200 Subject: [PATCH] Fixed: http->https redirects do not use the tls1.2 curl fallback. fixes #2082 --- .../Http/HttpClientFixture.cs | 31 +++++++- .../Http/Dispatchers/CurlHttpDispatcher.cs | 4 +- .../Http/Dispatchers/ManagedHttpDispatcher.cs | 2 +- src/NzbDrone.Common/Http/HttpClient.cs | 76 +++++++++++++------ 4 files changed, 85 insertions(+), 28 deletions(-) diff --git a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs b/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs index 823e5cdd9..b903e85b4 100644 --- a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs +++ b/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs @@ -61,7 +61,7 @@ namespace NzbDrone.Common.Test.Http response.Content.Should().NotBeNullOrWhiteSpace(); } - + [Test] public void should_execute_https_get() { @@ -144,6 +144,33 @@ namespace NzbDrone.Common.Test.Http ExceptionVerification.ExpectedErrors(0); } + [Test] + public void should_follow_redirects_to_https() + { + var request = new HttpRequestBuilder($"http://{_httpBinHost}/redirect-to") + .AddQueryParam("url", $"https://sonarr.tv/") + .Build(); + request.AllowAutoRedirect = true; + + var response = Subject.Get(request); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + response.Content.Should().Contain("Sonarr"); + + ExceptionVerification.ExpectedErrors(0); + } + + [Test] + public void should_throw_on_too_many_redirects() + { + var request = new HttpRequest($"http://{_httpBinHost}/redirect/4"); + request.AllowAutoRedirect = true; + + Assert.Throws(() => Subject.Get(request)); + + ExceptionVerification.ExpectedErrors(0); + } + [Test] public void should_send_user_agent() { @@ -407,4 +434,4 @@ namespace NzbDrone.Common.Test.Http public string Url { get; set; } public string Data { get; set; } } -} \ No newline at end of file +} diff --git a/src/NzbDrone.Common/Http/Dispatchers/CurlHttpDispatcher.cs b/src/NzbDrone.Common/Http/Dispatchers/CurlHttpDispatcher.cs index 83d6fb1d1..17574982d 100644 --- a/src/NzbDrone.Common/Http/Dispatchers/CurlHttpDispatcher.cs +++ b/src/NzbDrone.Common/Http/Dispatchers/CurlHttpDispatcher.cs @@ -38,7 +38,7 @@ namespace NzbDrone.Common.Http.Dispatchers _caBundleFilePath = _caBundleFileName; } } - + public CurlHttpDispatcher(IHttpProxySettingsProvider proxySettingsProvider, IUserAgentBuilder userAgentBuilder, Logger logger) { _proxySettingsProvider = proxySettingsProvider; @@ -107,7 +107,7 @@ namespace NzbDrone.Common.Http.Dispatchers throw new NotSupportedException($"HttpCurl method {request.Method} not supported"); } curlEasy.UserAgent = _userAgentBuilder.GetUserAgent(request.UseSimplifiedUserAgent); - curlEasy.FollowLocation = request.AllowAutoRedirect; + curlEasy.FollowLocation = false; if (request.RequestTimeout != TimeSpan.Zero) { diff --git a/src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs b/src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs index 27f6e30c9..e33a7edd7 100644 --- a/src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs +++ b/src/NzbDrone.Common/Http/Dispatchers/ManagedHttpDispatcher.cs @@ -32,7 +32,7 @@ namespace NzbDrone.Common.Http.Dispatchers webRequest.Method = request.Method.ToString(); webRequest.UserAgent = _userAgentBuilder.GetUserAgent(request.UseSimplifiedUserAgent); webRequest.KeepAlive = request.ConnectionKeepAlive; - webRequest.AllowAutoRedirect = request.AllowAutoRedirect; + webRequest.AllowAutoRedirect = false; webRequest.CookieContainer = cookies; if (request.RequestTimeout != TimeSpan.Zero) diff --git a/src/NzbDrone.Common/Http/HttpClient.cs b/src/NzbDrone.Common/Http/HttpClient.cs index 849647f64..5b499c0e0 100644 --- a/src/NzbDrone.Common/Http/HttpClient.cs +++ b/src/NzbDrone.Common/Http/HttpClient.cs @@ -7,6 +7,7 @@ using System.Net; using NLog; using NzbDrone.Common.Cache; using NzbDrone.Common.EnvironmentInfo; +using NzbDrone.Common.Extensions; using NzbDrone.Common.Http.Dispatchers; using NzbDrone.Common.TPL; @@ -50,6 +51,57 @@ namespace NzbDrone.Common.Http } public HttpResponse Execute(HttpRequest request) + { + var autoRedirectCount = 0; + var autoRedirectChain = new List(); + autoRedirectChain.Add(request.Url.ToString()); + + var response = ExecuteRequest(request); + + while (response.StatusCode == HttpStatusCode.Moved || + response.StatusCode == HttpStatusCode.MovedPermanently || + response.StatusCode == HttpStatusCode.Found) + { + if (request.AllowAutoRedirect) + { + request.Url += new HttpUri(response.Headers.GetSingleValue("Location")); + autoRedirectChain.Add(request.Url.ToString()); + + _logger.Trace("Redirected to {0}", request.Url); + + autoRedirectCount++; + if (autoRedirectCount > 3) + { + throw new WebException($"Too many automatic redirections were attempted for {autoRedirectChain.Join(" -> ")}", WebExceptionStatus.ProtocolError); + } + + response = ExecuteRequest(request); + } + else if (!RuntimeInfo.IsProduction) + { + _logger.Error("Server requested a redirect to [{0}]. Update the request URL to avoid this redirect.", response.Headers["Location"]); + break; + } + } + + if (!request.SuppressHttpError && response.HasHttpError) + { + _logger.Warn("HTTP Error - {0}", response); + + if ((int)response.StatusCode == 429) + { + throw new TooManyRequestsException(request, response); + } + else + { + throw new HttpException(request, response); + } + } + + return response; + } + + private HttpResponse ExecuteRequest(HttpRequest request) { foreach (var interceptor in _requestInterceptors) { @@ -85,28 +137,6 @@ namespace NzbDrone.Common.Http _logger.Trace("Response content ({0} bytes): {1}", response.ResponseData.Length, response.Content); } - if (!RuntimeInfo.IsProduction && - (response.StatusCode == HttpStatusCode.Moved || - response.StatusCode == HttpStatusCode.MovedPermanently || - response.StatusCode == HttpStatusCode.Found)) - { - _logger.Error("Server requested a redirect to [{0}]. Update the request URL to avoid this redirect.", response.Headers["Location"]); - } - - if (!request.SuppressHttpError && response.HasHttpError) - { - _logger.Warn("HTTP Error - {0}", response); - - if ((int)response.StatusCode == 429) - { - throw new TooManyRequestsException(request, response); - } - else - { - throw new HttpException(request, response); - } - } - return response; } @@ -217,4 +247,4 @@ namespace NzbDrone.Common.Http return new HttpResponse(response); } } -} \ No newline at end of file +}