From d16779c3c1743b0c0f14cbd59aadd0b97fc23aac Mon Sep 17 00:00:00 2001 From: martincostello Date: Sun, 11 Mar 2018 15:57:27 +0000 Subject: [PATCH 1/3] Fix default port handling Fix the default handling for ports, particularly when re-using a builder for multiple registrations, where the scheme is changed without explicitly changing the port. This was causing the default port for the first scheme to be carried through into registrations for subsequent schemes as creating a UriBuilder from another copies the Port value through, causing it to appear to have been explicitly set, rather than keeping the "use default for scheme" behaviour. --- .../HttpClientInterceptorOptions.cs | 5 ++++ .../HttpInterceptionResponse.cs | 2 ++ .../HttpRequestInterceptionBuilder.cs | 4 +++ .../Matching/RegistrationMatcher.cs | 5 ++++ .../HttpRequestInterceptionBuilderTests.cs | 27 +++++++++++++++++++ 5 files changed, 43 insertions(+) diff --git a/src/HttpClientInterception/HttpClientInterceptorOptions.cs b/src/HttpClientInterception/HttpClientInterceptorOptions.cs index 628697f7..d2e9dc10 100644 --- a/src/HttpClientInterception/HttpClientInterceptorOptions.cs +++ b/src/HttpClientInterception/HttpClientInterceptorOptions.cs @@ -410,6 +410,11 @@ private static string BuildKey(HttpInterceptionResponse interceptor) keyPrefix += "IGNOREQUERY;"; } + if (!interceptor.HasCustomPort) + { + builderForKey.Port = -1; + } + return $"{keyPrefix};{interceptor.Method.Method}:{builderForKey}"; } diff --git a/src/HttpClientInterception/HttpInterceptionResponse.cs b/src/HttpClientInterception/HttpInterceptionResponse.cs index 70361d99..9721c327 100644 --- a/src/HttpClientInterception/HttpInterceptionResponse.cs +++ b/src/HttpClientInterception/HttpInterceptionResponse.cs @@ -34,6 +34,8 @@ internal sealed class HttpInterceptionResponse internal IEnumerable>> ContentHeaders { get; set; } + internal bool HasCustomPort { get; set; } + internal bool IgnoreHost { get; set; } internal bool IgnorePath { get; set; } diff --git a/src/HttpClientInterception/HttpRequestInterceptionBuilder.cs b/src/HttpClientInterception/HttpRequestInterceptionBuilder.cs index 9f90f2cd..713dfee0 100644 --- a/src/HttpClientInterception/HttpRequestInterceptionBuilder.cs +++ b/src/HttpClientInterception/HttpRequestInterceptionBuilder.cs @@ -43,6 +43,8 @@ public class HttpRequestInterceptionBuilder private Version _version; + private bool _hasCustomPort; + private bool _ignoreHost; private bool _ignorePath; @@ -135,6 +137,7 @@ public HttpRequestInterceptionBuilder ForHost(string host) public HttpRequestInterceptionBuilder ForPort(int port) { _uriBuilder.Port = port; + _hasCustomPort = port != -1; return this; } @@ -629,6 +632,7 @@ internal HttpInterceptionResponse Build() ContentFactory = _contentFactory ?? EmptyContentFactory, ContentStream = _contentStream, ContentMediaType = _mediaType, + HasCustomPort = _hasCustomPort, IgnoreHost = _ignoreHost, IgnorePath = _ignorePath, IgnoreQuery = _ignoreQuery, diff --git a/src/HttpClientInterception/Matching/RegistrationMatcher.cs b/src/HttpClientInterception/Matching/RegistrationMatcher.cs index fa13560b..ba52cf7a 100644 --- a/src/HttpClientInterception/Matching/RegistrationMatcher.cs +++ b/src/HttpClientInterception/Matching/RegistrationMatcher.cs @@ -65,6 +65,11 @@ private static string GetUriStringForMatch(HttpInterceptionResponse registration { var builder = new UriBuilder(uri ?? registration.RequestUri); + if (!registration.HasCustomPort) + { + builder.Port = -1; + } + if (registration.IgnoreHost) { builder.Host = "*"; diff --git a/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs b/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs index 8a789b53..43bed5f7 100644 --- a/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs +++ b/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs @@ -1040,6 +1040,33 @@ public static void RegisterWith_Validates_Parameters() Assert.Throws("options", () => builder.RegisterWith(null)); } + [Fact] + public static async Task Using_The_Same_Builder_For_Multiple_Registrations_On_The_Same_Host_Retains_Default_Port() + { + // Arrange + var options = new HttpClientInterceptorOptions() + { + ThrowOnMissingRegistration = true + }; + + var builder = new HttpRequestInterceptionBuilder() + .ForHttps() + .ForHost("api.github.com") + .ForPath("orgs/justeat") + .RegisterWith(options); + + // Change to a different scheme without an explicit port + builder = builder + .ForHttp() + .RegisterWith(options); + + // Act and Assert + await HttpAssert.GetAsync(options, "http://api.github.com/orgs/justeat"); + await HttpAssert.GetAsync(options, "http://api.github.com:80/orgs/justeat"); + await HttpAssert.GetAsync(options, "https://api.github.com/orgs/justeat"); + await HttpAssert.GetAsync(options, "https://api.github.com:443/orgs/justeat"); + } + private sealed class CustomObject { internal enum Color From 54c1018aa967d7e25548c59d303832bd183afb48 Mon Sep 17 00:00:00 2001 From: martincostello Date: Sun, 11 Mar 2018 16:06:09 +0000 Subject: [PATCH 2/3] Remove port workaround from benchmarks Remove the workaround for the default port handling bug from the benchmarks setup found by #24. --- .../HttpClientInterception.Benchmarks/InterceptionBenchmarks.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/HttpClientInterception.Benchmarks/InterceptionBenchmarks.cs b/tests/HttpClientInterception.Benchmarks/InterceptionBenchmarks.cs index 6f358d9a..cd862549 100644 --- a/tests/HttpClientInterception.Benchmarks/InterceptionBenchmarks.cs +++ b/tests/HttpClientInterception.Benchmarks/InterceptionBenchmarks.cs @@ -39,7 +39,6 @@ public InterceptionBenchmarks() builder .Requests() .ForHttps() - .ForPort(443) .ForHost("files.domain.com") .ForPath("setup.exe") .ForQuery(string.Empty) From 8348da1d0d33259c2af001d9e1da5ed1953faa55 Mon Sep 17 00:00:00 2001 From: martincostello Date: Sun, 11 Mar 2018 16:20:24 +0000 Subject: [PATCH 3/3] Add unit test for custom port handling Add a unit test to verify custom handling of ports and schemes works as intended. --- .../HttpRequestInterceptionBuilderTests.cs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs b/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs index 43bed5f7..068f5dbd 100644 --- a/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs +++ b/tests/HttpClientInterception.Tests/HttpRequestInterceptionBuilderTests.cs @@ -1067,6 +1067,54 @@ public static async Task Using_The_Same_Builder_For_Multiple_Registrations_On_Th await HttpAssert.GetAsync(options, "https://api.github.com:443/orgs/justeat"); } + [Fact] + public static async Task Using_The_Same_Builder_For_Multiple_Registrations_With_Custom_Ports() + { + // Arrange + var options = new HttpClientInterceptorOptions() + { + ThrowOnMissingRegistration = true + }; + + var builder = new HttpRequestInterceptionBuilder() + .ForPort(123) + .ForHttps() + .ForHost("api.github.com") + .ForPath("orgs/justeat") + .RegisterWith(options); + + // Change to a different scheme without changing the port + builder = builder + .ForHttp() + .RegisterWith(options); + + // Change the port and not the scheme + builder = builder + .ForPort(456) + .RegisterWith(options); + + // Change the scheme and use an explicit port + builder = builder + .ForHttps() + .ForPort(443) + .RegisterWith(options); + + // Restore default scheme and port + builder = builder + .ForHttp() + .ForPort(-1) + .RegisterWith(options); + + // Act and Assert + await HttpAssert.GetAsync(options, "https://api.github.com:123/orgs/justeat"); + await HttpAssert.GetAsync(options, "http://api.github.com:123/orgs/justeat"); + await HttpAssert.GetAsync(options, "http://api.github.com:456/orgs/justeat"); + await HttpAssert.GetAsync(options, "https://api.github.com/orgs/justeat"); + await HttpAssert.GetAsync(options, "https://api.github.com:443/orgs/justeat"); + await HttpAssert.GetAsync(options, "http://api.github.com/orgs/justeat"); + await HttpAssert.GetAsync(options, "http://api.github.com:80/orgs/justeat"); + } + private sealed class CustomObject { internal enum Color