diff --git a/eng/scripts/vs.17.buildtools.intpreview.json b/eng/scripts/vs.17.buildtools.intpreview.json index 8a7c5ac445cb..8102e007eb3b 100644 --- a/eng/scripts/vs.17.buildtools.intpreview.json +++ b/eng/scripts/vs.17.buildtools.intpreview.json @@ -16,10 +16,10 @@ "Microsoft.VisualStudio.Component.VC.ATL.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.x86.x64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL", "Microsoft.VisualStudio.Component.Windows10SDK.19041", "Microsoft.VisualStudio.Workload.ManagedDesktopBuildTools", "Microsoft.VisualStudio.Workload.MSBuildTools", diff --git a/eng/scripts/vs.17.buildtools.json b/eng/scripts/vs.17.buildtools.json index 3a125c9d35eb..f3844c15990a 100644 --- a/eng/scripts/vs.17.buildtools.json +++ b/eng/scripts/vs.17.buildtools.json @@ -16,10 +16,10 @@ "Microsoft.VisualStudio.Component.VC.ATL.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.x86.x64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL", "Microsoft.VisualStudio.Component.Windows10SDK.19041", "Microsoft.VisualStudio.Workload.ManagedDesktopBuildTools", "Microsoft.VisualStudio.Workload.MSBuildTools", diff --git a/eng/scripts/vs.17.buildtools.preview.json b/eng/scripts/vs.17.buildtools.preview.json index 571a49978103..000473d6fda0 100644 --- a/eng/scripts/vs.17.buildtools.preview.json +++ b/eng/scripts/vs.17.buildtools.preview.json @@ -16,10 +16,10 @@ "Microsoft.VisualStudio.Component.VC.ATL.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.x86.x64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL", "Microsoft.VisualStudio.Component.Windows10SDK.19041", "Microsoft.VisualStudio.Workload.ManagedDesktopBuildTools", "Microsoft.VisualStudio.Workload.MSBuildTools", diff --git a/eng/scripts/vs.17.intpreview.json b/eng/scripts/vs.17.intpreview.json index 67b7a1f8e90d..b54cad956354 100644 --- a/eng/scripts/vs.17.intpreview.json +++ b/eng/scripts/vs.17.intpreview.json @@ -13,10 +13,10 @@ "Microsoft.VisualStudio.Component.VC.ATL.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.x86.x64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL", "Microsoft.VisualStudio.Component.Windows10SDK.19041", "Microsoft.VisualStudio.Workload.ManagedDesktop", "Microsoft.VisualStudio.Workload.NativeDesktop", diff --git a/eng/scripts/vs.17.json b/eng/scripts/vs.17.json index 86acfd28fdbe..b88f97c4da7f 100644 --- a/eng/scripts/vs.17.json +++ b/eng/scripts/vs.17.json @@ -13,10 +13,10 @@ "Microsoft.VisualStudio.Component.VC.ATL.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.x86.x64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL", "Microsoft.VisualStudio.Component.Windows10SDK.19041", "Microsoft.VisualStudio.Workload.ManagedDesktop", "Microsoft.VisualStudio.Workload.NativeDesktop", diff --git a/eng/scripts/vs.17.preview.json b/eng/scripts/vs.17.preview.json index 39f8297f1f96..769a8e7ad7e6 100644 --- a/eng/scripts/vs.17.preview.json +++ b/eng/scripts/vs.17.preview.json @@ -13,10 +13,10 @@ "Microsoft.VisualStudio.Component.VC.ATL.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.ARM64", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.x86.x64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL.ARM64", - "Microsoft.VisualStudio.Component.VC.14.29.16.11.ATL", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.x86.x64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL.ARM64", + "Microsoft.VisualStudio.Component.VC.14.41.17.11.ATL", "Microsoft.VisualStudio.Component.Windows10SDK.19041", "Microsoft.VisualStudio.Workload.ManagedDesktop", "Microsoft.VisualStudio.Workload.NativeDesktop", diff --git a/eng/targets/Cpp.Common.props b/eng/targets/Cpp.Common.props index c824b87caa9a..fb168b25890b 100644 --- a/eng/targets/Cpp.Common.props +++ b/eng/targets/Cpp.Common.props @@ -20,8 +20,8 @@ 15.0 Win32Proj x64 - v143 - $(PlatformToolsetVersion) + 143 + v$(PlatformToolsetVersion) 10.0.19041.0 diff --git a/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/IIS-Common/lib/IISSetup.CommonLib.vcxproj b/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/IIS-Common/lib/IISSetup.CommonLib.vcxproj index d19c8d3f9759..3f315ea4d5be 100644 --- a/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/IIS-Common/lib/IISSetup.CommonLib.vcxproj +++ b/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/IIS-Common/lib/IISSetup.CommonLib.vcxproj @@ -38,7 +38,6 @@ StaticLibrary - $(PlatformToolsetVersion) false diff --git a/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/iisca/lib/iisca.vcxproj b/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/iisca/lib/iisca.vcxproj index 4d3383bd08f8..b4d663ab1d07 100644 --- a/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/iisca/lib/iisca.vcxproj +++ b/src/Installers/Windows/AspNetCoreModule-Setup/IIS-Setup/iisca/lib/iisca.vcxproj @@ -81,7 +81,6 @@ StaticLibrary Unicode - $(PlatformToolsetVersion) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp index e424cc6cb361..8684f0a49a5a 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/AppOfflineApplication.cpp @@ -12,6 +12,9 @@ HRESULT AppOfflineApplication::CreateHandler(IHttpContext* pHttpContext, IREQUES try { auto handler = std::make_unique(*pHttpContext, m_strAppOfflineContent); + // 'Don't return a pointer that may be invalid' + // We use make_unique to prevent memory leaks in case of exception during ctor and then release it so we don't delete the pointer +#pragma warning(suppress: 26487) *pRequestHandler = handler.release(); } CATCH_RETURN(); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ModuleEnvironment.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ModuleEnvironment.cpp index 7b333bdda889..63f1760a3faa 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ModuleEnvironment.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ModuleEnvironment.cpp @@ -9,8 +9,8 @@ extern DWORD g_dwIISServerVersion; static std::wstring GetIISVersion() { - int major = (int)(g_dwIISServerVersion >> 16); - int minor = (int)(g_dwIISServerVersion & 0xffff); + const int major = (int)(g_dwIISServerVersion >> 16); + const int minor = (int)(g_dwIISServerVersion & 0xffff); std::wstringstream version; version << major << "." << minor; @@ -47,11 +47,11 @@ void SetApplicationEnvironmentVariables(_In_ IHttpServer &server, _In_ IHttpCont SetEnvironmentVariable(L"ASPNETCORE_IIS_APP_POOL_CONFIG_FILE", server2->GetAppPoolConfigFile()); } - IHttpSite* site = pHttpContext.GetSite(); + const IHttpSite* site = pHttpContext.GetSite(); SetEnvironmentVariable(L"ASPNETCORE_IIS_SITE_NAME", site->GetSiteName()); SetEnvironmentVariable(L"ASPNETCORE_IIS_SITE_ID", std::to_wstring(site->GetSiteId()).c_str()); - IHttpApplication* app = pHttpContext.GetApplication(); + const IHttpApplication* app = pHttpContext.GetApplication(); SetEnvironmentVariable(L"ASPNETCORE_IIS_APP_CONFIG_PATH", app->GetAppConfigPath()); SetEnvironmentVariable(L"ASPNETCORE_IIS_APPLICATION_ID", app->GetApplicationId()); SetEnvironmentVariable(L"ASPNETCORE_IIS_APPLICATION_VIRTUAL_PATH", ToVirtualPath(app->GetAppConfigPath()).c_str()); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ShimOptions.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ShimOptions.cpp index 88865cd9b132..b01ffe7116b3 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ShimOptions.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/ShimOptions.cpp @@ -79,9 +79,9 @@ ShimOptions::ShimOptions(const ConfigurationSource &configurationSource) : m_strArguments = Environment::GetEnvironmentVariableValue(CS_ANCM_LAUNCHER_ARGS) .value_or(m_strArguments); - auto detailedErrorsEnabled = equals_ignore_case(L"1", detailedErrors) || equals_ignore_case(L"true", detailedErrors); - auto aspnetCoreEnvironmentEnabled = equals_ignore_case(L"Development", aspnetCoreEnvironment); - auto dotnetEnvironmentEnabled = equals_ignore_case(L"Development", dotnetEnvironment); + const auto detailedErrorsEnabled = equals_ignore_case(L"1", detailedErrors) || equals_ignore_case(L"true", detailedErrors); + const auto aspnetCoreEnvironmentEnabled = equals_ignore_case(L"Development", aspnetCoreEnvironment); + const auto dotnetEnvironmentEnabled = equals_ignore_case(L"Development", dotnetEnvironment); m_fShowDetailedErrors = detailedErrorsEnabled || aspnetCoreEnvironmentEnabled || dotnetEnvironmentEnabled; @@ -111,7 +111,7 @@ ShimOptions::ShimOptions(const ConfigurationSource &configurationSource) : void ShimOptions::SetShutdownDelay(const std::wstring& shutdownDelay) { - auto millsecondsValue = std::stoi(shutdownDelay); + const auto millsecondsValue = std::stoi(shutdownDelay); if (millsecondsValue < 0) { throw ConfigurationLoadException(format( diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp index 519a36da628e..5da0aa9dea1c 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp @@ -116,7 +116,7 @@ APPLICATION_INFO::CreateApplication(IHttpContext& pHttpContext) pHttpApplication.GetApplicationId(), hr); - auto page = ANCM_ERROR_PAGE; + constexpr auto page = ANCM_ERROR_PAGE; std::string responseContent; if (options.QueryShowDetailedErrors()) { @@ -301,7 +301,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt auto shadowCopyBaseDirectory = std::filesystem::directory_entry(shadowCopyPath); if (!shadowCopyBaseDirectory.exists()) { - CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), NULL); + CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), nullptr); } for (auto& entry : std::filesystem::directory_iterator(shadowCopyPath)) @@ -311,7 +311,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt try { auto tempDirName = entry.path().filename().string(); - int intFileName = std::stoi(tempDirName); + const int intFileName = std::stoi(tempDirName); if (intFileName > directoryName) { directoryName = intFileName; @@ -334,7 +334,7 @@ APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHt // Avoid using canonical for shadowCopyBaseDirectory // It could expand to a network drive, or an expanded link folder path // We already made it an absolute path relative to the physicalPath above - HRESULT hr = Environment::CopyToDirectory(physicalPath, shadowCopyPath, options.QueryCleanShadowCopyDirectory(), shadowCopyBaseDirectory.path(), copiedFileCount); + const HRESULT hr = Environment::CopyToDirectory(physicalPath, shadowCopyPath, options.QueryCleanShadowCopyDirectory(), shadowCopyBaseDirectory.path(), copiedFileCount); LOG_INFOF(L"Finished copying %d files to shadow copy directory %ls.", copiedFileCount, shadowCopyBaseDirectory.path().c_str()); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/dllmain.cpp b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/dllmain.cpp index 4d55e36b80d9..050e6b301b4e 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/dllmain.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/dllmain.cpp @@ -57,6 +57,7 @@ BOOL WINAPI DllMain(HMODULE hModule, // this is a bug in IIS. To try to avoid AVs, we will set a global flag g_fInShutdown = TRUE; StaticCleanup(); + break; default: break; } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.h b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.h index 3bcb30c0b2e8..145f7f935ba1 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/globalmodule.h @@ -75,7 +75,7 @@ class ASPNET_CORE_GLOBAL_MODULE : NonCopyable, public CGlobalModule // IIS will actually stop giving us new requests and queue them instead for processing by the new app process. m_shutdown = std::thread([this]() { - auto delay = m_pApplicationManager->GetShutdownDelay(); + const auto delay = m_pApplicationManager->GetShutdownDelay(); LOG_INFOF(L"Shutdown starting in %d ms.", delay.count()); // Delay so that any incoming requests while we're returning from OnGlobalStopListening are allowed to be processed std::this_thread::sleep_for(delay); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ErrorContext.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ErrorContext.h index aa53a8249fef..2326a4154309 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ErrorContext.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ErrorContext.h @@ -5,6 +5,14 @@ struct ErrorContext { + ErrorContext() : + detailedErrorContent(), + generalErrorType(), + errorReason(), + statusCode(), + subStatusCode() + {} + // TODO consider adding HRESULT here std::string detailedErrorContent; USHORT statusCode; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.cpp b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.cpp index a9d942aabfec..290212f9248b 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.cpp @@ -87,6 +87,9 @@ void HostFxr::Load(const std::wstring& location) } } +// No array to pointer decay +// This is in reference to main taking PCWSTR argv[] and the analyzer wanting us to use span instead +#pragma warning(suppress: 26485) void HostFxr::SetMain(hostfxr_main_fn hostfxr_main_fn) { m_hostfxr_main_fn = hostfxr_main_fn; @@ -114,7 +117,7 @@ HostFxrErrorRedirector HostFxr::RedirectOutput(RedirectionOutput* writer) const return HostFxrErrorRedirector(m_corehost_set_error_writer_fn, writer); } -int HostFxr::InitializeForApp(int argc, PCWSTR* argv, const std::wstring& dotnetExe) const noexcept +int HostFxr::InitializeForApp(int argc, PCWSTR* argv, const std::wstring& dotnetExe) const { if (m_hostfxr_initialize_for_dotnet_commandline_fn == nullptr || m_hostfxr_main_fn != nullptr) { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.h index 48665b8400d8..6b273a39e698 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/HostFxr.h @@ -30,7 +30,7 @@ typedef int(*hostfxr_get_runtime_property_value_fn)(void* host_context_handle, P typedef int(*hostfxr_run_app_fn)(void* host_context_handle); typedef int(*hostfxr_close_fn)(void* hostfxr_context_handle); -const int AppArgNotRunnable = 0x80008094; +constexpr int AppArgNotRunnable = 0x80008094; class HostFxrErrorRedirector: NonCopyable { @@ -53,6 +53,9 @@ class HostFxr: NonCopyable { } + // No array to pointer decay + // This is in reference to main taking PCWSTR argv[] and the analyzer wanting us to use span instead +#pragma warning(suppress: 26485) HostFxr( hostfxr_main_fn hostfxr_main_fn, hostfxr_get_native_search_directories_fn hostfxr_get_native_search_directories_fn, @@ -60,7 +63,12 @@ class HostFxr: NonCopyable : m_hostfxr_main_fn(hostfxr_main_fn), m_hostfxr_get_native_search_directories_fn(hostfxr_get_native_search_directories_fn), m_corehost_set_error_writer_fn(corehost_set_error_writer_fn), - m_host_context_handle(nullptr) + m_host_context_handle(nullptr), + m_hostfxr_close_fn(nullptr), + m_hostfxr_get_runtime_property_value_fn(nullptr), + m_hostfxr_initialize_for_dotnet_commandline_fn(nullptr), + m_hostfxr_run_app_fn(nullptr), + m_hostfxr_set_runtime_property_value_fn(nullptr) { } @@ -78,7 +86,7 @@ class HostFxr: NonCopyable HostFxrErrorRedirector RedirectOutput(RedirectionOutput* writer) const noexcept; int SetRuntimePropertyValue(PCWSTR name, PCWSTR value) const noexcept; int GetRuntimePropertyValue(PCWSTR name, PWSTR* value) const noexcept; - int InitializeForApp(int argc, PCWSTR* argv, const std::wstring& m_dotnetExeKnownLocation) const noexcept; + int InitializeForApp(int argc, PCWSTR* argv, const std::wstring& m_dotnetExeKnownLocation) const; void Close() const noexcept; private: diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/RegistryKey.cpp b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/RegistryKey.cpp index 63828821f93c..475edbe6b90f 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/RegistryKey.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/RegistryKey.cpp @@ -18,7 +18,7 @@ std::optional RegistryKey::TryGetDWORD(HKEY section, const std::wstring& std::optional RegistryKey::TryGetString(HKEY section, const std::wstring& subSectionName, const std::wstring& valueName) { - DWORD cbData; + DWORD cbData{}; if (!CheckReturnValue(RegGetValue(section, subSectionName.c_str(), valueName.c_str(), RRF_RT_REG_SZ, nullptr, nullptr, &cbData))) { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorApplication.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorApplication.h index 6cb0cd934221..e6aa02ec86c0 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorApplication.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorApplication.h @@ -24,6 +24,9 @@ class ServerErrorApplication : public PollingAppOfflineApplication HRESULT CreateHandler(IHttpContext *pHttpContext, IREQUEST_HANDLER ** pRequestHandler) override { + // 'Don't return a pointer that may be invalid' + // We use make_unique to prevent memory leaks and then release it so we don't delete the pointer +#pragma warning(suppress: 26487) *pRequestHandler = std::make_unique(*pHttpContext, m_statusCode, m_subStatusCode, m_statusText, m_HR, m_disableStartupPage, m_responseContent).release(); return S_OK; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorHandler.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorHandler.h index 77d94230b256..31f7131f73a0 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorHandler.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/ServerErrorHandler.h @@ -9,13 +9,13 @@ class ServerErrorHandler : public REQUEST_HANDLER { public: - ServerErrorHandler(IHttpContext& pContext, USHORT statusCode, USHORT subStatusCode, const std::string& statusText, HRESULT hr, bool disableStartupPage, std::string& responseContent) noexcept + ServerErrorHandler(IHttpContext& pContext, USHORT statusCode, USHORT subStatusCode, const std::string& statusText, HRESULT hr, bool disableStartupPage, std::string& responseContent) : REQUEST_HANDLER(pContext), m_HR(hr), m_disableStartupPage(disableStartupPage), m_statusCode(statusCode), m_subStatusCode(subStatusCode), - m_statusText(std::move(statusText)), + m_statusText(statusText), m_ExceptionInfoContent(responseContent) { } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/StandardStreamRedirection.cpp b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/StandardStreamRedirection.cpp index 4abb04f85b93..94d093e31826 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/StandardStreamRedirection.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/StandardStreamRedirection.cpp @@ -156,6 +156,8 @@ void StandardStreamRedirection::Stop() dwThreadStatus == STILL_ACTIVE) { LOG_WARN(L"Thread reading stdout/err hit timeout, forcibly closing thread."); + // Using TerminateThread does not allow proper thread clean up. +#pragma warning(suppress: 6258) TerminateThread(m_hErrThread, STATUS_CONTROL_C_EXIT); } } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/config_utility.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/config_utility.h index e621447753ab..1e3e1a60b749 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/config_utility.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/config_utility.h @@ -61,7 +61,7 @@ class ConfigUtility HRESULT FindKeyValuePair(IAppHostElement* pElement, PCWSTR key, STRU& strHandlerVersionValue) { - HRESULT hr; + HRESULT hr = S_OK; CComPtr pHandlerSettings = nullptr; CComPtr pHandlerSettingsCollection = nullptr; CComPtr pHandlerVar = nullptr; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/debugutil.cpp b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/debugutil.cpp index 513c571cdbed..05749ceedac7 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/debugutil.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/debugutil.cpp @@ -114,7 +114,7 @@ std::wstring GetModuleName() { WCHAR path[MAX_PATH]; - LOG_LAST_ERROR_IF(!GetModuleFileName(g_hModule, path, sizeof(path))); + LOG_LAST_ERROR_IF(!GetModuleFileName(g_hModule, path, sizeof(path) / sizeof(WCHAR))); return path; } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/exceptions.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/exceptions.h index f4cd95c7f07d..9de03ce87f95 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/exceptions.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/exceptions.h @@ -35,14 +35,12 @@ #define RETURN_CAUGHT_EXCEPTION() return CaughtExceptionHResult(LOCATION_INFO); #define _CHECK_FAILED(expr) __pragma(warning(push)) \ - __pragma(warning(disable:4127)) /*disable condition is const warning*/ \ - FAILED(expr) \ - __pragma(warning(pop)) + __pragma(warning(suppress:4127)) /*disable condition is const warning*/ \ + FAILED(expr) #define _HR_RET(hr) __pragma(warning(push)) \ - __pragma(warning(disable:26498)) /*disable constexpr warning */ \ - const HRESULT __hrRet = hr; \ - __pragma(warning(pop)) + __pragma(warning(suppress:26498 26814)) /*disable constexpr warning */ \ + const HRESULT __hrRet = hr; #define RETURN_HR(hr) do { _HR_RET(hr); if (_CHECK_FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); } return __hrRet; } while (0, 0) #define RETURN_LAST_ERROR() do { return LogLastError(LOCATION_INFO); } while (0, 0) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/iapplication.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/iapplication.h index 52a1a7e2a9ee..fb0d311099a3 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/iapplication.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/iapplication.h @@ -62,7 +62,7 @@ std::unique_ptr make_application(Params&&... } template< class TYPE > -TYPE FindParameter(LPCSTR sRequiredParameter, APPLICATION_PARAMETER *pParameters, DWORD nParameters) +TYPE FindParameter(LPCSTR sRequiredParameter, const APPLICATION_PARAMETER *pParameters, DWORD nParameters) { for (DWORD i = 0; i < nParameters; i++) { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/sttimer.h b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/sttimer.h index 86b62b6fcf2a..3248d9252429 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/sttimer.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/sttimer.h @@ -13,7 +13,7 @@ class STTIMER public: STTIMER() - : _pTimer( NULL ) + : _pTimer( nullptr ) { fInCanel = FALSE; } @@ -25,7 +25,7 @@ class STTIMER { CancelTimer(); CloseThreadpoolTimer( _pTimer ); - _pTimer = NULL; + _pTimer = nullptr; } } @@ -39,7 +39,7 @@ class STTIMER { _pTimer = CreateThreadpoolTimer( pfnCallback, pContext, - NULL ); + nullptr ); if ( !_pTimer ) { @@ -61,7 +61,7 @@ class STTIMER DWORD dwPeriod = 0 ) { - FILETIME ftInitialWait; + FILETIME ftInitialWait{}; if ( dwInitialWait == 0 && dwPeriod == 0 ) { @@ -74,9 +74,9 @@ class STTIMER // re-enabled by setting non-zero initial wait or // period values. // - if (_pTimer != NULL) + if (_pTimer != nullptr) { - SetThreadpoolTimer(_pTimer, NULL, 0, 0); + SetThreadpoolTimer(_pTimer, nullptr, 0, 0); } return; @@ -106,7 +106,7 @@ class STTIMER // Wait until any callbacks queued prior to disabling // have completed. // - if (_pTimer != NULL) + if (_pTimer != nullptr) { WaitForThreadpoolTimerCallbacks(_pTimer, TRUE); } @@ -123,14 +123,14 @@ class STTIMER _In_ PTP_TIMER ) { - STRU* pstruLogFilePath = (STRU*)Context; - HANDLE hStdoutHandle = NULL; + const STRU* pstruLogFilePath = (STRU*)Context; + HANDLE hStdoutHandle = nullptr; SECURITY_ATTRIBUTES saAttr = { 0 }; HRESULT hr = S_OK; saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); saAttr.bInheritHandle = TRUE; - saAttr.lpSecurityDescriptor = NULL; + saAttr.lpSecurityDescriptor = nullptr; hStdoutHandle = CreateFileW(pstruLogFilePath->QueryStr(), FILE_READ_DATA, @@ -138,7 +138,7 @@ class STTIMER &saAttr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, - NULL); + nullptr); if (hStdoutHandle == INVALID_HANDLE_VALUE) { hr = HRESULT_FROM_WIN32(GetLastError()); @@ -186,8 +186,8 @@ class STELAPSED _dwPerfCountsPerMillisecond( 0 ), _fUsingHighResolution( FALSE ) { - LARGE_INTEGER li; - BOOL fResult; + LARGE_INTEGER li{}; + BOOL fResult = false; _dwInitTickCount = GetTickCount64(); @@ -224,11 +224,11 @@ class STELAPSED LONGLONG QueryElapsedTime() { - LARGE_INTEGER li; + LARGE_INTEGER li{}; if ( _fUsingHighResolution && QueryPerformanceCounter( &li ) ) { - DWORD64 dwCurrentTime = li.QuadPart / _dwPerfCountsPerMillisecond; + const DWORD64 dwCurrentTime = li.QuadPart / _dwPerfCountsPerMillisecond; if ( dwCurrentTime < _dwInitTime ) { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLibTests/CommonLibTests.vcxproj b/src/Servers/IIS/AspNetCoreModuleV2/CommonLibTests/CommonLibTests.vcxproj index e85b795c685a..38c026598a06 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLibTests/CommonLibTests.vcxproj +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLibTests/CommonLibTests.vcxproj @@ -9,7 +9,6 @@ {1eac8125-1765-4e2d-8cbe-56dc98a1c8c1} Win32Proj Application - $(PlatformToolsetVersion) Unicode true true diff --git a/src/Servers/IIS/AspNetCoreModuleV2/DefaultRules.ruleset b/src/Servers/IIS/AspNetCoreModuleV2/DefaultRules.ruleset index 1c2bbdaeaf40..16c7a1ce2768 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/DefaultRules.ruleset +++ b/src/Servers/IIS/AspNetCoreModuleV2/DefaultRules.ruleset @@ -1,5 +1,5 @@  - + @@ -12,14 +12,14 @@ - + - + - - + + @@ -27,73 +27,81 @@ - + - - + + - + - - + + - - - + + + - + - + - + - - - + + + - + + + - - - + + + - - - + + + + - - - - + + + + - - - - - + + + + + - - - - - + + + + + - + + + + + + @@ -142,7 +150,7 @@ - + @@ -213,9 +221,9 @@ - - - + + + @@ -286,9 +294,9 @@ - + - + @@ -389,8 +397,8 @@ - - + + diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/base64.cpp b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/base64.cpp index 0122dcb33ec5..c5516b06a6d0 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/base64.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/base64.cpp @@ -53,10 +53,12 @@ Return Values: *pcchEncoded = cchEncoded; } - if (cchEncodedStringSize == 0 && pszEncodedString == NULL) { + if (pszEncodedString == NULL) { return ERROR_SUCCESS; } + *pszEncodedString = 0; + if (cchEncodedStringSize < cchEncoded) { // Given buffer is too small to hold encoded string. return ERROR_INSUFFICIENT_BUFFER; @@ -66,8 +68,16 @@ Return Values: ib = ich = 0; while (ib < cbDecodedBufferSize) { b0 = pbDecodedBuffer[ib++]; - b1 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0; - b2 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0; + b1 = 0; + b2 = 0; + if (ib < cbDecodedBufferSize) + { + b1 = pbDecodedBuffer[ib++]; + } + if (ib < cbDecodedBufferSize) + { + b2 = pbDecodedBuffer[ib++]; + } // // The checks below for buffer overflow seems redundant to me. @@ -176,11 +186,20 @@ constexpr auto NA = (255); BYTE b0, b1, b2, b3; BYTE * pbDecodeBuffer = (BYTE *) pDecodeBuffer; - cchEncodedSize = (DWORD)wcslen(pszEncodedString); - if (NULL != pcbDecoded) { + if (NULL != pcbDecoded) + { *pcbDecoded = 0; } + if (pDecodeBuffer == NULL) + { + return ERROR_SUCCESS; + } + + *pbDecodeBuffer = 0; + + cchEncodedSize = (DWORD)wcslen(pszEncodedString); + if ((0 == cchEncodedSize) || (0 != (cchEncodedSize % 4))) { // Input string is not sized correctly to be base64. return ERROR_INVALID_PARAMETER; @@ -292,10 +311,12 @@ Return Values: *pcchEncoded = cchEncoded; } - if (cchEncodedStringSize == 0 && pszEncodedString == NULL) { + if (pszEncodedString == NULL) { return ERROR_SUCCESS; } + *pszEncodedString = 0; + if (cchEncodedStringSize < cchEncoded) { // Given buffer is too small to hold encoded string. return ERROR_INSUFFICIENT_BUFFER; @@ -305,8 +326,16 @@ Return Values: ib = ich = 0; while (ib < cbDecodedBufferSize) { b0 = pbDecodedBuffer[ib++]; - b1 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0; - b2 = (ib < cbDecodedBufferSize) ? pbDecodedBuffer[ib++] : 0; + b1 = 0; + b2 = 0; + if (ib < cbDecodedBufferSize) + { + b1 = pbDecodedBuffer[ib++]; + } + if (ib < cbDecodedBufferSize) + { + b2 = pbDecodedBuffer[ib++]; + } // // The checks below for buffer overflow seems redundant to me. @@ -415,11 +444,18 @@ Return Values: BYTE b0, b1, b2, b3; BYTE * pbDecodeBuffer = (BYTE *) pDecodeBuffer; - cchEncodedSize = (DWORD)strlen(pszEncodedString); if (NULL != pcbDecoded) { *pcbDecoded = 0; } + if (pDecodeBuffer == NULL) { + return ERROR_SUCCESS; + } + + *pbDecodeBuffer = 0; + + cchEncodedSize = (DWORD)strlen(pszEncodedString); + if ((0 == cchEncodedSize) || (0 != (cchEncodedSize % 4))) { // Input string is not sized correctly to be base64. return ERROR_INVALID_PARAMETER; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/buffer.h b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/buffer.h index 385b73d717e3..339f307fb60a 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/buffer.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/buffer.h @@ -259,7 +259,7 @@ C_ASSERT( sizeof(VOID*) <= sizeof(ULONGLONG) ); // bytes. If the buffer overflows then a heap buffer will be allocated. // #define STACK_BUFFER( _name, _size ) \ - ULONGLONG __aqw##_name[ ( ( (_size) + sizeof(ULONGLONG) - 1 ) / sizeof(ULONGLONG) ) ]; \ + ULONGLONG __aqw##_name[ ( ( (_size) + sizeof(ULONGLONG) - 1 ) / sizeof(ULONGLONG) ) ]{}; \ BUFFER _name( (BYTE*)__aqw##_name, sizeof(__aqw##_name) ) // diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashfn.h b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashfn.h index a7bfeda2cfad..aed28418af1a 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashfn.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashfn.h @@ -12,7 +12,7 @@ // the low end of the hashtable otherwise. LKRhash applies this internally // to all hash signatures for exactly this reason. -inline DWORD +inline constexpr DWORD HashScramble(DWORD dwHash) { // Here are 10 primes slightly greater than 10^9 @@ -20,9 +20,9 @@ HashScramble(DWORD dwHash) // 1000000093, 1000000097, 1000000103, 1000000123, 1000000181. // default value for "scrambling constant" - const DWORD RANDOM_CONSTANT = 314159269UL; + constexpr DWORD RANDOM_CONSTANT = 314159269UL; // large prime number, also used for scrambling - const DWORD RANDOM_PRIME = 1000000007UL; + constexpr DWORD RANDOM_PRIME = 1000000007UL; return (RANDOM_CONSTANT * dwHash) % RANDOM_PRIME ; } @@ -30,7 +30,7 @@ HashScramble(DWORD dwHash) // Faster scrambling function suggested by Eric Jacobsen -inline DWORD +inline DWORD constexpr HashRandomizeBits(DWORD dw) { return (((dw * 1103515245 + 12345) >> 16) @@ -39,7 +39,7 @@ HashRandomizeBits(DWORD dw) // Small prime number used as a multiplier in the supplied hash functions -const DWORD HASH_MULTIPLIER = 101; +constexpr DWORD HASH_MULTIPLIER = 101; #undef HASH_SHIFT_MULTIPLY @@ -255,10 +255,10 @@ inline DWORD Hash(const char* psz) { return HashString(psz); } inline DWORD Hash(const unsigned char* pusz) -{ return HashString(reinterpret_cast(pusz)); } +{ return HashString((const char*)pusz); } inline DWORD Hash(const signed char* pssz) -{ return HashString(reinterpret_cast(pssz)); } +{ return HashString((const char*)pssz); } inline DWORD Hash(const wchar_t* pwsz) { return HashString(pwsz); } @@ -268,56 +268,55 @@ Hash( const GUID* pguid, DWORD dwHash = 0) { - - return * reinterpret_cast(const_cast(pguid)) + dwHash; + return pguid->Data1 + dwHash; } // Identity hash functions: scalar values map to themselves -inline DWORD Hash(char c) +inline constexpr DWORD Hash(char c) { return c; } -inline DWORD Hash(unsigned char uc) +inline constexpr DWORD Hash(unsigned char uc) { return uc; } -inline DWORD Hash(signed char sc) +inline constexpr DWORD Hash(signed char sc) { return sc; } -inline DWORD Hash(short sh) +inline constexpr DWORD Hash(short sh) { return sh; } -inline DWORD Hash(unsigned short ush) +inline constexpr DWORD Hash(unsigned short ush) { return ush; } -inline DWORD Hash(int i) +inline constexpr DWORD Hash(int i) { return i; } -inline DWORD Hash(unsigned int u) +inline constexpr DWORD Hash(unsigned int u) { return u; } -inline DWORD Hash(long l) +inline constexpr DWORD Hash(long l) { return l; } -inline DWORD Hash(unsigned long ul) +inline constexpr DWORD Hash(unsigned long ul) { return ul; } -inline DWORD Hash(float f) +inline constexpr DWORD Hash(float f) { // be careful of rounding errors when computing keys union { float f; DWORD dw; - } u; + } u{}; u.f = f; return u.dw; } -inline DWORD Hash(double dbl) +inline constexpr DWORD Hash(double dbl) { // be careful of rounding errors when computing keys union { double dbl; DWORD dw[2]; - } u; + } u{}; u.dbl = dbl; return u.dw[0] * HASH_MULTIPLIER + u.dw[1]; } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashtable.h b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashtable.h index cde38374fe67..a7c541b40f38 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashtable.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/hashtable.h @@ -16,14 +16,14 @@ class HASH_NODE HASH_NODE( _Record * pRecord, DWORD dwHash - ) : _pNext (NULL), + ) : _pNext (nullptr), _pRecord (pRecord), _dwHash (dwHash) {} ~HASH_NODE() { - _ASSERTE(_pRecord == NULL); + _ASSERTE(_pRecord == nullptr); } private: @@ -57,7 +57,7 @@ class HASH_TABLE HASH_TABLE( VOID ) - : _ppBuckets( NULL ), + : _ppBuckets(nullptr), _nBuckets( 0 ), _nItems( 0 ) { @@ -158,7 +158,7 @@ class HASH_TABLE __deref_out HASH_NODE<_Record> ** ppNode, __deref_opt_out - HASH_NODE<_Record> *** pppPreviousNodeNextPointer = NULL + HASH_NODE<_Record> *** pppPreviousNodeNextPointer = nullptr ); VOID @@ -166,10 +166,10 @@ class HASH_TABLE HASH_NODE<_Record> * pNode ) { - if (pNode->_pRecord != NULL) + if (pNode->_pRecord != nullptr) { DereferenceRecord(pNode->_pRecord); - pNode->_pRecord = NULL; + pNode->_pRecord = nullptr; } delete pNode; @@ -210,8 +210,8 @@ HASH_TABLE<_Record,_Key>::Initialize( goto Failed; } - _ASSERTE(_ppBuckets == NULL ); - if ( _ppBuckets != NULL ) + _ASSERTE(_ppBuckets == nullptr); + if (_ppBuckets != nullptr) { hr = E_INVALIDARG; goto Failed; @@ -227,7 +227,7 @@ HASH_TABLE<_Record,_Key>::Initialize( GetProcessHeap(), HEAP_ZERO_MEMORY, nBuckets*sizeof(HASH_NODE<_Record> *)); - if (_ppBuckets == NULL) + if (_ppBuckets == nullptr) { hr = HRESULT_FROM_WIN32(ERROR_NOT_ENOUGH_MEMORY); goto Failed; @@ -243,7 +243,7 @@ HASH_TABLE<_Record,_Key>::Initialize( HeapFree(GetProcessHeap(), 0, _ppBuckets); - _ppBuckets = NULL; + _ppBuckets = nullptr; } return hr; @@ -253,7 +253,7 @@ HASH_TABLE<_Record,_Key>::Initialize( template HASH_TABLE<_Record,_Key>::~HASH_TABLE() { - if (_ppBuckets == NULL) + if (_ppBuckets == nullptr) { return; } @@ -263,7 +263,7 @@ HASH_TABLE<_Record,_Key>::~HASH_TABLE() HeapFree(GetProcessHeap(), 0, _ppBuckets); - _ppBuckets = NULL; + _ppBuckets = nullptr; _nBuckets = 0; } @@ -288,8 +288,8 @@ template VOID HASH_TABLE<_Record,_Key>::Clear() { - HASH_NODE<_Record> *pCurrent; - HASH_NODE<_Record> *pNext; + HASH_NODE<_Record> *pCurrent = nullptr; + HASH_NODE<_Record> *pNext = nullptr; // This is here in the off cases where someone instantiates a hashtable // and then does an automatic "clear" before its destruction WITHOUT @@ -304,8 +304,8 @@ HASH_TABLE<_Record,_Key>::Clear() for (DWORD i=0; i<_nBuckets; i++) { pCurrent = _ppBuckets[i]; - _ppBuckets[i] = NULL; - while (pCurrent != NULL) + _ppBuckets[i] = nullptr; + while (pCurrent != nullptr) { pNext = pCurrent->_pNext; DeleteNode(pCurrent); @@ -318,7 +318,7 @@ HASH_TABLE<_Record,_Key>::Clear() } template -__success(*ppNode != NULL && return != FALSE) +__success(*ppNode != nullptr && return != FALSE) BOOL HASH_TABLE<_Record,_Key>::FindNodeInternal( _Key key, @@ -338,13 +338,13 @@ HASH_TABLE<_Record,_Key>::FindNodeInternal( This routine may be called under either read or write lock --*/ { - HASH_NODE<_Record> **ppPreviousNodeNextPointer; - HASH_NODE<_Record> *pNode; + HASH_NODE<_Record> **ppPreviousNodeNextPointer = nullptr; + HASH_NODE<_Record> *pNode = nullptr; BOOL fFound = FALSE; ppPreviousNodeNextPointer = _ppBuckets + (dwHash % _nBuckets); pNode = *ppPreviousNodeNextPointer; - while (pNode != NULL) + while (pNode != nullptr) { if (pNode->_dwHash == dwHash) { @@ -364,10 +364,10 @@ HASH_TABLE<_Record,_Key>::FindNodeInternal( pNode = *ppPreviousNodeNextPointer; } - __analysis_assume( (pNode == NULL && fFound == FALSE) || - (pNode != NULL && fFound == TRUE ) ); + __analysis_assume( (pNode == nullptr && fFound == FALSE) || + (pNode != nullptr && fFound == TRUE ) ); *ppNode = pNode; - if (pppPreviousNodeNextPointer != NULL) + if (pppPreviousNodeNextPointer != nullptr) { *pppPreviousNodeNextPointer = ppPreviousNodeNextPointer; } @@ -381,16 +381,19 @@ HASH_TABLE<_Record,_Key>::FindKey( _Record ** ppRecord ) { - HASH_NODE<_Record> *pNode; + HASH_NODE<_Record> *pNode = nullptr; - *ppRecord = NULL; + *ppRecord = nullptr; - DWORD dwHash = CalcKeyHash(key); + const DWORD dwHash = CalcKeyHash(key); _tableLock.SharedAcquire(); + // Dereferencing NULL pointer 'pNode' + // FindNodeInternal will set a non-null pNode when true is returned, just need to figure out how to correctly SAL the method +#pragma warning(suppress: 6011) if (FindNodeInternal(key, dwHash, &pNode) && - pNode->_pRecord != NULL) + pNode->_pRecord != nullptr) { ReferenceRecord(pNode->_pRecord); *ppRecord = pNode->_pRecord; @@ -419,11 +422,11 @@ HASH_TABLE<_Record,_Key>::InsertRecord( { BOOL fLocked = FALSE; _Key key = ExtractKey(pRecord); - DWORD dwHash = CalcKeyHash(key); + const DWORD dwHash = CalcKeyHash(key); HRESULT hr = S_OK; - HASH_NODE<_Record> * pNewNode; - HASH_NODE<_Record> * pNextNode; - HASH_NODE<_Record> ** ppPreviousNodeNextPointer; + HASH_NODE<_Record> * pNewNode = nullptr; + HASH_NODE<_Record> * pNextNode = nullptr; + HASH_NODE<_Record> ** ppPreviousNodeNextPointer = nullptr; // // Ownership of pRecord is not transferred to pNewNode yet, so remember @@ -432,7 +435,7 @@ HASH_TABLE<_Record,_Key>::InsertRecord( // which users may view as getting flushed out of the hash-table // pNewNode = new HASH_NODE<_Record>(pRecord, dwHash); - if (pNewNode == NULL) + if (pNewNode == nullptr) { hr = HRESULT_FROM_WIN32(ERROR_NOT_ENOUGH_MEMORY); goto Finished; @@ -451,7 +454,7 @@ HASH_TABLE<_Record,_Key>::InsertRecord( // // If node already there, return error // - pNewNode->_pRecord = NULL; + pNewNode->_pRecord = nullptr; DeleteNode(pNewNode); // @@ -470,10 +473,10 @@ HASH_TABLE<_Record,_Key>::InsertRecord( pNewNode, pNextNode) != pNextNode); // pass ownership of pRecord now - if (pRecord != NULL) + if (pRecord != nullptr) { ReferenceRecord(pRecord); - pRecord = NULL; + pRecord = nullptr; } InterlockedIncrement((LONG *)&_nItems); @@ -498,15 +501,18 @@ HASH_TABLE<_Record,_Key>::DeleteKey( _Key key ) { - HASH_NODE<_Record> *pNode; - HASH_NODE<_Record> **ppPreviousNodeNextPointer; + HASH_NODE<_Record> *pNode = nullptr; + HASH_NODE<_Record> **ppPreviousNodeNextPointer = nullptr; - DWORD dwHash = CalcKeyHash(key); + const DWORD dwHash = CalcKeyHash(key); _tableLock.ExclusiveAcquire(); if (FindNodeInternal(key, dwHash, &pNode, &ppPreviousNodeNextPointer)) { + // Dereferencing NULL pointer 'pNode' + // FindNodeInternal will set a non-null pNode when true is returned, just need to figure out how to correctly SAL the method +#pragma warning(suppress: 6011) *ppPreviousNodeNextPointer = pNode->_pNext; DeleteNode(pNode); _nItems--; @@ -522,8 +528,8 @@ HASH_TABLE<_Record,_Key>::DeleteIf( PVOID pvContext ) { - HASH_NODE<_Record> *pNode; - HASH_NODE<_Record> **ppPreviousNodeNextPointer; + HASH_NODE<_Record> *pNode = nullptr; + HASH_NODE<_Record> **ppPreviousNodeNextPointer = nullptr; _tableLock.ExclusiveAcquire(); @@ -531,7 +537,7 @@ HASH_TABLE<_Record,_Key>::DeleteIf( { ppPreviousNodeNextPointer = _ppBuckets + i; pNode = *ppPreviousNodeNextPointer; - while (pNode != NULL) + while (pNode != nullptr) { // // Non empty nodes deleted based on DeleteIf, empty nodes deleted @@ -569,9 +575,9 @@ HASH_TABLE<_Record,_Key>::Apply( for (DWORD i=0; i<_nBuckets; i++) { pNode = _ppBuckets[i]; - while (pNode != NULL) + while (pNode != nullptr) { - if (pNode->_pRecord != NULL) + if (pNode->_pRecord != nullptr) { pfnApply(pNode->_pRecord, pvContext); } @@ -589,13 +595,13 @@ HASH_TABLE<_Record,_Key>::RehashTableIfNeeded( VOID ) { - HASH_NODE<_Record> **ppBuckets; - DWORD nBuckets; - HASH_NODE<_Record> *pNode; - HASH_NODE<_Record> *pNextNode; - HASH_NODE<_Record> **ppNextPointer; - HASH_NODE<_Record> *pNewNextNode; - DWORD nNewBuckets; + HASH_NODE<_Record> **ppBuckets = nullptr; + DWORD nBuckets{}; + HASH_NODE<_Record> *pNode = nullptr; + HASH_NODE<_Record> *pNextNode = nullptr; + HASH_NODE<_Record> **ppNextPointer = nullptr; + HASH_NODE<_Record> *pNewNextNode = nullptr; + DWORD nNewBuckets{}; // // If number of items has become too many, we will double the hash table @@ -624,7 +630,7 @@ HASH_TABLE<_Record,_Key>::RehashTableIfNeeded( GetProcessHeap(), HEAP_ZERO_MEMORY, nBuckets*sizeof(HASH_NODE<_Record> *)); - if (ppBuckets == NULL) + if (ppBuckets == nullptr) { goto Finished; } @@ -636,13 +642,13 @@ HASH_TABLE<_Record,_Key>::RehashTableIfNeeded( for (DWORD i=0; i<_nBuckets; i++) { pNode = _ppBuckets[i]; - while (pNode != NULL) + while (pNode != nullptr) { pNextNode = pNode->_pNext; ppNextPointer = ppBuckets + (pNode->_dwHash % nBuckets); pNewNextNode = *ppNextPointer; - while (pNewNextNode != NULL && + while (pNewNextNode != nullptr && pNewNextNode->_dwHash <= pNode->_dwHash) { ppNextPointer = &pNewNextNode->_pNext; @@ -658,7 +664,7 @@ HASH_TABLE<_Record,_Key>::RehashTableIfNeeded( HeapFree(GetProcessHeap(), 0, _ppBuckets); _ppBuckets = ppBuckets; _nBuckets = nBuckets; - ppBuckets = NULL; + ppBuckets = nullptr; Finished: diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/prime.h b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/prime.h index 6a6a88ed780e..0b308aa0002b 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/prime.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/prime.h @@ -33,7 +33,7 @@ class PRIME // for ( DWORD Index = 0; Index < _countof( g_Primes ); Index++ ) { - DWORD dwCandidate = g_Primes[Index]; + const DWORD dwCandidate = g_Primes[Index]; if ( dwCandidate >= dwMinimum ) { return dwCandidate; @@ -68,7 +68,7 @@ class PRIME return ( dwCandidate == 2 ); } - DWORD dwMax = static_cast(sqrt(static_cast(dwCandidate))); + const DWORD dwMax = static_cast(sqrt(static_cast(dwCandidate))); for ( DWORD Index = 3; Index <= dwMax; Index += 2 ) { @@ -82,4 +82,4 @@ class PRIME PRIME() {} ~PRIME() {} -}; \ No newline at end of file +}; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/rwlock.h b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/rwlock.h index dc7ccf834bef..c7e236ca5940 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/rwlock.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/rwlock.h @@ -160,21 +160,25 @@ class CWSDRWLock return S_OK; } + _Acquires_shared_lock_(this->m_rwLock) void SharedAcquire() { AcquireSRWLockShared(&m_rwLock); } - + + _Releases_shared_lock_(this->m_rwLock) void SharedRelease() { ReleaseSRWLockShared(&m_rwLock); } + _Acquires_exclusive_lock_(this->m_rwLock) void ExclusiveAcquire() { AcquireSRWLockExclusive(&m_rwLock); } - + + _Releases_exclusive_lock_(this->m_rwLock) void ExclusiveRelease() { ReleaseSRWLockExclusive(&m_rwLock); @@ -190,4 +194,4 @@ class CWSDRWLock // // Rename the lock class to a more clear name. // -typedef CWSDRWLock READ_WRITE_LOCK; \ No newline at end of file +typedef CWSDRWLock READ_WRITE_LOCK; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.cpp b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.cpp index 8e2cbfe9caf0..5e878377fa71 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.cpp @@ -135,6 +135,12 @@ STRA::QueryStr( return m_Buff.QueryPtr(); } +VOID STRA::EnsureNullTerminated() +{ + // m_cchLen represents the strings length, not the underlying buffer length + m_Buff.QueryPtr()[m_cchLen] = '\0'; +} + VOID STRA::Reset( ) @@ -695,7 +701,7 @@ Return Value: _ASSERTE( pch ); - while (pch[i] != NULL) + while ((DWORD)i < m_cchLen && pch[i] != NULL) { // // Escape characters that are in the non-printable range @@ -1153,7 +1159,7 @@ STRA::AuxAppendW( // ensure we're still NULL terminated in the right spot // (regardless of success or failure) // - QueryStr()[m_cchLen] = '\0'; + EnsureNullTerminated(); return hr; } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.h b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.h index 0d1f50d50c93..f58971b48444 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/stringa.h @@ -143,6 +143,9 @@ class STRA QueryStr( ) const; + + VOID EnsureNullTerminated(); + VOID Reset( ); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/util.cpp b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/util.cpp index 214ee65abfe4..37689befec92 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/IISLib/util.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/IISLib/util.cpp @@ -47,6 +47,9 @@ Return Values: // we need to change it to Win32 from ("\\?\") // + // Buffer overrun while writing to 'pstrPath->QueryStr()' + // We know it's not an overrun because we just copied pszName into pstrPath and pszName is at least 4 in length +#pragma warning(suppress: 6386) pstrPath->QueryStr()[2] = L'?'; } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h index d3e50e620fa8..2793c7c404f2 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/StartupExceptionApplication.h @@ -24,7 +24,7 @@ class StartupExceptionApplication : public InProcessApplicationBase m_error(errorPageContent), m_statusCode(statusCode), m_subStatusCode(subStatusCode), - m_statusText(std::move(statusText)), + m_statusText(statusText), InProcessApplicationBase(pServer, pApplication) { } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp index 3040e06924a5..a321ea1e5fb2 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cpp @@ -85,6 +85,7 @@ BOOL APIENTRY DllMain(HMODULE hModule, IN_PROCESS_HANDLER::StaticTerminate(); ALLOC_CACHE_HANDLER::StaticTerminate(); DebugStop(); + break; default: break; } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp index c510ecdb7f97..a4ce6ccfc833 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp @@ -14,7 +14,7 @@ #include "Environment.h" #include "HostFxr.h" -IN_PROCESS_APPLICATION* IN_PROCESS_APPLICATION::s_Application = NULL; +IN_PROCESS_APPLICATION* IN_PROCESS_APPLICATION::s_Application = nullptr; IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( IHttpServer& pHttpServer, @@ -28,7 +28,8 @@ IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( m_blockManagedCallbacks(true), m_waitForShutdown(true), m_pConfig(std::move(pConfig)), - m_requestCount(0) + m_requestCount(0), + m_AsyncCompletionHandler(nullptr) { DBG_ASSERT(m_pConfig); @@ -317,7 +318,7 @@ IN_PROCESS_APPLICATION::ExecuteApplication() ForwardingRedirectionOutput redirectionForwarder(&context->m_redirectionOutput); const auto redirect = context->m_hostFxr.RedirectOutput(&redirectionForwarder); - auto startupReturnCode = context->m_hostFxr.InitializeForApp(context->m_argc, context->m_argv.get(), m_dotnetExeKnownLocation); + const auto startupReturnCode = context->m_hostFxr.InitializeForApp(context->m_argc, context->m_argv.get(), m_dotnetExeKnownLocation); if (startupReturnCode != 0) { auto content = m_stringRedirectionOutput->GetOutput(); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp index 23639860527a..193e66bc8b6b 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/managedexports.cpp @@ -117,6 +117,8 @@ http_get_server_variable( PCWSTR pszVariableValue; DWORD cbLength; + *pwszReturn = nullptr; + HRESULT hr = pInProcessHandler ->QueryHttpContext() ->GetServerVariable(pszVariableName, &pszVariableValue, &cbLength); @@ -276,6 +278,7 @@ http_read_request_bytes( ) { HRESULT hr = S_OK; + *pvBuffer = 0; if (pInProcessHandler == NULL) { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp index a89165dd72b4..6aa55e6c9bb8 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cpp @@ -219,6 +219,7 @@ BOOL APIENTRY DllMain(HMODULE hModule, FORWARDING_HANDLER::StaticTerminate(); ALLOC_CACHE_HANDLER::StaticTerminate(); DebugStop(); + break; default: break; } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp index d30883ea5332..21478cc03b5b 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.cpp @@ -262,7 +262,7 @@ FORWARDING_HANDLER::ExecuteRequestHandler() NULL, 0, cbContentLength, - reinterpret_cast(static_cast(this)))) + reinterpret_cast(this))) { hr = HRESULT_FROM_WIN32(GetLastError()); @@ -576,7 +576,7 @@ REQUEST_NOTIFICATION_STATUS strDescription.QueryStr(), strDescription.QuerySizeCCH()); } - (VOID)strDescription.SyncWithBuffer(); + std::ignore = strDescription.SyncWithBuffer(); if (strDescription.QueryCCH() != 0) { @@ -1495,6 +1495,8 @@ FORWARDING_HANDLER::OnWinHttpCompletionSendRequestOrWriteComplete( HRESULT hr = S_OK; IHttpRequest * pRequest = m_pW3Context->GetRequest(); + *pfClientError = FALSE; + // // completion for sending the initial request or request entity to // winhttp, get more request entity if available, else start receiving @@ -1596,7 +1598,7 @@ FORWARDING_HANDLER::OnWinHttpCompletionStatusHeadersAvailable( STACK_STRA(strHeaders, 2048); DWORD dwHeaderSize = bufHeaderBuffer.QuerySize(); - UNREFERENCED_PARAMETER(pfAnotherCompletionExpected); + *pfAnotherCompletionExpected = FALSE; // // Headers are available, read the status line and headers and pass @@ -1691,6 +1693,8 @@ FORWARDING_HANDLER::OnWinHttpCompletionStatusDataAvailable( { HRESULT hr = S_OK; + *pfAnotherCompletionExpected = FALSE; + // // Response data is available from winhttp, read it // @@ -1744,6 +1748,8 @@ FORWARDING_HANDLER::OnWinHttpCompletionStatusReadComplete( { HRESULT hr = S_OK; + *pfAnotherCompletionExpected = FALSE; + // // Response data has been read from winhttp, send it to the client // @@ -1820,6 +1826,8 @@ FORWARDING_HANDLER::OnSendingRequest( __out BOOL * pfClientError ) { + *pfClientError = FALSE; + // // This is a completion for a read from http.sys, abort in case // of failure, if we read anything write it out over WinHTTP, @@ -2091,7 +2099,7 @@ FORWARDING_HANDLER::SetStatusAndHeaders( // RETURN_IF_FAILED(strHeaderValue.Copy( pchStatus, - (DWORD)(pchEndofHeaderValue - pchStatus) + 1)); + (SIZE_T)(pchEndofHeaderValue - pchStatus) + 1)); RETURN_IF_FAILED(pResponse->SetStatus(uStatus, strHeaderValue.QueryStr(), 0, @@ -2126,6 +2134,10 @@ FORWARDING_HANDLER::SetStatusAndHeaders( pchNewline[1] == '\t') { pchNewline = strchr(pchNewline + 1, '\n'); + if (pchNewline == NULL) + { + return HRESULT_FROM_WIN32(ERROR_INVALID_PARAMETER); + } } DBG_ASSERT( @@ -2405,7 +2417,7 @@ FORWARDING_HANDLER::DoReverseRewrite( { RETURN_HR(E_OUTOFMEMORY); } - StringCchCopyA(const_cast(pszHeader), strTemp.QueryCCH() + 1, strTemp.QueryStr()); + StringCchCopyA(const_cast(pszHeader), (size_t)strTemp.QueryCCH() + 1, strTemp.QueryStr()); pHeaders->pUnknownHeaders[i].pRawValue = pszHeader; pHeaders->pUnknownHeaders[i].RawValueLength = static_cast(strTemp.QueryCCH()); @@ -2460,6 +2472,7 @@ FORWARDING_HANDLER::NotifyDisconnect() } } +_Acquires_exclusive_lock_(this->m_RequestLock) VOID FORWARDING_HANDLER::AcquireLockExclusive() { @@ -2469,6 +2482,7 @@ FORWARDING_HANDLER::AcquireLockExclusive() DBG_ASSERT(TlsGetValue(g_dwTlsIndex) == this); } +_Releases_exclusive_lock_(this->m_RequestLock) VOID FORWARDING_HANDLER::ReleaseLockExclusive() { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.h b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.h index c564a7b4f03a..8e5c5c74d250 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/forwardinghandler.h @@ -77,9 +77,11 @@ class FORWARDING_HANDLER : public REQUEST_HANDLER private: + _Acquires_exclusive_lock_(this->m_RequestLock) VOID AcquireLockExclusive(); + _Releases_exclusive_lock_(this->m_RequestLock) VOID ReleaseLockExclusive(); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/serverprocess.cpp b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/serverprocess.cpp index 52146c27d5cb..1a98dfefd88d 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/serverprocess.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/serverprocess.cpp @@ -171,7 +171,7 @@ SERVER_PROCESS::SetupListenPort( goto Finished; } - if (swprintf_s(buffer, 15, L"%d", m_dwPort) <= 0) + if (swprintf_s(buffer, 15, L"%u", m_dwPort) <= 0) { hr = E_INVALIDARG; goto Finished; @@ -360,7 +360,7 @@ SERVER_PROCESS::OutputEnvironmentVariables pszEqualChar = wcschr(pszCurrentVariable, L'='); if (pszEqualChar != NULL) { - if (FAILED_LOG(hr = strEnvVar.Copy(pszCurrentVariable, (DWORD)(pszEqualChar - pszCurrentVariable) + 1))) + if (FAILED_LOG(hr = strEnvVar.Copy(pszCurrentVariable, (SIZE_T)(pszEqualChar - pszCurrentVariable) + 1))) { goto Finished; } @@ -471,7 +471,7 @@ SERVER_PROCESS::SetupCommandLine( Finished: if (pszFullPath != NULL) { - delete pszFullPath; + delete[] pszFullPath; } return hr; } @@ -963,6 +963,7 @@ SERVER_PROCESS::SetWindowsAuthToken( ) { HRESULT hr = S_OK; + *pTargetTokenHandle = nullptr; if (m_hListeningProcessHandle != NULL && m_hListeningProcessHandle != INVALID_HANDLE_VALUE) { @@ -1300,6 +1301,8 @@ SERVER_PROCESS::SendSignal( DWORD dwThreadStatus = 0; if (GetExitCodeThread(hThread, &dwThreadStatus)!= 0 && dwThreadStatus == STILL_ACTIVE) { + // Using TerminateThread does not allow proper thread clean up. +#pragma warning(suppress: 6258) TerminateThread(hThread, STATUS_CONTROL_C_EXIT); } CloseHandle(hThread); @@ -1834,6 +1837,8 @@ SERVER_PROCESS::~SERVER_PROCESS() dwThreadStatus == STILL_ACTIVE) { LOG_WARN(L"Thread reading stdout/err hit timeout, forcibly closing thread."); + // Using TerminateThread does not allow proper thread clean up. +#pragma warning(suppress: 6258) TerminateThread(m_hReadThread, STATUS_CONTROL_C_EXIT); } } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.cpp b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.cpp index 8f6c5d37204e..cc3720003435 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.cpp @@ -69,6 +69,10 @@ WINHTTP_HELPER::GetFlagsFromBufferType( __out BOOL * pfClose ) { + *pfClose = FALSE; + *pfFinalFragment = FALSE; + *pfUtf8Encoded = FALSE; + switch (BufferType) { case WINHTTP_WEB_SOCKET_BINARY_MESSAGE_BUFFER_TYPE: diff --git a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.h b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.h index d583f6fb1032..a8746c377466 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/OutOfProcessRequestHandler/winhttphelper.h @@ -7,7 +7,7 @@ typedef HINTERNET (WINAPI * PFN_WINHTTP_WEBSOCKET_COMPLETE_UPGRADE)( _In_ HINTERNET hRequest, - _In_opt_ DWORD_PTR pContext + DWORD_PTR pContext ); @@ -88,4 +88,4 @@ class WINHTTP_HELPER static PFN_WINHTTP_WEBSOCKET_QUERY_CLOSE_STATUS sm_pfnWinHttpWebSocketQueryCloseStatus; -}; \ No newline at end of file +}; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp index c7079d63417e..f3cc0266f604 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/AppOfflineTrackingApplication.cpp @@ -9,7 +9,7 @@ HRESULT AppOfflineTrackingApplication::StartMonitoringAppOffline() { LOG_INFOF(L"Starting app_offline monitoring in application '%ls'", m_applicationPath.c_str()); - HRESULT hr = StartMonitoringAppOflineImpl(); + const HRESULT hr = StartMonitoringAppOflineImpl(); if (FAILED_LOG(hr)) { diff --git a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/environmentvariablehash.h b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/environmentvariablehash.h index 248ca1aa9354..cf36d892bcaf 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/environmentvariablehash.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/environmentvariablehash.h @@ -89,7 +89,7 @@ class ENVIRONMENT_VAR_HASH : public HASH_TABLE PWSTR ExtractKey( ENVIRONMENT_VAR_ENTRY * pEntry - ) + ) override { return pEntry->QueryName(); } @@ -97,7 +97,7 @@ class ENVIRONMENT_VAR_HASH : public HASH_TABLE DWORD CalcKeyHash( PWSTR pszName - ) + ) override { return HashStringNoCase(pszName); } @@ -106,7 +106,7 @@ class ENVIRONMENT_VAR_HASH : public HASH_TABLE EqualKeys( PWSTR pszName1, PWSTR pszName2 - ) + ) override { return (_wcsicmp(pszName1, pszName2) == 0); } @@ -114,7 +114,7 @@ class ENVIRONMENT_VAR_HASH : public HASH_TABLE VOID ReferenceRecord( ENVIRONMENT_VAR_ENTRY * pEntry - ) + ) override { pEntry->Reference(); } @@ -122,7 +122,7 @@ class ENVIRONMENT_VAR_HASH : public HASH_TABLE VOID DereferenceRecord( ENVIRONMENT_VAR_ENTRY * pEntry - ) + ) override { pEntry->Dereference(); } diff --git a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.cpp b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.cpp index 4f0229dc5c3c..3c578e2a6759 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.cpp @@ -13,7 +13,9 @@ FILE_WATCHER::FILE_WATCHER() : m_hChangeNotificationThread(nullptr), m_fThreadExit(false), m_fShadowCopyEnabled(false), - m_copied(false) + m_copied(false), + _overlapped(), + m_shutdownTimeout(0) { m_pDoneCopyEvent = CreateEvent( nullptr, // default security attributes @@ -50,13 +52,13 @@ void FILE_WATCHER::WaitForWatcherThreadExit() { // This is the old behavior, which is now opt-in using an environment variable. Wait for // the thread to exit, but if it doesn't exit soon enough, terminate it. - const int totalWaitTimeMs = 10000; - const int waitIntervalMs = 50; - const int iterations = totalWaitTimeMs / waitIntervalMs; + constexpr int totalWaitTimeMs = 10000; + constexpr int waitIntervalMs = 50; + constexpr int iterations = totalWaitTimeMs / waitIntervalMs; for (int i = 0; i < iterations && !m_fThreadExit; i++) { // Check if the thread has exited. - DWORD result = WaitForSingleObject(m_hChangeNotificationThread, waitIntervalMs); + const DWORD result = WaitForSingleObject(m_hChangeNotificationThread, waitIntervalMs); if (result == WAIT_OBJECT_0) { // The thread has exited. @@ -68,6 +70,9 @@ void FILE_WATCHER::WaitForWatcherThreadExit() if (!m_fThreadExit) { LOG_INFO(L"File watcher thread did not exit. Forcing termination."); + // Using TerminateThread does not allow propert thread clean up. + // Off by default, behind a backcompat switch +#pragma warning(suppress: 6258) TerminateThread(m_hChangeNotificationThread, 1); } } @@ -99,7 +104,7 @@ FILE_WATCHER::Create( (LPTHREAD_START_ROUTINE)ChangeNotificationThread, this, 0, - NULL)); + nullptr)); if (pszDirectoryToMonitor == nullptr || pszFileNameToMonitor == nullptr || @@ -128,7 +133,7 @@ FILE_WATCHER::Create( nullptr, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED, - NULL); + nullptr); RETURN_LAST_ERROR_IF(_hDirectory == INVALID_HANDLE_VALUE); @@ -176,7 +181,7 @@ Win32 error OVERLAPPED* pOverlapped = nullptr; ULONG_PTR completionKey; - BOOL success = GetQueuedCompletionStatus( + const BOOL success = GetQueuedCompletionStatus( pFileMonitor->m_hCompletionPort, &bytesTransferred, &completionKey, @@ -272,7 +277,7 @@ HRESULT if (bytesTransferred == 0) { LOG_INFO(L"0 bytes transferred for file notifications. Falling back to manually looking for app_offline."); - DWORD fileAttr = GetFileAttributesW(_strFullName.QueryStr()); + const DWORD fileAttr = GetFileAttributesW(_strFullName.QueryStr()); if (fileAttr != INVALID_FILE_ATTRIBUTES && !(fileAttr & FILE_ATTRIBUTE_DIRECTORY)) { fAppOfflineChanged = TRUE; @@ -353,7 +358,9 @@ HRESULT return S_OK; } - +// The pointer argument can be marked as a pointer to a const +// This is a callback that's passed to a windows function without const qualifications on the arguments +#pragma warning(suppress: 26461) VOID CALLBACK FILE_WATCHER::TimerCallback( @@ -475,7 +482,7 @@ FILE_WATCHER::StopMonitor() LOG_INFO(L"Stopping file watching."); // Signal the file watcher thread to exit - PostQueuedCompletionStatus(m_hCompletionPort, 0, FILE_WATCHER_SHUTDOWN_KEY, NULL); + PostQueuedCompletionStatus(m_hCompletionPort, 0, FILE_WATCHER_SHUTDOWN_KEY, nullptr); WaitForWatcherThreadExit(); if (m_fShadowCopyEnabled) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.h b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.h index c73a76849703..3c4c01bc80ca 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.h @@ -50,7 +50,7 @@ class FILE_WATCHER{ static DWORD WINAPI CopyAndShutdown(FILE_WATCHER* watcher); - HRESULT HandleChangeCompletion(DWORD cbCompletion); + HRESULT HandleChangeCompletion(_In_ DWORD bytesTransferred); HRESULT Monitor(); void StopMonitor(); diff --git a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp index c32bc51ced79..55399898076d 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.cpp @@ -11,35 +11,35 @@ REQUESTHANDLER_CONFIG::~REQUESTHANDLER_CONFIG() { - if (m_ppStrArguments != NULL) + if (m_ppStrArguments != nullptr) { delete[] m_ppStrArguments; - m_ppStrArguments = NULL; + m_ppStrArguments = nullptr; } } HRESULT REQUESTHANDLER_CONFIG::CreateRequestHandlerConfig( - _In_ IHttpServer *pHttpServer, - _In_ IHttpSite *pSite, - _In_ IHttpApplication *pHttpApplication, - _Out_ REQUESTHANDLER_CONFIG **ppAspNetCoreConfig + const _In_ IHttpServer *pHttpServer, + const _In_ IHttpSite *pSite, + const _In_ IHttpApplication *pHttpApplication, + _Out_ REQUESTHANDLER_CONFIG **ppAspNetCoreConfig ) { HRESULT hr = S_OK; - REQUESTHANDLER_CONFIG *pRequestHandlerConfig = NULL; + REQUESTHANDLER_CONFIG *pRequestHandlerConfig = nullptr; STRU struHostFxrDllLocation; STRU struExeLocation; try { - if (ppAspNetCoreConfig == NULL) + if (ppAspNetCoreConfig == nullptr) { hr = E_INVALIDARG; goto Finished; } - *ppAspNetCoreConfig = NULL; + *ppAspNetCoreConfig = nullptr; pRequestHandlerConfig = new REQUESTHANDLER_CONFIG; @@ -58,7 +58,7 @@ REQUESTHANDLER_CONFIG::CreateRequestHandlerConfig( } *ppAspNetCoreConfig = pRequestHandlerConfig; - pRequestHandlerConfig = NULL; + pRequestHandlerConfig = nullptr; } catch (std::bad_alloc&) { @@ -67,10 +67,10 @@ REQUESTHANDLER_CONFIG::CreateRequestHandlerConfig( Finished: - if (pRequestHandlerConfig != NULL) + if (pRequestHandlerConfig != nullptr) { delete pRequestHandlerConfig; - pRequestHandlerConfig = NULL; + pRequestHandlerConfig = nullptr; } return hr; @@ -78,9 +78,9 @@ REQUESTHANDLER_CONFIG::CreateRequestHandlerConfig( HRESULT REQUESTHANDLER_CONFIG::Populate( - IHttpServer *pHttpServer, - IHttpSite *pSite, - IHttpApplication *pHttpApplication + const IHttpServer *pHttpServer, + const IHttpSite *pSite, + const IHttpApplication *pHttpApplication ) { STACK_STRU(strHostingModel, 300); @@ -89,19 +89,19 @@ REQUESTHANDLER_CONFIG::Populate( STRU strEnvValue; STRU strExpandedEnvValue; STRU strApplicationFullPath; - IAppHostAdminManager *pAdminManager = NULL; - IAppHostElement *pAspNetCoreElement = NULL; - IAppHostElement *pWindowsAuthenticationElement = NULL; - IAppHostElement *pBasicAuthenticationElement = NULL; - IAppHostElement *pAnonymousAuthenticationElement = NULL; + IAppHostAdminManager *pAdminManager = nullptr; + IAppHostElement *pAspNetCoreElement = nullptr; + IAppHostElement *pWindowsAuthenticationElement = nullptr; + IAppHostElement *pBasicAuthenticationElement = nullptr; + IAppHostElement *pAnonymousAuthenticationElement = nullptr; ULONGLONG ullRawTimeSpan = 0; DWORD dwCounter = 0; DWORD dwPosition = 0; - WCHAR* pszPath = NULL; - BSTR bstrWindowAuthSection = NULL; - BSTR bstrBasicAuthSection = NULL; - BSTR bstrAnonymousAuthSection = NULL; - BSTR bstrAspNetCoreSection = NULL; + WCHAR* pszPath = nullptr; + BSTR bstrWindowAuthSection = nullptr; + BSTR bstrBasicAuthSection = nullptr; + BSTR bstrAnonymousAuthSection = nullptr; + BSTR bstrAspNetCoreSection = nullptr; std::optional launcherPathEnv; std::optional launcherArgsEnv; @@ -161,7 +161,7 @@ REQUESTHANDLER_CONFIG::Populate( } bstrWindowAuthSection = SysAllocString(CS_WINDOWS_AUTHENTICATION_SECTION); - if (bstrWindowAuthSection == NULL) + if (bstrWindowAuthSection == nullptr) { hr = E_OUTOFMEMORY; goto Finished; @@ -188,7 +188,7 @@ REQUESTHANDLER_CONFIG::Populate( } bstrBasicAuthSection = SysAllocString(CS_BASIC_AUTHENTICATION_SECTION); - if (bstrBasicAuthSection == NULL) + if (bstrBasicAuthSection == nullptr) { hr = E_OUTOFMEMORY; goto Finished; @@ -213,7 +213,7 @@ REQUESTHANDLER_CONFIG::Populate( } bstrAnonymousAuthSection = SysAllocString(CS_ANONYMOUS_AUTHENTICATION_SECTION); - if (bstrAnonymousAuthSection == NULL) + if (bstrAnonymousAuthSection == nullptr) { hr = E_OUTOFMEMORY; goto Finished; @@ -238,7 +238,7 @@ REQUESTHANDLER_CONFIG::Populate( } bstrAspNetCoreSection = SysAllocString(CS_ASPNETCORE_SECTION); - if (bstrAspNetCoreSection == NULL) + if (bstrAspNetCoreSection == nullptr) { hr = E_OUTOFMEMORY; goto Finished; @@ -423,28 +423,28 @@ REQUESTHANDLER_CONFIG::Populate( Finished: - if (pAspNetCoreElement != NULL) + if (pAspNetCoreElement != nullptr) { pAspNetCoreElement->Release(); - pAspNetCoreElement = NULL; + pAspNetCoreElement = nullptr; } - if (pWindowsAuthenticationElement != NULL) + if (pWindowsAuthenticationElement != nullptr) { pWindowsAuthenticationElement->Release(); - pWindowsAuthenticationElement = NULL; + pWindowsAuthenticationElement = nullptr; } - if (pAnonymousAuthenticationElement != NULL) + if (pAnonymousAuthenticationElement != nullptr) { pAnonymousAuthenticationElement->Release(); - pAnonymousAuthenticationElement = NULL; + pAnonymousAuthenticationElement = nullptr; } - if (pBasicAuthenticationElement != NULL) + if (pBasicAuthenticationElement != nullptr) { pBasicAuthenticationElement->Release(); - pBasicAuthenticationElement = NULL; + pBasicAuthenticationElement = nullptr; } return hr; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.h b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.h index fc5d01b109e0..1938f5c83486 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.h +++ b/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/requesthandler_config.h @@ -60,9 +60,9 @@ class REQUESTHANDLER_CONFIG static HRESULT CreateRequestHandlerConfig( - _In_ IHttpServer *pHttpServer, - _In_ IHttpSite *pSite, - _In_ IHttpApplication *pHttpApplication, + const _In_ IHttpServer *pHttpServer, + const _In_ IHttpSite *pSite, + const _In_ IHttpApplication *pHttpApplication, _Out_ REQUESTHANDLER_CONFIG **ppAspNetCoreConfig ); @@ -236,15 +236,26 @@ class REQUESTHANDLER_CONFIG REQUESTHANDLER_CONFIG() : m_fStdoutLogEnabled(FALSE), m_hostingModel(HOSTING_UNKNOWN), - m_ppStrArguments(NULL) + m_ppStrArguments(nullptr), + m_dwArgc(0), + m_fAnonymousAuthEnabled(false), + m_fDisableStartUpErrorPage(true), + m_dwProcessesPerApplication(0), + m_dwRapidFailsPerMinute(0), + m_dwRequestTimeoutInMS(0), + m_dwShutdownTimeLimitInMS(0), + m_fBasicAuthEnabled(false), + m_fForwardWindowsAuthToken(false), + m_fWindowsAuthEnabled(false), + m_dwStartupTimeLimitInMS(0) { } HRESULT Populate( - IHttpServer *pHttpServer, - IHttpSite *pSite, - IHttpApplication *pHttpApplication + const IHttpServer *pHttpServer, + const IHttpSite *pSite, + const IHttpApplication *pHttpApplication ); DWORD m_dwRequestTimeoutInMS; diff --git a/src/Servers/IIS/AspNetCoreModuleV2/gtest/gtest.vcxproj b/src/Servers/IIS/AspNetCoreModuleV2/gtest/gtest.vcxproj index bf7e421173d9..c02269e206d7 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/gtest/gtest.vcxproj +++ b/src/Servers/IIS/AspNetCoreModuleV2/gtest/gtest.vcxproj @@ -7,6 +7,7 @@ {CAC1267B-8778-4257-AAC6-CAF481723B01} gtest + false gtest diff --git a/src/Servers/IIS/Directory.Build.props b/src/Servers/IIS/Directory.Build.props index e167d783b593..624773c5b877 100644 --- a/src/Servers/IIS/Directory.Build.props +++ b/src/Servers/IIS/Directory.Build.props @@ -8,6 +8,6 @@ x64 $(NetCoreTargetingPackRoot)Microsoft.NETCore.App.Host.win-$(HostArch)\$(LibNetHostAppPackVersion)\runtimes\win-$(HostArch)\native - v142 + 143 diff --git a/src/Servers/IIS/build/Build.Common.Settings b/src/Servers/IIS/build/Build.Common.Settings index 196e069eafa4..cebc071c6d4c 100644 --- a/src/Servers/IIS/build/Build.Common.Settings +++ b/src/Servers/IIS/build/Build.Common.Settings @@ -32,6 +32,12 @@ + + ..\DefaultRules.ruleset + true + $(RunCodeAnalysis) + + Use