From 4be0fe1b76f12cfbda61dc5e0ad5308636f25a77 Mon Sep 17 00:00:00 2001 From: ta264 Date: Sat, 1 Aug 2015 13:54:20 +0100 Subject: [PATCH] Add tests for CurlHttpClient and fix the failures --- build.ps1 | 3 + build.sh | 3 + integration_mono.sh | 1 + .../Http/HttpClientFixture.cs | 57 ++++- .../NzbDrone.Common.Test.csproj | 7 +- src/NzbDrone.Common/Http/CurlHttpClient.cs | 28 ++- src/NzbDrone.Common/Http/HttpClient.cs | 219 ++++++++++-------- src/NzbDrone.Common/Http/HttpHeader.cs | 4 +- 8 files changed, 222 insertions(+), 100 deletions(-) diff --git a/build.ps1 b/build.ps1 index e699a4646..e6146c6c8 100644 --- a/build.ps1 +++ b/build.ps1 @@ -218,6 +218,9 @@ Function PackageTests() Write-Host "Adding NzbDrone.Core.dll.config (for dllmap)" Copy-Item "$sourceFolder\NzbDrone.Core\NzbDrone.Core.dll.config" -Destination $testPackageFolder -Force + Write-Host "Copying CurlSharp libraries" + Copy-Item $sourceFolder\ExternalModules\CurlSharp\libs\i386\* $testPackageFolder + Write-Host "##teamcity[progressFinish 'Creating Test Package']" } diff --git a/build.sh b/build.sh index 4eb966f81..11963146d 100755 --- a/build.sh +++ b/build.sh @@ -224,6 +224,9 @@ PackageTests() echo "Adding CurlSharp.dll.config (for dllmap)" cp $sourceFolder/NzbDrone.Common/CurlSharp.dll.config $testPackageFolder + echo "Copying CurlSharp libraries" + cp $sourceFolder/ExternalModules/CurlSharp/libs/i386/* $testPackageFolder + echo "##teamcity[progressFinish 'Creating Test Package']" } diff --git a/integration_mono.sh b/integration_mono.sh index f7c8ef40a..1df48b91f 100644 --- a/integration_mono.sh +++ b/integration_mono.sh @@ -5,3 +5,4 @@ NUNIT="$TESTDIR/NUnit.Runners.2.6.1/tools/nunit-console-x86.exe" mono --debug --runtime=v4.0 $NUNIT $EXCLUDE -xml:NzbDrone.Api.Result.xml $TESTDIR/NzbDrone.Api.Test.dll mono --debug --runtime=v4.0 $NUNIT $EXCLUDE -xml:NzbDrone.Core.Result.xml $TESTDIR/NzbDrone.Core.Test.dll mono --debug --runtime=v4.0 $NUNIT $EXCLUDE -xml:NzbDrone.Integration.Result.xml $TESTDIR/NzbDrone.Integration.Test.dll +mono --debug --runtime=v4.0 $NUNIT $EXCLUDE -xml:NzbDrone.Common.Result.xml $TESTDIR/NzbDrone.Common.Test.dll diff --git a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs b/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs index 517392055..a1eb10fda 100644 --- a/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs +++ b/src/NzbDrone.Common.Test/Http/HttpClientFixture.cs @@ -14,16 +14,33 @@ namespace NzbDrone.Common.Test.Http { - [TestFixture] + [TestFixture(true)] + [TestFixture(false)] [IntegrationTest] public class HttpClientFixture : TestBase { + private bool _forceCurl; + + public HttpClientFixture(bool forceCurl) + { + _forceCurl = forceCurl; + } + [SetUp] public void SetUp() { Mocker.SetConstant(Mocker.Resolve()); Mocker.SetConstant(Mocker.Resolve()); Mocker.SetConstant>(new IHttpRequestInterceptor[0]); + + if (_forceCurl) + { + Mocker.SetConstant(Mocker.Resolve()); + } + else + { + Mocker.SetConstant(Mocker.Resolve()); + } } [Test] @@ -35,6 +52,16 @@ public void should_execute_simple_get() response.Content.Should().NotBeNullOrWhiteSpace(); } + + [Test] + public void should_execute_https_get() + { + var request = new HttpRequest("https://eu.httpbin.org/get"); + + var response = Subject.Execute(request); + + response.Content.Should().NotBeNullOrWhiteSpace(); + } [Test] public void should_execute_typed_get() @@ -163,7 +190,7 @@ public void GivenOldCookie() var oldRequest = new HttpRequest("http://eu.httpbin.org/get"); oldRequest.AddCookie("my", "cookie"); - var oldClient = new HttpClient(new IHttpRequestInterceptor[0], Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve()); + var oldClient = new HttpClient(new IHttpRequestInterceptor[0], Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve(), Mocker.Resolve()); oldClient.Should().NotBeSameAs(Subject); @@ -295,6 +322,32 @@ public void should_call_interceptor() Mocker.GetMock() .Verify(v => v.PostResponse(It.IsAny()), Times.Once()); } + + public void should_parse_malformed_cloudflare_cookie() + { + // the date is bad in the below - should be 13-Jul-2016 + string malformedCookie = @"__cfduid=d29e686a9d65800021c66faca0a29b4261436890790; expires=Wed, 13-Jul-16 16:19:50 GMT; path=/; HttpOnly"; + string url = "http://eu.httpbin.org/response-headers?Set-Cookie=" + + System.Uri.EscapeUriString(malformedCookie); + + var requestSet = new HttpRequest(url); + requestSet.AllowAutoRedirect = false; + requestSet.StoreResponseCookie = true; + + var responseSet = Subject.Get(requestSet); + + var request = new HttpRequest("http://eu.httpbin.org/get"); + + var response = Subject.Get(request); + + response.Resource.Headers.Should().ContainKey("Cookie"); + + var cookie = response.Resource.Headers["Cookie"].ToString(); + + cookie.Should().Contain("__cfduid=d29e686a9d65800021c66faca0a29b4261436890790"); + + ExceptionVerification.IgnoreErrors(); + } } public class HttpBinResource diff --git a/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj b/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj index 36bcbe6b7..f267befcc 100644 --- a/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj +++ b/src/NzbDrone.Common.Test/NzbDrone.Common.Test.csproj @@ -1,4 +1,4 @@ - + Debug @@ -142,6 +142,9 @@ + + xcopy /s /y "$(SolutionDir)\ExternalModules\CurlSharp\libs\i386\*" "$(TargetDir)" + - \ No newline at end of file + diff --git a/src/NzbDrone.Common/Http/CurlHttpClient.cs b/src/NzbDrone.Common/Http/CurlHttpClient.cs index 68a2e9804..c7af89cac 100644 --- a/src/NzbDrone.Common/Http/CurlHttpClient.cs +++ b/src/NzbDrone.Common/Http/CurlHttpClient.cs @@ -6,8 +6,10 @@ using System.Net; using System.Runtime.InteropServices; using System.Text; +using System.Text.RegularExpressions; using CurlSharp; using NLog; +using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Common.Extensions; using NzbDrone.Common.Instrumentation; @@ -16,6 +18,7 @@ namespace NzbDrone.Common.Http public class CurlHttpClient { private static Logger Logger = NzbDroneLogger.GetLogger(typeof(CurlHttpClient)); + private static readonly Regex ExpiryDate = new Regex(@"(expires=)([^;]+)", RegexOptions.IgnoreCase | RegexOptions.Compiled); public CurlHttpClient() { @@ -64,7 +67,12 @@ public HttpResponse GetResponse(HttpRequest httpRequest, HttpWebRequest webReque curlEasy.HttpGet = webRequest.Method == "GET"; curlEasy.Post = webRequest.Method == "POST"; curlEasy.Put = webRequest.Method == "PUT"; - curlEasy.Url = webRequest.RequestUri.ToString(); + curlEasy.Url = webRequest.RequestUri.AbsoluteUri; + + if (OsInfo.IsWindows) + { + curlEasy.CaInfo = "curl-ca-bundle.crt"; + } if (webRequest.CookieContainer != null) { @@ -152,20 +160,34 @@ private WebHeaderCollection ProcessHeaderStream(HttpWebRequest webRequest, Strea var webHeaderCollection = new WebHeaderCollection(); - foreach (var header in headerString.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).Skip(1)) + // following a redirect we could have two sets of headers, so only process the last one + foreach (var header in headerString.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).Reverse()) { + if (!header.Contains(":")) break; webHeaderCollection.Add(header); } var setCookie = webHeaderCollection.Get("Set-Cookie"); if (setCookie != null && setCookie.Length > 0 && webRequest.CookieContainer != null) { - webRequest.CookieContainer.SetCookies(webRequest.RequestUri, setCookie); + webRequest.CookieContainer.SetCookies(webRequest.RequestUri, FixSetCookieHeader(setCookie)); } return webHeaderCollection; } + private string FixSetCookieHeader(string setCookie) + { + // fix up the date if it was malformed + var setCookieClean = ExpiryDate.Replace(setCookie, delegate(Match match) + { + string format = "ddd, dd-MMM-yyyy HH:mm:ss"; + DateTime dt = Convert.ToDateTime(match.Groups[2].Value); + return match.Groups[1].Value + dt.ToUniversalTime().ToString(format) + " GMT"; + }); + return setCookieClean; + } + private byte[] ProcessResponseStream(HttpWebRequest webRequest, Stream responseStream, WebHeaderCollection webHeaderCollection) { responseStream.Position = 0; diff --git a/src/NzbDrone.Common/Http/HttpClient.cs b/src/NzbDrone.Common/Http/HttpClient.cs index 4750a839e..bb6ba5313 100644 --- a/src/NzbDrone.Common/Http/HttpClient.cs +++ b/src/NzbDrone.Common/Http/HttpClient.cs @@ -23,6 +23,119 @@ public interface IHttpClient HttpResponse Post(HttpRequest request) where T : new(); } + public interface IHttpDispatcher + { + HttpResponse GetResponse(HttpRequest request, HttpWebRequest webRequest); + } + + public class ManagedHttpDispatcher : IHttpDispatcher + { + public HttpResponse GetResponse(HttpRequest request, HttpWebRequest webRequest) + { + if (!request.Body.IsNullOrWhiteSpace()) + { + var bytes = request.Headers.GetEncodingFromContentType().GetBytes(request.Body.ToCharArray()); + + webRequest.ContentLength = bytes.Length; + using (var writeStream = webRequest.GetRequestStream()) + { + writeStream.Write(bytes, 0, bytes.Length); + } + } + + HttpWebResponse httpWebResponse; + + try + { + httpWebResponse = (HttpWebResponse)webRequest.GetResponse(); + } + catch (WebException e) + { + httpWebResponse = (HttpWebResponse)e.Response; + + if (httpWebResponse == null) + { + throw; + } + } + + Byte[] data = null; + + using (var responseStream = httpWebResponse.GetResponseStream()) + { + if (responseStream != null) + { + data = responseStream.ToBytes(); + } + } + + return new HttpResponse(request, new HttpHeader(httpWebResponse.Headers), data, httpWebResponse.StatusCode); + + } + } + + public class CurlHttpDispatcher : IHttpDispatcher + { + public HttpResponse GetResponse(HttpRequest request, HttpWebRequest webRequest) + { + var curlClient = new CurlHttpClient(); + + return curlClient.GetResponse(request, webRequest); + } + } + + public class FallbackHttpDispatcher : IHttpDispatcher + { + private readonly Logger _logger; + private readonly ICached _curlTLSFallbackCache; + + public FallbackHttpDispatcher(ICached curlTLSFallbackCache, Logger logger) + { + _logger = logger; + _curlTLSFallbackCache = curlTLSFallbackCache; + } + + public HttpResponse GetResponse(HttpRequest request, HttpWebRequest webRequest) + { + + ManagedHttpDispatcher managedDispatcher = new ManagedHttpDispatcher(); + CurlHttpDispatcher curlDispatcher = new CurlHttpDispatcher(); + + if (OsInfo.IsMonoRuntime && webRequest.RequestUri.Scheme == "https") + { + if (!_curlTLSFallbackCache.Find(webRequest.RequestUri.Host)) + { + try + { + return managedDispatcher.GetResponse(request, webRequest); + } + catch (Exception ex) + { + if (ex.ToString().Contains("The authentication or decryption has failed.")) + { + _logger.Debug("https request failed in tls error for {0}, trying curl fallback.", webRequest.RequestUri.Host); + + _curlTLSFallbackCache.Set(webRequest.RequestUri.Host, true); + } + else + { + throw; + } + } + } + + if (CurlHttpClient.CheckAvailability()) + { + return curlDispatcher.GetResponse(request, webRequest); + } + + _logger.Trace("Curl not available, using default WebClient."); + } + + return managedDispatcher.GetResponse(request, webRequest); + } + } + public class HttpClient : IHttpClient { private readonly Logger _logger; @@ -30,16 +143,23 @@ public class HttpClient : IHttpClient private readonly ICached _cookieContainerCache; private readonly ICached _curlTLSFallbackCache; private readonly List _requestInterceptors; + private readonly IHttpDispatcher _httpDispatcher; - public HttpClient(IEnumerable requestInterceptors, ICacheManager cacheManager, IRateLimitService rateLimitService, Logger logger) + public HttpClient(IEnumerable requestInterceptors, ICacheManager cacheManager, IRateLimitService rateLimitService, IHttpDispatcher httpDispatcher, Logger logger) { _logger = logger; _rateLimitService = rateLimitService; _requestInterceptors = requestInterceptors.ToList(); ServicePointManager.DefaultConnectionLimit = 12; + _httpDispatcher = httpDispatcher; _cookieContainerCache = cacheManager.GetCache(typeof(HttpClient)); - _curlTLSFallbackCache = cacheManager.GetCache(typeof(HttpClient), "curlTLSFallback"); + } + + public HttpClient(IEnumerable requestInterceptors, ICacheManager cacheManager, IRateLimitService rateLimitService, Logger logger) + : this(requestInterceptors, cacheManager, rateLimitService, null, logger) + { + _httpDispatcher = new FallbackHttpDispatcher(cacheManager.GetCache(typeof(HttpClient), "curlTLSFallback"), _logger); } public HttpResponse Execute(HttpRequest request) @@ -79,7 +199,7 @@ public HttpResponse Execute(HttpRequest request) PrepareRequestCookies(request, webRequest); - var response = ExecuteRequest(request, webRequest); + var response = _httpDispatcher.GetResponse(request, webRequest); HandleResponseCookies(request, webRequest); @@ -89,8 +209,8 @@ public HttpResponse Execute(HttpRequest request) if (!RuntimeInfoBase.IsProduction && (response.StatusCode == HttpStatusCode.Moved || - response.StatusCode == HttpStatusCode.MovedPermanently || - response.StatusCode == HttpStatusCode.Found)) + response.StatusCode == HttpStatusCode.MovedPermanently || + response.StatusCode == HttpStatusCode.Found)) { _logger.Error("Server requested a redirect to [" + response.Headers["Location"] + "]. Update the request URL to avoid this redirect."); } @@ -129,7 +249,9 @@ private void PrepareRequestCookies(HttpRequest request, HttpWebRequest webReques { persistentCookieContainer.Add(new Cookie(pair.Key, pair.Value, "/", request.Url.Host) { - Expires = DateTime.UtcNow.AddHours(1) + // Use Now rather than UtcNow to work around Mono cookie expiry bug. + // See https://gist.github.com/ta264/7822b1424f72e5b4c961 + Expires = DateTime.Now.AddHours(1) }); } } @@ -167,91 +289,6 @@ private void HandleResponseCookies(HttpRequest request, HttpWebRequest webReques } } - private HttpResponse ExecuteRequest(HttpRequest request, HttpWebRequest webRequest) - { - if (OsInfo.IsMonoRuntime && webRequest.RequestUri.Scheme == "https") - { - if (!_curlTLSFallbackCache.Find(webRequest.RequestUri.Host)) - { - try - { - return ExecuteWebRequest(request, webRequest); - } - catch (Exception ex) - { - if (ex.ToString().Contains("The authentication or decryption has failed.")) - { - _logger.Debug("https request failed in tls error for {0}, trying curl fallback.", webRequest.RequestUri.Host); - - _curlTLSFallbackCache.Set(webRequest.RequestUri.Host, true); - } - else - { - throw; - } - } - } - - if (CurlHttpClient.CheckAvailability()) - { - return ExecuteCurlRequest(request, webRequest); - } - - _logger.Trace("Curl not available, using default WebClient."); - } - - return ExecuteWebRequest(request, webRequest); - } - - private HttpResponse ExecuteCurlRequest(HttpRequest request, HttpWebRequest webRequest) - { - var curlClient = new CurlHttpClient(); - - return curlClient.GetResponse(request, webRequest); - } - - private HttpResponse ExecuteWebRequest(HttpRequest request, HttpWebRequest webRequest) - { - if (!request.Body.IsNullOrWhiteSpace()) - { - var bytes = request.Headers.GetEncodingFromContentType().GetBytes(request.Body.ToCharArray()); - - webRequest.ContentLength = bytes.Length; - using (var writeStream = webRequest.GetRequestStream()) - { - writeStream.Write(bytes, 0, bytes.Length); - } - } - - HttpWebResponse httpWebResponse; - - try - { - httpWebResponse = (HttpWebResponse)webRequest.GetResponse(); - } - catch (WebException e) - { - httpWebResponse = (HttpWebResponse)e.Response; - - if (httpWebResponse == null) - { - throw; - } - } - - byte[] data = null; - - using (var responseStream = httpWebResponse.GetResponseStream()) - { - if (responseStream != null) - { - data = responseStream.ToBytes(); - } - } - - return new HttpResponse(request, new HttpHeader(httpWebResponse.Headers), data, httpWebResponse.StatusCode); - } - public void DownloadFile(string url, string fileName) { try diff --git a/src/NzbDrone.Common/Http/HttpHeader.cs b/src/NzbDrone.Common/Http/HttpHeader.cs index 1ca9ddcd8..36d10a996 100644 --- a/src/NzbDrone.Common/Http/HttpHeader.cs +++ b/src/NzbDrone.Common/Http/HttpHeader.cs @@ -9,7 +9,7 @@ namespace NzbDrone.Common.Http { public class HttpHeader : Dictionary { - public HttpHeader(NameValueCollection headers) + public HttpHeader(NameValueCollection headers) : base(StringComparer.OrdinalIgnoreCase) { foreach (var key in headers.AllKeys) { @@ -17,7 +17,7 @@ public HttpHeader(NameValueCollection headers) } } - public HttpHeader() + public HttpHeader() : base(StringComparer.OrdinalIgnoreCase) { }