From 2621566904aabc833bf3515748c9c6253f5d37bc Mon Sep 17 00:00:00 2001 From: David Shulman Date: Tue, 25 Jul 2017 11:26:23 -0700 Subject: [PATCH 1/4] Fix HttpClient redirection logic on UAP This PR changes the implementation of sending requests in HttpClient for UAP when dealing with redirection. In the past, we would let the WinRT layer handle auto redirection logic. However, due to #9003, the cookies were getting lost on 3xx responses since the .NET layer didn't see them. So, this PR implements the redirection ourselves. One important part of redirection is that we need to drop credentials. The WinRT layer did this for us. However, we are unable to use a single WinRT HttpBaseProtocolFilter object since we need to remove the credential after the first redirect. So, we need to maintain a second filter that uses no credentials and keep it in sync regarding all the other properties of the primary filter. With this PR, the behavior of other aspects (such as controlling max number of redirects, etc.) now matches .NET Framework. So, some tests were adjusted to remove the UAP specific behavior checks. Fixes #9003 Fixes #22191 --- .../src/uap/System/Net/HttpClientHandler.cs | 145 +++++------- .../src/uap/System/Net/HttpHandlerToFilter.cs | 207 +++++++++++++++--- .../FunctionalTests/HttpClientHandlerTest.cs | 97 ++++++-- 3 files changed, 313 insertions(+), 136 deletions(-) diff --git a/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs b/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs index 50147daaef21..527f56dd251b 100644 --- a/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs +++ b/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs @@ -39,7 +39,7 @@ public partial class HttpClientHandler : HttpMessageHandler private static Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1"); private static readonly Lazy s_RTCookieUsageBehaviorSupported = new Lazy(InitRTCookieUsageBehaviorSupported); - private static bool RTCookieUsageBehaviorSupported => s_RTCookieUsageBehaviorSupported.Value; + internal static bool RTCookieUsageBehaviorSupported => s_RTCookieUsageBehaviorSupported.Value; private static readonly Lazy s_RTNoCacheSupported = new Lazy(InitRTNoCacheSupported); private static bool RTNoCacheSupported => s_RTNoCacheSupported.Value; @@ -49,14 +49,20 @@ public partial class HttpClientHandler : HttpMessageHandler #region Fields - private readonly RTHttpBaseProtocolFilter _rtFilter; private readonly HttpHandlerToFilter _handlerToFilter; private readonly HttpMessageHandler _diagnosticsPipeline; + // We need two different WinRT filters because we need to remove credentials during redirection requests + // and WinRT doesn't allow changing the filter properties after the first request. + private RTHttpBaseProtocolFilter _rtFilter; + private RTHttpBaseProtocolFilter _rtFilterWithNoCredentials; + private ClientCertificateOption _clientCertificateOptions; private CookieContainer _cookieContainer; private bool _useCookies; private DecompressionMethods _automaticDecompression; + private bool _allowAutoRedirect; + private int _maxAutomaticRedirections; private ICredentials _defaultProxyCredentials; private ICredentials _credentials; private IWebProxy _proxy; @@ -138,6 +144,7 @@ public DecompressionMethods AutomaticDecompression // HBPF will decompress both gzip and deflate, we will set // accept-encoding for one, the other, or both passed in here. _rtFilter.AutomaticDecompression = (value != DecompressionMethods.None); + _rtFilterWithNoCredentials.AutomaticDecompression = _rtFilter.AutomaticDecompression; _automaticDecompression = value; } } @@ -149,6 +156,7 @@ public bool UseProxy { CheckDisposedOrStarted(); _rtFilter.UseProxy = value; + _rtFilterWithNoCredentials.UseProxy = value; } } @@ -227,19 +235,23 @@ public ICredentials DefaultProxyCredentials public bool AllowAutoRedirect { - get { return _rtFilter.AllowAutoRedirect; } + get { return _allowAutoRedirect; } set { CheckDisposedOrStarted(); - _rtFilter.AllowAutoRedirect = value; + _allowAutoRedirect = value; } } public int MaxAutomaticRedirections { - get { return 10; } // WinRT Windows.Web.Http constant via use of native WinINet. + get { return _maxAutomaticRedirections; } set { + if (value <= 0) + { + throw new ArgumentOutOfRangeException("value"); + } CheckDisposedOrStarted(); } } @@ -251,6 +263,7 @@ public int MaxConnectionsPerServer { CheckDisposedOrStarted(); _rtFilter.MaxConnectionsPerServer = (uint)value; + _rtFilterWithNoCredentials.MaxConnectionsPerServer = (uint)value; } } @@ -350,36 +363,52 @@ public IDictionary Properties public HttpClientHandler() { - _rtFilter = new RTHttpBaseProtocolFilter(); - _handlerToFilter = new HttpHandlerToFilter(_rtFilter); + _rtFilter = CreateFilter(); + _rtFilterWithNoCredentials = CreateFilter(); + + _handlerToFilter = new HttpHandlerToFilter(_rtFilter, _rtFilterWithNoCredentials, this); _handlerToFilter.RequestMessageLookupKey = RequestMessageLookupKey; _handlerToFilter.SavedExceptionDispatchInfoLookupKey = SavedExceptionDispatchInfoLookupKey; _diagnosticsPipeline = new DiagnosticsHandler(_handlerToFilter); _clientCertificateOptions = ClientCertificateOption.Manual; + _useCookies = true; // deal with cookies by default. + _cookieContainer = new CookieContainer(); // default container used for dealing with auto-cookies. + + _allowAutoRedirect = true; + _maxAutomaticRedirections = 50; + + _automaticDecompression = DecompressionMethods.None; + } + + private RTHttpBaseProtocolFilter CreateFilter() + { + var filter = new RTHttpBaseProtocolFilter(); + // Always turn off WinRT cookie processing if the WinRT API supports turning it off. // Use .NET CookieContainer handling only. if (RTCookieUsageBehaviorSupported) { - _rtFilter.CookieUsageBehavior = RTHttpCookieUsageBehavior.NoCookies; + filter.CookieUsageBehavior = RTHttpCookieUsageBehavior.NoCookies; } - _useCookies = true; // deal with cookies by default. - _cookieContainer = new CookieContainer(); // default container used for dealing with auto-cookies. + // Handle redirections at the .NET layer so that we can see cookies on redirect responses + // and have control of the number of redirections allowed. + filter.AllowAutoRedirect = false; - // Managed at this layer for granularity, but uses the desktop default. - _rtFilter.AutomaticDecompression = false; - _automaticDecompression = DecompressionMethods.None; + filter.AutomaticDecompression = false; // We don't support using the UI model in HttpBaseProtocolFilter() especially for auto-handling 401 responses. - _rtFilter.AllowUI = false; - + filter.AllowUI = false; + // The .NET Desktop System.Net Http APIs (based on HttpWebRequest/HttpClient) uses no caching by default. // To preserve app-compat, we turn off caching in the WinRT HttpClient APIs. - _rtFilter.CacheControl.ReadBehavior = RTNoCacheSupported ? + filter.CacheControl.ReadBehavior = RTNoCacheSupported ? RTHttpCacheReadBehavior.NoCache : RTHttpCacheReadBehavior.MostRecent; - _rtFilter.CacheControl.WriteBehavior = RTHttpCacheWriteBehavior.NoCache; + filter.CacheControl.WriteBehavior = RTHttpCacheWriteBehavior.NoCache; + + return filter; } protected override void Dispose(bool disposing) @@ -408,27 +437,11 @@ protected override void Dispose(bool disposing) private async Task ConfigureRequest(HttpRequestMessage request) { - ApplyRequestCookies(request); - ApplyDecompressionSettings(request); await ApplyClientCertificateSettings().ConfigureAwait(false); } - // Taken from System.Net.CookieModule.OnSendingHeaders - private void ApplyRequestCookies(HttpRequestMessage request) - { - if (UseCookies) - { - string cookieHeader = CookieContainer.GetCookieHeader(request.RequestUri); - if (!string.IsNullOrWhiteSpace(cookieHeader)) - { - bool success = request.Headers.TryAddWithoutValidation(HttpKnownHeaderNames.Cookie, cookieHeader); - Debug.Assert(success); - } - } - } - private void ApplyDecompressionSettings(HttpRequestMessage request) { // Decompression: Add the Gzip and Deflate headers if not already present. @@ -604,68 +617,11 @@ protected internal override async Task SendAsync(HttpReques throw new HttpRequestException(SR.net_http_client_execution_error, ex); } - ProcessResponse(response); return response; } #endregion Request Execution - #region Response Processing - - private void ProcessResponse(HttpResponseMessage response) - { - ProcessResponseCookies(response); - } - - // Taken from System.Net.CookieModule.OnReceivedHeaders - private void ProcessResponseCookies(HttpResponseMessage response) - { - if (UseCookies) - { - IEnumerable values; - if (response.Headers.TryGetValues(HttpKnownHeaderNames.SetCookie, out values)) - { - foreach (string cookieString in values) - { - if (!string.IsNullOrWhiteSpace(cookieString)) - { - try - { - // Parse the cookies so that we can filter some of them out - CookieContainer helper = new CookieContainer(); - helper.SetCookies(response.RequestMessage.RequestUri, cookieString); - CookieCollection cookies = helper.GetCookies(response.RequestMessage.RequestUri); - foreach (Cookie cookie in cookies) - { - // We don't want to put HttpOnly cookies in the CookieContainer if the system - // doesn't support the RTHttpBaseProtocolFilter CookieUsageBehavior property. - // Prior to supporting that, the WinRT HttpClient could not turn off cookie - // processing. So, it would always be storing all cookies in its internal container. - // Putting HttpOnly cookies in the .NET CookieContainer would cause problems later - // when the .NET layer tried to add them on outgoing requests and conflicted with - // the WinRT internal cookie processing. - // - // With support for WinRT CookieUsageBehavior, cookie processing is turned off - // within the WinRT layer. This allows us to process cookies using only the .NET - // layer. So, we need to add all applicable cookies that are received to the - // CookieContainer. - if (RTCookieUsageBehaviorSupported || !cookie.HttpOnly) - { - CookieContainer.Add(response.RequestMessage.RequestUri, cookie); - } - } - } - catch (Exception) - { - } - } - } - } - } - } - - #endregion Response Processing - #region Helpers private void SetOperationStarted() @@ -701,6 +657,15 @@ private void SetOperationStarted() _rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Untrusted); _rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.WrongUsage); _rtFilter.ServerCustomValidationRequested += RTServerCertificateCallback; + + _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Expired); + _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.IncompleteChain); + _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.InvalidName); + _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.RevocationFailure); + _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.RevocationInformationMissing); + _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Untrusted); + _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.WrongUsage); + _rtFilterWithNoCredentials.ServerCustomValidationRequested += RTServerCertificateCallback; } _operationStarted = true; diff --git a/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs b/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs index dc75083cd60e..54f332a23140 100644 --- a/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs +++ b/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs @@ -26,18 +26,25 @@ namespace System.Net.Http { internal class HttpHandlerToFilter : HttpMessageHandler { - private readonly RTHttpBaseProtocolFilter _next; + private readonly RTHttpBaseProtocolFilter _filter; + private readonly RTHttpBaseProtocolFilter _filterWithNoCredentials; private int _filterMaxVersionSet; + private HttpClientHandler _handler; - internal HttpHandlerToFilter(RTHttpBaseProtocolFilter filter) + internal HttpHandlerToFilter( + RTHttpBaseProtocolFilter filter, + RTHttpBaseProtocolFilter filterWithNoCredentials, + HttpClientHandler handler) { if (filter == null) { throw new ArgumentNullException(nameof(filter)); } - _next = filter; + _filter = filter; + _filterWithNoCredentials = filterWithNoCredentials; _filterMaxVersionSet = 0; + _handler = handler; } internal string RequestMessageLookupKey { get; set; } @@ -45,46 +52,181 @@ internal HttpHandlerToFilter(RTHttpBaseProtocolFilter filter) protected internal override async Task SendAsync(HttpRequestMessage request, CancellationToken cancel) { + int redirects = 0; + HttpMethod requestHttpMethod; + bool skipRequestContentIfPresent = false; + HttpResponseMessage response = null; + if (request == null) { throw new ArgumentNullException(nameof(request)); } - cancel.ThrowIfCancellationRequested(); - RTHttpRequestMessage rtRequest = await ConvertRequestAsync(request).ConfigureAwait(false); + requestHttpMethod = request.Method; - RTHttpResponseMessage rtResponse; - try - { - rtResponse = await _next.SendRequestAsync(rtRequest).AsTask(cancel).ConfigureAwait(false); - } - catch (TaskCanceledException) - { - throw; - } - catch (Exception) + while (true) { - object info; - if (rtRequest.Properties.TryGetValue(SavedExceptionDispatchInfoLookupKey, out info)) + cancel.ThrowIfCancellationRequested(); + + if (response != null) { - ((ExceptionDispatchInfo)info).Throw(); + response.Dispose(); + response = null; } - throw; - } + RTHttpRequestMessage rtRequest = await ConvertRequestAsync( + request, + requestHttpMethod, + skipRequestContentIfPresent).ConfigureAwait(false); + + RTHttpResponseMessage rtResponse; + try + { + if (redirects > 0) + { + rtResponse = await _filterWithNoCredentials.SendRequestAsync(rtRequest).AsTask(cancel).ConfigureAwait(false); + } + else + { + rtResponse = await _filter.SendRequestAsync(rtRequest).AsTask(cancel).ConfigureAwait(false); + } + } + catch (TaskCanceledException) + { + throw; + } + catch (Exception) + { + object info; + if (rtRequest.Properties.TryGetValue(SavedExceptionDispatchInfoLookupKey, out info)) + { + ((ExceptionDispatchInfo)info).Throw(); + } + + throw; + } + + response = ConvertResponse(rtResponse); + + ProcessResponseCookies(response, request.RequestUri); + + if (!_handler.AllowAutoRedirect) + { + break; + } + + if (response.StatusCode != HttpStatusCode.MultipleChoices && + response.StatusCode != HttpStatusCode.MovedPermanently && + response.StatusCode != HttpStatusCode.Redirect && + response.StatusCode != HttpStatusCode.RedirectMethod && + response.StatusCode != HttpStatusCode.RedirectKeepVerb) + { + break; + } + + redirects++; + if (redirects > _handler.MaxAutomaticRedirections) + { + break; + } + + Uri redirectUri = response.Headers.Location; + if (redirectUri == null) + { + break; + } + + if (!redirectUri.IsAbsoluteUri) + { + redirectUri = new Uri(string.Format("{0}://{1}:{2}{3}", + request.RequestUri.Scheme, + request.RequestUri.Host, + request.RequestUri.Port, + redirectUri.OriginalString)); + } + + if (redirectUri.Scheme != Uri.UriSchemeHttp && + redirectUri.Scheme != Uri.UriSchemeHttps) + { + break; + } + + if (request.RequestUri.Scheme == Uri.UriSchemeHttps && + redirectUri.Scheme == Uri.UriSchemeHttp) + { + break; + } + + if (response.StatusCode != HttpStatusCode.RedirectKeepVerb && + requestHttpMethod == HttpMethod.Post) + { + requestHttpMethod = HttpMethod.Get; + skipRequestContentIfPresent = true; + } - // Update in case of redirects - request.RequestUri = rtRequest.RequestUri; + request.RequestUri = redirectUri; + } - HttpResponseMessage response = ConvertResponse(rtResponse); response.RequestMessage = request; return response; } - private async Task ConvertRequestAsync(HttpRequestMessage request) + // Taken from System.Net.CookieModule.OnReceivedHeaders + private void ProcessResponseCookies(HttpResponseMessage response, Uri uri) + { + if (_handler.UseCookies) + { + IEnumerable values; + if (response.Headers.TryGetValues(HttpKnownHeaderNames.SetCookie, out values)) + { + foreach (string cookieString in values) + { + if (!string.IsNullOrWhiteSpace(cookieString)) + { + try + { + // Parse the cookies so that we can filter some of them out + CookieContainer helper = new CookieContainer(); + helper.SetCookies(uri, cookieString); + CookieCollection cookies = helper.GetCookies(uri); + foreach (Cookie cookie in cookies) + { + // We don't want to put HttpOnly cookies in the CookieContainer if the system + // doesn't support the RTHttpBaseProtocolFilter CookieUsageBehavior property. + // Prior to supporting that, the WinRT HttpClient could not turn off cookie + // processing. So, it would always be storing all cookies in its internal container. + // Putting HttpOnly cookies in the .NET CookieContainer would cause problems later + // when the .NET layer tried to add them on outgoing requests and conflicted with + // the WinRT internal cookie processing. + // + // With support for WinRT CookieUsageBehavior, cookie processing is turned off + // within the WinRT layer. This allows us to process cookies using only the .NET + // layer. So, we need to add all applicable cookies that are received to the + // CookieContainer. + if (HttpClientHandler.RTCookieUsageBehaviorSupported || !cookie.HttpOnly) + { + _handler.CookieContainer.Add(uri, cookie); + } + } + } + catch (Exception) + { + } + } + } + } + } + } + + private async Task ConvertRequestAsync( + HttpRequestMessage request, + HttpMethod httpMethod, + bool skipRequestContentIfPresent) { - RTHttpRequestMessage rtRequest = new RTHttpRequestMessage(new RTHttpMethod(request.Method.Method), request.RequestUri); + RTHttpRequestMessage rtRequest = new RTHttpRequestMessage( + new RTHttpMethod(httpMethod.Method), + request.RequestUri); // Add a reference from the WinRT object back to the .NET object. rtRequest.Properties.Add(RequestMessageLookupKey, request); @@ -117,7 +259,7 @@ private async Task ConvertRequestAsync(HttpRequestMessage // So, we only have to change it if we don't want HTTP/2.0. if (maxVersion != RTHttpVersion.Http20) { - _next.MaxVersion = maxVersion; + _filter.MaxVersion = maxVersion; } } @@ -131,6 +273,17 @@ private async Task ConvertRequestAsync(HttpRequestMessage } } + // Cookies + if (_handler.UseCookies) + { + string cookieHeader = _handler.CookieContainer.GetCookieHeader(request.RequestUri); + if (!string.IsNullOrWhiteSpace(cookieHeader)) + { + bool success = rtRequest.Headers.TryAppendWithoutValidation(HttpKnownHeaderNames.Cookie, cookieHeader); + Debug.Assert(success); + } + } + // Properties foreach (KeyValuePair propertyPair in request.Properties) { @@ -138,7 +291,7 @@ private async Task ConvertRequestAsync(HttpRequestMessage } // Content - if (request.Content != null) + if (!skipRequestContentIfPresent && request.Content != null) { rtRequest.Content = await CreateRequestContentAsync(request, rtRequest.Headers).ConfigureAwait(false); } diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs index f6036491afce..9bcaf9ec4598 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs @@ -67,6 +67,28 @@ public class HttpClientHandlerTest : RemoteExecutorTestBase new object[] { 307 } }; + public static readonly object[][] RedirectStatusCodesOldMethodsNewMethods = { + new object[] { 300, "GET", "GET" }, + new object[] { 300, "POST", "GET" }, + new object[] { 300, "HEAD", "HEAD" }, + + new object[] { 301, "GET", "GET" }, + new object[] { 301, "POST", "GET" }, + new object[] { 301, "HEAD", "HEAD" }, + + new object[] { 302, "GET", "GET" }, + new object[] { 302, "POST", "GET" }, + new object[] { 302, "HEAD", "HEAD" }, + + new object[] { 303, "GET", "GET" }, + new object[] { 303, "POST", "GET" }, + new object[] { 303, "HEAD", "HEAD" }, + + new object[] { 307, "GET", "GET" }, + new object[] { 307, "POST", "POST" }, + new object[] { 307, "HEAD", "HEAD" }, + }; + // Standard HTTP methods defined in RFC7231: http://tools.ietf.org/html/rfc7231#section-4.3 // "GET", "HEAD", "POST", "PUT", "DELETE", "OPTIONS", "TRACE" public static readonly IEnumerable HttpMethods = @@ -114,6 +136,7 @@ public void Ctor_ExpectedDefaultPropertyValues_CommonPlatform() Assert.NotNull(cookies); Assert.Equal(0, cookies.Count); Assert.Null(handler.Credentials); + Assert.Equal(50, handler.MaxAutomaticRedirections); Assert.NotNull(handler.Properties); Assert.Equal(null, handler.Proxy); Assert.True(handler.SupportsAutomaticDecompression); @@ -130,7 +153,6 @@ public void Ctor_ExpectedDefaultPropertyValues_NotUapPlatform() using (var handler = new HttpClientHandler()) { // Same as .NET Framework (Desktop). - Assert.Equal(50, handler.MaxAutomaticRedirections); Assert.Equal(64, handler.MaxResponseHeadersLength); Assert.False(handler.PreAuthenticate); Assert.True(handler.SupportsProxy); @@ -152,7 +174,6 @@ public void Ctor_ExpectedDefaultPropertyValues_UapPlatform() using (var handler = new HttpClientHandler()) { Assert.True(handler.CheckCertificateRevocationList); - Assert.Equal(10, handler.MaxAutomaticRedirections); Assert.Equal(0, handler.MaxRequestContentBufferSize); Assert.Equal(-1, handler.MaxResponseHeadersLength); Assert.True(handler.PreAuthenticate); @@ -180,6 +201,17 @@ public void Credentials_SetGet_Roundtrips() } } + [Theory] + [InlineData(-1)] + [InlineData(0)] + public void MaxAutomaticRedirections_InvalidValue_Throws(int redirects) + { + using (var handler = new HttpClientHandler()) + { + Assert.Throws(() => handler.MaxAutomaticRedirections = redirects); + } + } + [Theory] [InlineData(-1)] [InlineData((long)int.MaxValue + (long)1)] @@ -494,6 +526,46 @@ public async Task GetAsync_AllowAutoRedirectFalse_RedirectFromHttpToHttp_StatusC } } + [OuterLoop] // TODO: Issue #11345 + [Theory, MemberData(nameof(RedirectStatusCodesOldMethodsNewMethods))] + public async Task AllowAutoRedirect_True_ValidateNewMethodUsedOnRedirection( + int statusCode, string oldMethod, string newMethod) + { + if (ManagedHandlerTestHelpers.IsEnabled) + { + // TODO #22700: Managed handler not following RFC rules for method rewrites. + return; + } + + var handler = new HttpClientHandler() { AllowAutoRedirect = true }; + using (var client = new HttpClient(handler)) + { + await LoopbackServer.CreateServerAsync(async (origServer, origUrl) => + { + var request = new HttpRequestMessage(new HttpMethod(oldMethod), origUrl); + Task getResponse = client.SendAsync(request); + + await LoopbackServer.ReadRequestAndSendResponseAsync(origServer, + $"HTTP/1.1 {statusCode} OK\r\n" + + $"Date: {DateTimeOffset.UtcNow:R}\r\n" + + $"Location: {origUrl}\r\n" + + "\r\n"); + + List receivedRequest = await LoopbackServer.ReadRequestAndSendResponseAsync(origServer, + $"HTTP/1.1 200 OK\r\n" + + $"Date: {DateTimeOffset.UtcNow:R}\r\n" + + "\r\n"); + string[] statusLineParts = receivedRequest[0].Split(' '); + + using (HttpResponseMessage response = await getResponse) + { + Assert.Equal(200, (int)response.StatusCode); + Assert.Equal(newMethod, statusLineParts[0]); + } + }); + } + } + [OuterLoop] // TODO: Issue #11345 [Theory, MemberData(nameof(RedirectStatusCodes))] public async Task GetAsync_AllowAutoRedirectTrue_RedirectFromHttpToHttp_StatusCodeOK(int statusCode) @@ -553,23 +625,11 @@ public async Task GetAsync_AllowAutoRedirectTrue_RedirectFromHttpsToHttp_StatusC destinationUri: Configuration.Http.RemoteEchoServer, hops: 1); _output.WriteLine("Uri: {0}", uri); - - if (PlatformDetection.IsUap) - { - // UAP platform does not allow redirecting from HTTPS to HTTP (same as .NET Core). - // But in addition, it will throw an exception - // - // HttpRequestException: "An error occurred while sending the request." - // COMException: "A redirect request will change a secure to a non-secure connection" - await Assert.ThrowsAsync(() => client.GetAsync(uri)); - } - else + + using (HttpResponseMessage response = await client.GetAsync(uri)) { - using (HttpResponseMessage response = await client.GetAsync(uri)) - { - Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); - Assert.Equal(uri, response.RequestMessage.RequestUri); - } + Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); + Assert.Equal(uri, response.RequestMessage.RequestUri); } } } @@ -841,7 +901,6 @@ public async Task GetAsync_SetCookieContainer_CookieSent(string cookieName, stri } } - [ActiveIssue(9003, TargetFrameworkMonikers.Uap)] [OuterLoop] // TODO: Issue #11345 [Theory] [InlineData("cookieName1", "cookieValue1")] From ff3e8958c29d3b4a3d9c39eaa399bfc708cf5f71 Mon Sep 17 00:00:00 2001 From: David Shulman Date: Thu, 27 Jul 2017 15:54:30 -0700 Subject: [PATCH 2/4] Disable test on Linux --- .../tests/FunctionalTests/HttpClientHandlerTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs index 9bcaf9ec4598..a50cfd4ce47b 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs @@ -526,6 +526,7 @@ public async Task GetAsync_AllowAutoRedirectFalse_RedirectFromHttpToHttp_StatusC } } + [ActiveIssue(22707, TestPlatforms.AnyUnix)] [OuterLoop] // TODO: Issue #11345 [Theory, MemberData(nameof(RedirectStatusCodesOldMethodsNewMethods))] public async Task AllowAutoRedirect_True_ValidateNewMethodUsedOnRedirection( From a1722c1c160f7b7d59d7d8d651c9919f2536397f Mon Sep 17 00:00:00 2001 From: David Shulman Date: Fri, 28 Jul 2017 13:53:42 -0700 Subject: [PATCH 3/4] Address PR feedback --- .../src/uap/System/Net/HttpClientHandler.cs | 21 +------ .../src/uap/System/Net/HttpHandlerToFilter.cs | 55 ++++++++++++++++--- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs b/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs index 527f56dd251b..094fdd2c4456 100644 --- a/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs +++ b/src/System.Net.Http/src/uap/System/Net/HttpClientHandler.cs @@ -52,11 +52,7 @@ public partial class HttpClientHandler : HttpMessageHandler private readonly HttpHandlerToFilter _handlerToFilter; private readonly HttpMessageHandler _diagnosticsPipeline; - // We need two different WinRT filters because we need to remove credentials during redirection requests - // and WinRT doesn't allow changing the filter properties after the first request. private RTHttpBaseProtocolFilter _rtFilter; - private RTHttpBaseProtocolFilter _rtFilterWithNoCredentials; - private ClientCertificateOption _clientCertificateOptions; private CookieContainer _cookieContainer; private bool _useCookies; @@ -144,7 +140,6 @@ public DecompressionMethods AutomaticDecompression // HBPF will decompress both gzip and deflate, we will set // accept-encoding for one, the other, or both passed in here. _rtFilter.AutomaticDecompression = (value != DecompressionMethods.None); - _rtFilterWithNoCredentials.AutomaticDecompression = _rtFilter.AutomaticDecompression; _automaticDecompression = value; } } @@ -156,7 +151,6 @@ public bool UseProxy { CheckDisposedOrStarted(); _rtFilter.UseProxy = value; - _rtFilterWithNoCredentials.UseProxy = value; } } @@ -263,7 +257,6 @@ public int MaxConnectionsPerServer { CheckDisposedOrStarted(); _rtFilter.MaxConnectionsPerServer = (uint)value; - _rtFilterWithNoCredentials.MaxConnectionsPerServer = (uint)value; } } @@ -364,9 +357,8 @@ public IDictionary Properties public HttpClientHandler() { _rtFilter = CreateFilter(); - _rtFilterWithNoCredentials = CreateFilter(); - _handlerToFilter = new HttpHandlerToFilter(_rtFilter, _rtFilterWithNoCredentials, this); + _handlerToFilter = new HttpHandlerToFilter(_rtFilter, this); _handlerToFilter.RequestMessageLookupKey = RequestMessageLookupKey; _handlerToFilter.SavedExceptionDispatchInfoLookupKey = SavedExceptionDispatchInfoLookupKey; _diagnosticsPipeline = new DiagnosticsHandler(_handlerToFilter); @@ -657,15 +649,6 @@ private void SetOperationStarted() _rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Untrusted); _rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.WrongUsage); _rtFilter.ServerCustomValidationRequested += RTServerCertificateCallback; - - _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Expired); - _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.IncompleteChain); - _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.InvalidName); - _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.RevocationFailure); - _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.RevocationInformationMissing); - _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Untrusted); - _rtFilterWithNoCredentials.IgnorableServerCertificateErrors.Add(RTChainValidationResult.WrongUsage); - _rtFilterWithNoCredentials.ServerCustomValidationRequested += RTServerCertificateCallback; } _operationStarted = true; @@ -693,7 +676,7 @@ private static bool InitRTServerCustomValidationRequestedSupported() "ServerCustomValidationRequested"); } - private void RTServerCertificateCallback(RTHttpBaseProtocolFilter sender, RTHttpServerCustomValidationRequestedEventArgs args) + internal void RTServerCertificateCallback(RTHttpBaseProtocolFilter sender, RTHttpServerCustomValidationRequestedEventArgs args) { bool success = RTServerCertificateCallbackHelper( args.RequestMessage, diff --git a/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs b/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs index 54f332a23140..e2bcf684bdea 100644 --- a/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs +++ b/src/System.Net.Http/src/uap/System/Net/HttpHandlerToFilter.cs @@ -21,19 +21,23 @@ using RTIHttpContent = Windows.Web.Http.IHttpContent; using RTIInputStream = Windows.Storage.Streams.IInputStream; using RTHttpBaseProtocolFilter = Windows.Web.Http.Filters.HttpBaseProtocolFilter; +using RTChainValidationResult = Windows.Security.Cryptography.Certificates.ChainValidationResult; namespace System.Net.Http { internal class HttpHandlerToFilter : HttpMessageHandler { + // We need two different WinRT filters because we need to remove credentials during redirection requests + // and WinRT doesn't allow changing the filter properties after the first request. private readonly RTHttpBaseProtocolFilter _filter; - private readonly RTHttpBaseProtocolFilter _filterWithNoCredentials; + private Lazy _filterWithNoCredentials; + private RTHttpBaseProtocolFilter FilterWithNoCredentials => _filterWithNoCredentials.Value; + private int _filterMaxVersionSet; private HttpClientHandler _handler; internal HttpHandlerToFilter( RTHttpBaseProtocolFilter filter, - RTHttpBaseProtocolFilter filterWithNoCredentials, HttpClientHandler handler) { if (filter == null) @@ -42,9 +46,10 @@ internal HttpHandlerToFilter( } _filter = filter; - _filterWithNoCredentials = filterWithNoCredentials; _filterMaxVersionSet = 0; _handler = handler; + + _filterWithNoCredentials = new Lazy(InitFilterWithNoCredentials); } internal string RequestMessageLookupKey { get; set; } @@ -84,7 +89,7 @@ protected internal override async Task SendAsync(HttpReques { if (redirects > 0) { - rtResponse = await _filterWithNoCredentials.SendRequestAsync(rtRequest).AsTask(cancel).ConfigureAwait(false); + rtResponse = await FilterWithNoCredentials.SendRequestAsync(rtRequest).AsTask(cancel).ConfigureAwait(false); } else { @@ -138,11 +143,7 @@ protected internal override async Task SendAsync(HttpReques if (!redirectUri.IsAbsoluteUri) { - redirectUri = new Uri(string.Format("{0}://{1}:{2}{3}", - request.RequestUri.Scheme, - request.RequestUri.Host, - request.RequestUri.Port, - redirectUri.OriginalString)); + redirectUri = new Uri(request.RequestUri, redirectUri.OriginalString); } if (redirectUri.Scheme != Uri.UriSchemeHttp && @@ -157,6 +158,10 @@ protected internal override async Task SendAsync(HttpReques break; } + // Follow HTTP RFC 7231 rules. In general, 3xx responses + // except for 307 will keep verb except POST becomes GET. + // 307 responses have all verbs stay the same. + // https://tools.ietf.org/html/rfc7231#section-6.4 if (response.StatusCode != HttpStatusCode.RedirectKeepVerb && requestHttpMethod == HttpMethod.Post) { @@ -172,6 +177,38 @@ protected internal override async Task SendAsync(HttpReques return response; } + private RTHttpBaseProtocolFilter InitFilterWithNoCredentials() + { + RTHttpBaseProtocolFilter filter = new RTHttpBaseProtocolFilter(); + + filter.AllowAutoRedirect = _filter.AllowAutoRedirect; + filter.AllowUI = _filter.AllowUI; + filter.AutomaticDecompression = _filter.AutomaticDecompression; + filter.CacheControl.ReadBehavior = _filter.CacheControl.ReadBehavior; + filter.CacheControl.WriteBehavior = _filter.CacheControl.WriteBehavior; + + if (HttpClientHandler.RTCookieUsageBehaviorSupported) + { + filter.CookieUsageBehavior = _filter.CookieUsageBehavior; + } + + filter.MaxConnectionsPerServer = _filter.MaxConnectionsPerServer; + filter.MaxVersion = _filter.MaxVersion; + filter.UseProxy = _filter.UseProxy; + + if (_handler.ServerCertificateCustomValidationCallback != null) + { + foreach (RTChainValidationResult error in _filter.IgnorableServerCertificateErrors) + { + filter.IgnorableServerCertificateErrors.Add(error); + } + + filter.ServerCustomValidationRequested += _handler.RTServerCertificateCallback; + } + + return filter; + } + // Taken from System.Net.CookieModule.OnReceivedHeaders private void ProcessResponseCookies(HttpResponseMessage response, Uri uri) { From ae1353f3ce2698e27b5d212295dab8afb3eb7091 Mon Sep 17 00:00:00 2001 From: David Shulman Date: Fri, 28 Jul 2017 14:56:21 -0700 Subject: [PATCH 4/4] Disable failing HTTP/2 test --- .../tests/FunctionalTests/HttpClientHandlerTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs index a50cfd4ce47b..bcd3dac6d06f 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs @@ -1874,6 +1874,7 @@ public async Task SendAsync_RequestVersion20_ResponseVersion20IfHttp2Supported(U } } + [ActiveIssue(22735)] [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "Specifying Version(2,0) throws exception on netfx")] [OuterLoop] // TODO: Issue #11345 [ConditionalTheory(nameof(IsWindows10Version1607OrGreater)), MemberData(nameof(Http2NoPushServers))]