Fixed: Changes in http redirect logic causing failed grabs and >25% cpu usage.

This commit is contained in:
Taloth Saldono 2017-08-09 23:03:31 +02:00
parent 7e5e136930
commit 2d1d1c8a99
3 changed files with 34 additions and 18 deletions

View File

@ -139,11 +139,26 @@ namespace NzbDrone.Common.Test.Http
var request = new HttpRequest($"http://{_httpBinHost}/redirect/1"); var request = new HttpRequest($"http://{_httpBinHost}/redirect/1");
request.AllowAutoRedirect = true; request.AllowAutoRedirect = true;
Subject.Get(request); var response = Subject.Get(request);
response.StatusCode.Should().Be(HttpStatusCode.OK);
ExceptionVerification.ExpectedErrors(0); ExceptionVerification.ExpectedErrors(0);
} }
[Test]
public void should_not_follow_redirects()
{
var request = new HttpRequest($"http://{_httpBinHost}/redirect/1");
request.AllowAutoRedirect = false;
var response = Subject.Get(request);
response.StatusCode.Should().Be(HttpStatusCode.Found);
ExceptionVerification.ExpectedErrors(1);
}
[Test] [Test]
public void should_follow_redirects_to_https() public void should_follow_redirects_to_https()
{ {

View File

@ -52,36 +52,33 @@ namespace NzbDrone.Common.Http
public HttpResponse Execute(HttpRequest request) public HttpResponse Execute(HttpRequest request)
{ {
var autoRedirectCount = 0;
var autoRedirectChain = new List<string>();
autoRedirectChain.Add(request.Url.ToString());
var response = ExecuteRequest(request); var response = ExecuteRequest(request);
while (response.StatusCode == HttpStatusCode.Moved || if (request.AllowAutoRedirect && response.HasHttpRedirect)
response.StatusCode == HttpStatusCode.MovedPermanently ||
response.StatusCode == HttpStatusCode.Found)
{ {
if (request.AllowAutoRedirect) var autoRedirectChain = new List<string>();
autoRedirectChain.Add(request.Url.ToString());
do
{ {
request.Url += new HttpUri(response.Headers.GetSingleValue("Location")); request.Url += new HttpUri(response.Headers.GetSingleValue("Location"));
autoRedirectChain.Add(request.Url.ToString()); autoRedirectChain.Add(request.Url.ToString());
_logger.Trace("Redirected to {0}", request.Url); _logger.Trace("Redirected to {0}", request.Url);
autoRedirectCount++; if (autoRedirectChain.Count > 3)
if (autoRedirectCount > 3)
{ {
throw new WebException($"Too many automatic redirections were attempted for {autoRedirectChain.Join(" -> ")}", WebExceptionStatus.ProtocolError); throw new WebException($"Too many automatic redirections were attempted for {autoRedirectChain.Join(" -> ")}", WebExceptionStatus.ProtocolError);
} }
response = ExecuteRequest(request); response = ExecuteRequest(request);
} }
else if (!RuntimeInfo.IsProduction) while (response.HasHttpRedirect);
{ }
_logger.Error("Server requested a redirect to [{0}]. Update the request URL to avoid this redirect.", response.Headers["Location"]);
break; if (response.HasHttpRedirect && !RuntimeInfo.IsProduction)
} {
_logger.Error("Server requested a redirect to [{0}] while in developer mode. Update the request URL to avoid this redirect.", response.Headers["Location"]);
} }
if (!request.SuppressHttpError && response.HasHttpError) if (!request.SuppressHttpError && response.HasHttpError)

View File

@ -35,7 +35,7 @@ namespace NzbDrone.Common.Http
private string _content; private string _content;
public string Content public string Content
{ {
get get
{ {
@ -51,6 +51,10 @@ namespace NzbDrone.Common.Http
public bool HasHttpError => (int)StatusCode >= 400; public bool HasHttpError => (int)StatusCode >= 400;
public bool HasHttpRedirect => StatusCode == HttpStatusCode.Moved ||
StatusCode == HttpStatusCode.MovedPermanently ||
StatusCode == HttpStatusCode.Found;
public Dictionary<string, string> GetCookies() public Dictionary<string, string> GetCookies()
{ {
var result = new Dictionary<string, string>(); var result = new Dictionary<string, string>();
@ -95,4 +99,4 @@ namespace NzbDrone.Common.Http
public T Resource { get; private set; } public T Resource { get; private set; }
} }
} }