From 7977793d4fbb8f9eda2bc850435414b0b9fe08f3 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 20 Jun 2018 09:27:42 -0700 Subject: [PATCH] Catch all exceptions on ANCM/IIS border (#864) --- .../AspNetCore/src/dllmain.cpp | 4 +- .../AspNetCore/src/proxymodule.cxx | 177 ++++++++++-------- .../CommonLib/CommonLib.vcxproj | 1 + .../CommonLib/debugutil.cpp | 2 + src/AspNetCoreModuleV2/CommonLib/debugutil.h | 3 +- src/AspNetCoreModuleV2/CommonLib/exceptions.h | 59 ++++++ src/AspNetCoreModuleV2/CommonLib/stdafx.h | 2 +- .../exception_handler_tests.cpp | 49 +++++ 8 files changed, 212 insertions(+), 85 deletions(-) create mode 100644 src/AspNetCoreModuleV2/CommonLib/exceptions.h create mode 100644 test/CommonLibTests/exception_handler_tests.cpp diff --git a/src/AspNetCoreModuleV2/AspNetCore/src/dllmain.cpp b/src/AspNetCoreModuleV2/AspNetCore/src/dllmain.cpp index 2b77015e36..373b835190 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/src/dllmain.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/src/dllmain.cpp @@ -70,7 +70,7 @@ RegisterModule( DWORD dwServerVersion, IHttpModuleRegistrationInfo * pModuleInfo, IHttpServer * pHttpServer -) +) try /*++ Routine description: @@ -222,4 +222,4 @@ Finished: return hr; } - +CATCH_RETURN() diff --git a/src/AspNetCoreModuleV2/AspNetCore/src/proxymodule.cxx b/src/AspNetCoreModuleV2/AspNetCore/src/proxymodule.cxx index 59a1ac9c1f..94b6f356a1 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/src/proxymodule.cxx +++ b/src/AspNetCoreModuleV2/AspNetCore/src/proxymodule.cxx @@ -6,8 +6,7 @@ #include "applicationmanager.h" #include "applicationinfo.h" #include "acache.h" -#include "hostfxroptions.h" - +#include "exceptions.h" __override HRESULT @@ -16,13 +15,12 @@ ASPNET_CORE_PROXY_MODULE_FACTORY::GetHttpModule( IModuleAllocator * pAllocator ) { - ASPNET_CORE_PROXY_MODULE *pModule = new (pAllocator) ASPNET_CORE_PROXY_MODULE(); - if (pModule == NULL) + try { - return E_OUTOFMEMORY; + *ppModule = THROW_IF_NULL_ALLOC(new (pAllocator) ASPNET_CORE_PROXY_MODULE());; + return S_OK; } - *ppModule = pModule; - return S_OK; + CATCH_RETURN(); } __override @@ -82,21 +80,23 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( REQUEST_NOTIFICATION_STATUS retVal = RQ_NOTIFICATION_CONTINUE; IAPPLICATION* pApplication = NULL; STRU struExeLocation; - - if (g_fInShutdown) + try { - hr = HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS); - goto Finished; - } - pApplicationManager = APPLICATION_MANAGER::GetInstance(); + if (g_fInShutdown) + { + hr = HRESULT_FROM_WIN32(ERROR_SERVER_SHUTDOWN_IN_PROGRESS); + goto Finished; + } - hr = pApplicationManager->GetOrCreateApplicationInfo( - g_pHttpServer, + pApplicationManager = APPLICATION_MANAGER::GetInstance(); + + hr = pApplicationManager->GetOrCreateApplicationInfo( + g_pHttpServer, pHttpContext, - &m_pApplicationInfo); - if (FAILED(hr)) - { + &m_pApplicationInfo); + if (FAILED(hr)) + { goto Finished; } @@ -105,70 +105,76 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler( // Application cannot be started due to wrong hosting mode // the error should already been logged to window event log for the first request hr = E_APPLICATION_ACTIVATION_EXEC_FAILURE; - goto Finished; - } + goto Finished; + } - // app_offline check to avoid loading aspnetcorerh.dll unnecessarily - if (m_pApplicationInfo->AppOfflineFound()) + // app_offline check to avoid loading aspnetcorerh.dll unnecessarily + if (m_pApplicationInfo->AppOfflineFound()) + { + // servicing app_offline + HTTP_DATA_CHUNK DataChunk; + IHttpResponse *pResponse = NULL; + APP_OFFLINE_HTM *pAppOfflineHtm = NULL; + + pResponse = pHttpContext->GetResponse(); + pAppOfflineHtm = m_pApplicationInfo->QueryAppOfflineHtm(); + DBG_ASSERT(pAppOfflineHtm); + DBG_ASSERT(pResponse); + + // Ignore failure hresults as nothing we can do + // Set fTrySkipCustomErrors to true as we want client see the offline content + pResponse->SetStatus(503, "Service Unavailable", 0, hr, NULL, TRUE); + pResponse->SetHeader("Content-Type", + "text/html", + (USHORT)strlen("text/html"), + FALSE + ); + + DataChunk.DataChunkType = HttpDataChunkFromMemory; + DataChunk.FromMemory.pBuffer = (PVOID)pAppOfflineHtm->m_Contents.QueryStr(); + DataChunk.FromMemory.BufferLength = pAppOfflineHtm->m_Contents.QueryCB(); + pResponse->WriteEntityChunkByReference(&DataChunk); + + retVal = RQ_NOTIFICATION_FINISH_REQUEST; + goto Finished; + } + + // make sure assmebly is loaded and application is created + hr = m_pApplicationInfo->EnsureApplicationCreated(pHttpContext); + if (FAILED(hr)) + { + goto Finished; + } + + m_pApplicationInfo->ExtractApplication(&pApplication); + + // make sure application is in running state + // cannot recreate the application as we cannot reload clr for inprocess + if (pApplication != NULL && + pApplication->QueryStatus() != APPLICATION_STATUS::RUNNING && + pApplication->QueryStatus() != APPLICATION_STATUS::STARTING) + { + hr = HRESULT_FROM_WIN32(ERROR_SERVER_DISABLED); + goto Finished; + } + + // Create RequestHandler and process the request + hr = pApplication->CreateHandler(pHttpContext, + &m_pHandler); + + if (FAILED(hr)) + { + goto Finished; + } + + retVal = m_pHandler->OnExecuteRequestHandler(); + + } + catch (...) { - // servicing app_offline - HTTP_DATA_CHUNK DataChunk; - IHttpResponse *pResponse = NULL; - APP_OFFLINE_HTM *pAppOfflineHtm = NULL; - - pResponse = pHttpContext->GetResponse(); - pAppOfflineHtm = m_pApplicationInfo->QueryAppOfflineHtm(); - DBG_ASSERT(pAppOfflineHtm); - DBG_ASSERT(pResponse); - - // Ignore failure hresults as nothing we can do - // Set fTrySkipCustomErrors to true as we want client see the offline content - pResponse->SetStatus(503, "Service Unavailable", 0, hr, NULL, TRUE); - pResponse->SetHeader("Content-Type", - "text/html", - (USHORT)strlen("text/html"), - FALSE - ); - - DataChunk.DataChunkType = HttpDataChunkFromMemory; - DataChunk.FromMemory.pBuffer = (PVOID)pAppOfflineHtm->m_Contents.QueryStr(); - DataChunk.FromMemory.BufferLength = pAppOfflineHtm->m_Contents.QueryCB(); - pResponse->WriteEntityChunkByReference(&DataChunk); - - retVal = RQ_NOTIFICATION_FINISH_REQUEST; - goto Finished; + hr = CaughtExceptionHResult(); } - // make sure assmebly is loaded and application is created - hr = m_pApplicationInfo->EnsureApplicationCreated(pHttpContext); - if (FAILED(hr)) - { - goto Finished; - } - - m_pApplicationInfo->ExtractApplication(&pApplication); - - // make sure application is in running state - // cannot recreate the application as we cannot reload clr for inprocess - if (pApplication != NULL && - pApplication->QueryStatus() != APPLICATION_STATUS::RUNNING && - pApplication->QueryStatus() != APPLICATION_STATUS::STARTING) - { - hr = HRESULT_FROM_WIN32(ERROR_SERVER_DISABLED); - goto Finished; - } - - // Create RequestHandler and process the request - hr = pApplication->CreateHandler(pHttpContext, - &m_pHandler); - - if (FAILED(hr)) - { - goto Finished; - } - - retVal = m_pHandler->OnExecuteRequestHandler(); - Finished: if (FAILED(hr)) { @@ -200,7 +206,16 @@ ASPNET_CORE_PROXY_MODULE::OnAsyncCompletion( IHttpCompletionInfo * pCompletionInfo ) { - return m_pHandler->OnAsyncCompletion( - pCompletionInfo->GetCompletionBytes(), - pCompletionInfo->GetCompletionStatus()); + try + { + return m_pHandler->OnAsyncCompletion( + pCompletionInfo->GetCompletionBytes(), + pCompletionInfo->GetCompletionStatus()); + } + catch (...) + { + // Log exception + CaughtExceptionHResult(); + return RQ_NOTIFICATION_FINISH_REQUEST; + } } diff --git a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj index dde46fc814..3ce37711c7 100644 --- a/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj +++ b/src/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj @@ -190,6 +190,7 @@ + diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp index 023d578387..4cc4e3815d 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for license information. +#include "stdafx.h" + #include "debugutil.h" #include "dbgutil.h" diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.h b/src/AspNetCoreModuleV2/CommonLib/debugutil.h index d2340d6214..715f5273a2 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.h +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.h @@ -2,7 +2,8 @@ // Licensed under the MIT License. See License.txt in the project root for license information. #pragma once -#include "stdafx.h" + +#include #define ASPNETCORE_DEBUG_FLAG_INFO DEBUG_FLAG_INFO #define ASPNETCORE_DEBUG_FLAG_WARNING DEBUG_FLAG_WARN diff --git a/src/AspNetCoreModuleV2/CommonLib/exceptions.h b/src/AspNetCoreModuleV2/CommonLib/exceptions.h new file mode 100644 index 0000000000..84bb984bf4 --- /dev/null +++ b/src/AspNetCoreModuleV2/CommonLib/exceptions.h @@ -0,0 +1,59 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#pragma once + +#include + +#include "debugutil.h" + +inline VOID ReportUntypedException() +{ + DebugPrint(ASPNETCORE_DEBUG_FLAG_ERROR, "Unhandled non-standard exception"); +} + +inline VOID ReportException(std::exception& exception) +{ + DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, "Unhandled exception: %s", exception.what()); +} + +inline __declspec(noinline) HRESULT CaughtExceptionHResult() +{ + try + { + throw; + } + catch (const std::bad_alloc&) + { + return E_OUTOFMEMORY; + } + catch (std::system_error& exception) + { + ReportException(exception); + return exception.code().value(); + } + catch (std::exception& exception) + { + ReportException(exception); + return HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION); + } + catch (...) + { + ReportUntypedException(); + return HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION); + } +} + +template auto Throw_IfNullAlloc(PointerT pointer) +{ + if (pointer == nullptr) + { + throw std::bad_alloc(); + } + return pointer; +} + + +#define RETURN_CAUGHT_EXCEPTION() return CaughtExceptionHResult(); +#define CATCH_RETURN() catch (...) { RETURN_CAUGHT_EXCEPTION(); } +#define THROW_IF_NULL_ALLOC(ptr) Throw_IfNullAlloc(ptr) diff --git a/src/AspNetCoreModuleV2/CommonLib/stdafx.h b/src/AspNetCoreModuleV2/CommonLib/stdafx.h index 8b6893759c..24e683b14e 100644 --- a/src/AspNetCoreModuleV2/CommonLib/stdafx.h +++ b/src/AspNetCoreModuleV2/CommonLib/stdafx.h @@ -40,4 +40,4 @@ #include "hostfxr_utility.h" #include "EventLog.h" #include "hostfxroptions.h" - +#include "exceptions.h" diff --git a/test/CommonLibTests/exception_handler_tests.cpp b/test/CommonLibTests/exception_handler_tests.cpp new file mode 100644 index 0000000000..d0a9e97fdb --- /dev/null +++ b/test/CommonLibTests/exception_handler_tests.cpp @@ -0,0 +1,49 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +#include "stdafx.h" + +TEST(CaughtExceptionHResult, ReturnsOutOfMemoryForBadAlloc) +{ + HRESULT hr; + try + { + throw std::bad_alloc(); + } + catch(...) + { + hr = CaughtExceptionHResult(); + } + + EXPECT_EQ(E_OUTOFMEMORY, hr); +} + +TEST(CaughtExceptionHResult, ReturnsValueForSystemError) +{ + HRESULT hr; + try + { + throw std::system_error(E_INVALIDARG, std::system_category()); + } + catch(...) + { + hr = CaughtExceptionHResult(); + } + + EXPECT_EQ(E_INVALIDARG, hr); +} + +TEST(CaughtExceptionHResult, ReturnsUhandledExceptionForOtherExceptions) +{ + HRESULT hr; + try + { + throw E_INVALIDARG; + } + catch(...) + { + hr = CaughtExceptionHResult(); + } + + EXPECT_EQ(HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION), hr); +}