From ce4ebdfbf2080a38855ab8df5c9bb553db8724af Mon Sep 17 00:00:00 2001 From: David Shulman Date: Thu, 22 Mar 2018 12:58:54 -0700 Subject: [PATCH] Fix WinHttpHandler error handling (#28367) This PR addresses some error handling problems in WinHttpHandler as well as improves the error messages generated for WinHttpException. This was root caused to a bug in WinHttp where it can't do a GET request with a request body using chunked encoding. However, this exception was masked due to an error handling bug caused by an inconsistency in how WinHttp associates context values for callbacks during the WinHttpSendRequest API call. This PR now fixes the error handling around synchronous errors being returned from WinHttpSendRequest. The root WinHttp bug itself can't be fixed. So, doing a GET request will still throw an exception, but it will be a catchable HttpRequestException. Another bug was discovered while investigating #26278. WinHttpCloseHandle should only be called once and should never race between threads. This is normally protected when the request handle is closed via SafeHandle Dispose(). But there was a place in the callback method where we called WinHttpCloseHandle directly against the raw handle. Finally, the error messages for WinHttpExceptions have been improved to show the error number and the probable WinHttp API call that generated the error. This will save diagnostic time. I also moved the WinHttpException source code back from Common. It was originally shared between WinHttpHandler and ClientWebSocket. But then ClientWebSocket moved away from WinHttp implementation. So, it makes more sense for this class to be under WinHttpHandler. Example: Original WinHttpException message: "The parameter is incorrect" New WinHttpException message: "Error 87 calling WinHttpSendRequest, 'The parameter is incorrect'." Closes #28156 Contributes to #26278 --- .../src/Resources/Strings.resx | 3 ++ .../System.Net.Http.WinHttpHandler.msbuild | 2 +- .../src/System/Net/Http/WinHttpAuthHelper.cs | 4 +- .../Net/Http/WinHttpCookieContainerAdapter.cs | 4 +- .../src/System/Net/Http/WinHttpException.cs | 25 +++++++----- .../src/System/Net/Http/WinHttpHandler.cs | 38 ++++++++++--------- .../System/Net/Http/WinHttpRequestCallback.cs | 23 ++++++++--- .../System/Net/Http/WinHttpRequestStream.cs | 2 +- .../System/Net/Http/WinHttpResponseParser.cs | 8 ++-- .../System/Net/Http/WinHttpResponseStream.cs | 8 ++-- .../src/System/Net/Http/WinHttpTraceHelper.cs | 6 +-- .../FunctionalTests/WinHttpHandlerTest.cs | 21 ++++++++++ ....Net.Http.WinHttpHandler.Unit.Tests.csproj | 6 +-- .../src/Resources/Strings.resx | 3 ++ .../System.Net.Http.Unit.Tests.csproj | 6 +-- 15 files changed, 102 insertions(+), 57 deletions(-) rename src/{Common => System.Net.Http.WinHttpHandler}/src/System/Net/Http/WinHttpException.cs (76%) diff --git a/src/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx b/src/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx index 846190e58dcf..5ce718aaab7a 100644 --- a/src/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx +++ b/src/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx @@ -121,6 +121,9 @@ The stream was already consumed. It cannot be read again. + + Error {0} calling {1}, '{2}'. + WinHttpHandler is only supported on .NET Framework and .NET Core runtimes on Windows. It is not supported for Windows Store Applications (UWP) or Unix platforms. diff --git a/src/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.msbuild b/src/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.msbuild index eca0378858dd..043e9c4bd873 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.msbuild +++ b/src/System.Net.Http.WinHttpHandler/src/System.Net.Http.WinHttpHandler.msbuild @@ -23,7 +23,6 @@ - @@ -33,6 +32,7 @@ + diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs index d985c46f3eac..ab716d47ad2d 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpAuthHelper.cs @@ -296,7 +296,7 @@ public void ChangeDefaultCredentialsPolicy( Interop.WinHttp.WINHTTP_OPTION_AUTOLOGON_POLICY, ref optionData)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpSetOption)); } } @@ -365,7 +365,7 @@ private bool SetWinHttpCredential( password, IntPtr.Zero)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpSetCredentials)); } return true; diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs index 824d9cc776d2..5455255a4954 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpCookieContainerAdapter.cs @@ -61,7 +61,7 @@ public static void ResetCookieRequestHeaders(WinHttpRequestState state, Uri redi int lastError = Marshal.GetLastWin32Error(); if (lastError != Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND) { - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpAddRequestHeaders)); } } @@ -76,7 +76,7 @@ public static void ResetCookieRequestHeaders(WinHttpRequestState state, Uri redi (uint)cookieHeader.Length, Interop.WinHttp.WINHTTP_ADDREQ_FLAG_ADD)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpAddRequestHeaders)); } } } diff --git a/src/Common/src/System/Net/Http/WinHttpException.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpException.cs similarity index 76% rename from src/Common/src/System/Net/Http/WinHttpException.cs rename to src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpException.cs index d77e3471351f..bb7956762f62 100644 --- a/src/Common/src/System/Net/Http/WinHttpException.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpException.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.ComponentModel; +using System.Diagnostics; using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; @@ -39,37 +40,41 @@ public static int ConvertErrorCodeToHR(int error) } } - public static void ThrowExceptionUsingLastError() + public static void ThrowExceptionUsingLastError(string nameOfCalledFunction) { - throw CreateExceptionUsingLastError(); + throw CreateExceptionUsingLastError(nameOfCalledFunction); } - public static WinHttpException CreateExceptionUsingLastError() + public static WinHttpException CreateExceptionUsingLastError(string nameOfCalledFunction) { int lastError = Marshal.GetLastWin32Error(); - return CreateExceptionUsingError(lastError); + return CreateExceptionUsingError(lastError, nameOfCalledFunction); } - public static WinHttpException CreateExceptionUsingError(int error) + public static WinHttpException CreateExceptionUsingError(int error, string nameOfCalledFunction) { - var e = new WinHttpException(error, GetErrorMessage(error)); + var e = new WinHttpException(error, GetErrorMessage(error, nameOfCalledFunction)); ExceptionStackTrace.AddCurrentStack(e); return e; } - public static WinHttpException CreateExceptionUsingError(int error, Exception innerException) + public static WinHttpException CreateExceptionUsingError(int error, string nameOfCalledFunction, Exception innerException) { - var e = new WinHttpException(error, GetErrorMessage(error), innerException); + var e = new WinHttpException(error, GetErrorMessage(error, nameOfCalledFunction), innerException); ExceptionStackTrace.AddCurrentStack(e); return e; } - public static string GetErrorMessage(int error) + public static string GetErrorMessage(int error, string nameOfCalledFunction) { + Debug.Assert(!string.IsNullOrEmpty(nameOfCalledFunction)); + // Look up specific error message in WINHTTP.DLL since it is not listed in default system resources // and thus can't be found by default .Net interop. IntPtr moduleHandle = Interop.Kernel32.GetModuleHandle(Interop.Libraries.WinHttp); - return Interop.Kernel32.GetMessage(moduleHandle, error); + string httpError = Interop.Kernel32.GetMessage(moduleHandle, error); + + return SR.Format(SR.net_http_winhttp_error, error, nameOfCalledFunction, httpError); } } } diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs index 4b6447aa3efc..65ae9c528a9c 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs @@ -661,7 +661,7 @@ private static void AddRequestHeaders( (uint)requestHeadersBuffer.Length, Interop.WinHttp.WINHTTP_ADDREQ_FLAG_ADD)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpAddRequestHeaders)); } } @@ -728,7 +728,7 @@ private void EnsureSessionHandleExists(WinHttpRequestState state) WinHttpTraceHelper.Trace("WinHttpHandler.EnsureSessionHandleExists: error={0}", lastError); if (lastError != Interop.WinHttp.ERROR_INVALID_PARAMETER) { - ThrowOnInvalidHandle(sessionHandle); + ThrowOnInvalidHandle(sessionHandle, nameof(Interop.WinHttp.WinHttpOpen)); } // We must be running on a platform earlier than Win8.1/Win2K12R2 which doesn't support @@ -741,7 +741,7 @@ private void EnsureSessionHandleExists(WinHttpRequestState state) _proxyHelper.ManualSettingsOnly ? _proxyHelper.Proxy : Interop.WinHttp.WINHTTP_NO_PROXY_NAME, _proxyHelper.ManualSettingsOnly ? _proxyHelper.ProxyBypass : Interop.WinHttp.WINHTTP_NO_PROXY_BYPASS, (int)Interop.WinHttp.WINHTTP_FLAG_ASYNC); - ThrowOnInvalidHandle(sessionHandle); + ThrowOnInvalidHandle(sessionHandle, nameof(Interop.WinHttp.WinHttpOpen)); } uint optionAssuredNonBlockingTrue = 1; // TRUE @@ -757,7 +757,7 @@ private void EnsureSessionHandleExists(WinHttpRequestState state) int lastError = Marshal.GetLastWin32Error(); if (lastError != Interop.WinHttp.ERROR_WINHTTP_INVALID_OPTION) { - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpSetOption)); } } @@ -788,7 +788,7 @@ private async void StartRequest(WinHttpRequestState state) state.RequestMessage.RequestUri.Host, (ushort)state.RequestMessage.RequestUri.Port, 0); - ThrowOnInvalidHandle(connectHandle); + ThrowOnInvalidHandle(connectHandle, nameof(Interop.WinHttp.WinHttpConnect)); connectHandle.SetParentHandle(_sessionHandle); // Try to use the requested version if a known/supported version was explicitly requested. @@ -821,7 +821,7 @@ private async void StartRequest(WinHttpRequestState state) Interop.WinHttp.WINHTTP_NO_REFERER, Interop.WinHttp.WINHTTP_DEFAULT_ACCEPT_TYPES, flags); - ThrowOnInvalidHandle(state.RequestHandle); + ThrowOnInvalidHandle(state.RequestHandle, nameof(Interop.WinHttp.WinHttpOpenRequest)); state.RequestHandle.SetParentHandle(connectHandle); // Set callback function. @@ -957,7 +957,7 @@ private void SetSessionHandleTimeoutOptions(SafeWinHttpHandle sessionHandle) (int)_sendTimeout.TotalMilliseconds, (int)_receiveHeadersTimeout.TotalMilliseconds)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpSetTimeouts)); } } @@ -1214,7 +1214,7 @@ private void SetWinHttpOption(SafeWinHttpHandle handle, uint option, ref uint op option, ref optionData)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpSetOption)); } } @@ -1227,7 +1227,7 @@ private void SetWinHttpOption(SafeWinHttpHandle handle, uint option, string opti optionData, (uint)optionData.Length)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpSetOption)); } } @@ -1244,7 +1244,7 @@ private static void SetWinHttpOption( optionData, optionSize)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpSetOption)); } } @@ -1314,18 +1314,18 @@ private void SetStatusCallback( int lastError = Marshal.GetLastWin32Error(); if (lastError != Interop.WinHttp.ERROR_INVALID_HANDLE) // Ignore error if handle was already closed. { - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpSetStatusCallback)); } } } - private void ThrowOnInvalidHandle(SafeWinHttpHandle handle) + private void ThrowOnInvalidHandle(SafeWinHttpHandle handle, string nameOfCalledFunction) { if (handle.IsInvalid) { int lastError = Marshal.GetLastWin32Error(); WinHttpTraceHelper.Trace("WinHttpHandler.ThrowOnInvalidHandle: error={0}", lastError); - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, nameOfCalledFunction); } } @@ -1343,11 +1343,13 @@ private RendezvousAwaitable InternalSendRequestAsync(WinHttpRequestState st 0, state.ToIntPtr())) { - // Dispose (which will unpin) the state object. Since this failed, WinHTTP won't associate - // our context value (state object) to the request handle. And thus we won't get HANDLE_CLOSING - // notifications which would normally cause the state object to be unpinned and disposed. + // WinHTTP doesn't always associate our context value (state object) to the request handle. + // And thus we might not get a HANDLE_CLOSING notification which would normally cause the + // state object to be unpinned and disposed. So, we manually dispose the request handle and + // state object here. + state.RequestHandle.Dispose(); state.Dispose(); - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpSendRequest)); } } @@ -1371,7 +1373,7 @@ private RendezvousAwaitable InternalReceiveResponseHeadersAsync(WinHttpRequ { if (!Interop.WinHttp.WinHttpReceiveResponse(state.RequestHandle, IntPtr.Zero)) { - throw WinHttpException.CreateExceptionUsingLastError(); + throw WinHttpException.CreateExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpReceiveResponse)); } } diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs index ca3cf65d6ca1..4d2a9896670a 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs @@ -110,8 +110,19 @@ private static void RequestCallback( } catch (Exception ex) { - Interop.WinHttp.WinHttpCloseHandle(handle); state.SavedException = ex; + if (state.RequestHandle != null) + { + // Since we got a fatal error processing the request callback, + // we need to close the WinHttp request handle in order to + // abort the currently executing WinHttp async operation. + // + // We must always call Dispose() against the SafeWinHttpHandle + // wrapper and never close directly the raw WinHttp handle. + // The SafeWinHttpHandle wrapper is thread-safe and guarantees + // calling the underlying WinHttpCloseHandle() function only once. + state.RequestHandle.Dispose(); + } } } @@ -260,7 +271,7 @@ private static void OnRequestSendingRequest(WinHttpRequestState state) return; } - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, "WINHTTP_CALLBACK_STATUS_SENDING_REQUEST/WinHttpQueryOption"); } // Get any additional certificates sent from the remote server during the TLS/SSL handshake. @@ -295,7 +306,7 @@ private static void OnRequestSendingRequest(WinHttpRequestState state) catch (Exception ex) { throw WinHttpException.CreateExceptionUsingError( - (int)Interop.WinHttp.ERROR_WINHTTP_SECURE_FAILURE, ex); + (int)Interop.WinHttp.ERROR_WINHTTP_SECURE_FAILURE, "X509Chain.Build", ex); } finally { @@ -310,7 +321,7 @@ private static void OnRequestSendingRequest(WinHttpRequestState state) if (!result) { throw WinHttpException.CreateExceptionUsingError( - (int)Interop.WinHttp.ERROR_WINHTTP_SECURE_FAILURE); + (int)Interop.WinHttp.ERROR_WINHTTP_SECURE_FAILURE, "ServerCertificateValidationCallback"); } } } @@ -321,7 +332,7 @@ private static void OnRequestError(WinHttpRequestState state, Interop.WinHttp.WI Debug.Assert(state != null, "OnRequestError: state is null"); - Exception innerException = WinHttpException.CreateExceptionUsingError(unchecked((int)asyncResult.dwError)); + Exception innerException = WinHttpException.CreateExceptionUsingError(unchecked((int)asyncResult.dwError), "WINHTTP_CALLBACK_STATUS_REQUEST_ERROR"); switch (unchecked((uint)asyncResult.dwResult.ToInt32())) { @@ -424,7 +435,7 @@ private static void ResetAuthRequestHeaders(WinHttpRequestState state) int lastError = Marshal.GetLastWin32Error(); if (lastError != Interop.WinHttp.ERROR_WINHTTP_HEADER_NOT_FOUND) { - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, "WINHTTP_CALLBACK_STATUS_REDIRECT/WinHttpAddRequestHeaders"); } } } diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestStream.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestStream.cs index a1fc3aef658a..034fc564274d 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestStream.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestStream.cs @@ -249,7 +249,7 @@ private Task InternalWriteDataAsync(byte[] buffer, int offset, int count, IntPtr.Zero)) { _state.TcsInternalWriteDataToRequestStream.TrySetException( - new IOException(SR.net_http_io_write, WinHttpException.CreateExceptionUsingLastError())); + new IOException(SR.net_http_io_write, WinHttpException.CreateExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpWriteData)))); } } diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs index bc6ff3f10353..18e655f06bd3 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseParser.cs @@ -127,7 +127,7 @@ public static uint GetResponseHeaderNumberInfo(SafeWinHttpHandle requestHandle, ref resultSize, IntPtr.Zero)) { - WinHttpException.ThrowExceptionUsingLastError(); + WinHttpException.ThrowExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpQueryHeaders)); } return result; @@ -189,7 +189,7 @@ public static unsafe bool GetResponseHeader( return GetResponseHeader(requestHandle, infoLevel, ref buffer, ref index, out headerValue); } - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); } /// @@ -216,7 +216,7 @@ private static unsafe int GetResponseHeader(SafeWinHttpHandle requestHandle, uin Debug.Assert(lastError != Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER, "buffer must be of sufficient size."); - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); } } @@ -240,7 +240,7 @@ private static unsafe int GetResponseHeaderCharBufferLength(SafeWinHttpHandle re if (lastError != Interop.WinHttp.ERROR_INSUFFICIENT_BUFFER) { - throw WinHttpException.CreateExceptionUsingError(lastError); + throw WinHttpException.CreateExceptionUsingError(lastError, nameof(Interop.WinHttp.WinHttpQueryHeaders)); } } diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs index 2c3af3458ca1..a6aa8e444865 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpResponseStream.cs @@ -121,7 +121,7 @@ private async Task CopyToAsyncCore(Stream destination, byte[] buffer, Cancellati { if (!Interop.WinHttp.WinHttpQueryDataAvailable(_requestHandle, IntPtr.Zero)) { - throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError()); + throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpQueryDataAvailable))); } } int bytesAvailable = await _state.LifecycleAwaitable; @@ -137,7 +137,7 @@ private async Task CopyToAsyncCore(Stream destination, byte[] buffer, Cancellati { if (!Interop.WinHttp.WinHttpReadData(_requestHandle, Marshal.UnsafeAddrOfPinnedArrayElement(buffer, 0), (uint)Math.Min(bytesAvailable, buffer.Length), IntPtr.Zero)) { - throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError()); + throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpReadData))); } } int bytesRead = await _state.LifecycleAwaitable; @@ -222,7 +222,7 @@ private async Task ReadAsyncCore(byte[] buffer, int offset, int count, Canc Debug.Assert(!_requestHandle.IsInvalid); if (!Interop.WinHttp.WinHttpQueryDataAvailable(_requestHandle, IntPtr.Zero)) { - throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError()); + throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpQueryDataAvailable))); } } @@ -237,7 +237,7 @@ private async Task ReadAsyncCore(byte[] buffer, int offset, int count, Canc (uint)Math.Min(bytesAvailable, count), IntPtr.Zero)) { - throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError()); + throw new IOException(SR.net_http_io_read, WinHttpException.CreateExceptionUsingLastError(nameof(Interop.WinHttp.WinHttpReadData))); } } diff --git a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTraceHelper.cs b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTraceHelper.cs index 07aaa39f8422..5e2ff60b43b6 100644 --- a/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTraceHelper.cs +++ b/src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpTraceHelper.cs @@ -138,14 +138,14 @@ public static void TraceAsyncError(string message, Interop.WinHttp.WINHTTP_ASYNC uint apiIndex = (uint)asyncResult.dwResult.ToInt32(); uint error = asyncResult.dwError; - + string apiName = GetNameFromApiIndex(apiIndex); WriteLine( "{0}: api={1}, error={2}({3}) \"{4}\"", message, - GetNameFromApiIndex(apiIndex), + apiName, GetNameFromError(error), error, - WinHttpException.GetErrorMessage((int)error)); + WinHttpException.GetErrorMessage((int)error, apiName)); } private static void WriteLine(string message) diff --git a/src/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs b/src/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs index c15e68938663..cf5812a1d709 100644 --- a/src/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs +++ b/src/System.Net.Http.WinHttpHandler/tests/FunctionalTests/WinHttpHandlerTest.cs @@ -16,6 +16,8 @@ // WinHttpHandler is a class and not a namespace and can't be part of namespace paths. namespace System.Net.Http.WinHttpHandlerFunctional.Tests { + using Configuration = System.Net.Test.Common.Configuration; + // Note: Disposing the HttpClient object automatically disposes the handler within. So, it is not necessary // to separately Dispose (or have a 'using' statement) for the handler. [SkipOnTargetFramework(TargetFrameworkMonikers.Uap, "WinHttpHandler not supported on UAP")] @@ -113,6 +115,25 @@ public void SendAsync_SlowServerRespondsAfterDefaultReceiveTimeout_ThrowsHttpReq } } + [Fact] + public async Task SendAsync_GetUsingChunkedEncoding_ThrowsHttpRequestException() + { + // WinHTTP doesn't support GET requests with a request body that uses + // chunked encoding. This test pins this behavior and verifies that the + // error handling is working correctly. + var server = new Uri("http://www.microsoft.com"); // No network I/O actually happens. + var request = new HttpRequestMessage(HttpMethod.Get, server); + request.Content = new StringContent("Request body"); + request.Headers.TransferEncodingChunked = true; + + var handler = new WinHttpHandler(); + using (HttpClient client = new HttpClient(handler)) + { + HttpRequestException ex = await Assert.ThrowsAsync(() => client.SendAsync(request)); + _output.WriteLine(ex.ToString()); + } + } + public static bool JsonMessageContainsKeyValue(string message, string key, string value) { // TODO: Merge with System.Net.Http TestHelper class as part of GitHub Issue #4989. diff --git a/src/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj b/src/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj index 871e66692ea7..31adaf06e11b 100644 --- a/src/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj +++ b/src/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj @@ -73,9 +73,6 @@ Common\System\Net\Http\NoWriteNoSeekStreamContent.cs - - Common\System\Net\Http\WinHttpException.cs - Common\System\Net\Security\CertificateHelper.cs @@ -103,6 +100,9 @@ ProductionCode\WinHttpCookieContainerAdapter.cs + + ProductionCode\WinHttpException.cs + ProductionCode\WinHttpHandler.cs diff --git a/src/System.Net.Http/src/Resources/Strings.resx b/src/System.Net.Http/src/Resources/Strings.resx index c75d0a9adef6..b8cdb1d8a1d5 100644 --- a/src/System.Net.Http/src/Resources/Strings.resx +++ b/src/System.Net.Http/src/Resources/Strings.resx @@ -399,6 +399,9 @@ CONNECT request must contain Host header. + + Error {0} calling {1}, '{2}'. + This method is not implemented by this class. diff --git a/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj index 33dc3a78fdfa..cd2b2c90c225 100644 --- a/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj +++ b/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj @@ -373,6 +373,9 @@ ProductionCode\System\Net\Http\HttpSystemProxy.cs + + ProductionCode\System\Net\Http\WinHttpException.cs + ProductionCode\System\Net\Http\WinInetProxyHelper.cs @@ -394,9 +397,6 @@ Common\Interop\Windows\winhttp\Interop.SafeWinHttpHandle.cs - - Common\System\Net\Http\WinHttpException.cs - Common\System\Runtime\ExceptionServices\ExceptionStackTrace.cs