From f5f0988bafa80c3db498fe1cb56ec30560787d60 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 29 Jun 2018 08:44:03 -0700 Subject: [PATCH 1/8] Make handlerSettings optional (#989) --- .../AspNetCore/src/aspnetcore_shim_config.cpp | 5 ++++- src/AspNetCoreModuleV2/CommonLib/config_utility.h | 7 ++++++- src/AspNetCoreModuleV2/CommonLib/debugutil.cpp | 8 ++++---- src/AspNetCoreModuleV2/CommonLib/utility.cxx | 8 +------- test/CommonLibTests/ConfigUtilityTests.cpp | 14 ++++++++++++++ 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/AspNetCoreModuleV2/AspNetCore/src/aspnetcore_shim_config.cpp b/src/AspNetCoreModuleV2/AspNetCore/src/aspnetcore_shim_config.cpp index 0cfc9b4133..9fb17be078 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/src/aspnetcore_shim_config.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/src/aspnetcore_shim_config.cpp @@ -56,7 +56,10 @@ ASPNETCORE_SHIM_CONFIG::Populate( CS_ASPNETCORE_PROCESS_ARGUMENTS, &m_struArguments)); - RETURN_IF_FAILED(ConfigUtility::FindHandlerVersion(pAspNetCoreElement, &m_struHandlerVersion)); + if (m_hostingModel == HOSTING_OUT_PROCESS) + { + RETURN_IF_FAILED(ConfigUtility::FindHandlerVersion(pAspNetCoreElement, &m_struHandlerVersion)); + } return S_OK; } diff --git a/src/AspNetCoreModuleV2/CommonLib/config_utility.h b/src/AspNetCoreModuleV2/CommonLib/config_utility.h index 78efe9a04b..5585d14614 100644 --- a/src/AspNetCoreModuleV2/CommonLib/config_utility.h +++ b/src/AspNetCoreModuleV2/CommonLib/config_utility.h @@ -28,7 +28,12 @@ public: STRU strHandlerName; STRU strHandlerValue; - RETURN_IF_FAILED(GetElementChildByName(pElement, CS_ASPNETCORE_HANDLER_SETTINGS,&pHandlerSettings)); + // backwards complatibility with systems not having schema for handlerSettings + if (FAILED_LOG(GetElementChildByName(pElement, CS_ASPNETCORE_HANDLER_SETTINGS, &pHandlerSettings))) + { + return S_OK; + } + RETURN_IF_FAILED(pHandlerSettings->get_Collection(&pHandlerSettingsCollection)); RETURN_IF_FAILED(hr = FindFirstElement(pHandlerSettingsCollection, &index, &pHandlerVar)); diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp index d3d6fa6b32..b7fff04a5c 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp @@ -67,19 +67,18 @@ DebugInitialize() try { - const auto debugOutputFile = Environment::ExpandEnvironmentVariables(L"%ASPNETCORE_MODULE_DEBUG_FILE%"); + auto debugOutputFile = Environment::ExpandEnvironmentVariables(L"%ASPNETCORE_MODULE_DEBUG_FILE%"); if (!debugOutputFile.empty()) { g_logFile = CreateFileW(debugOutputFile.c_str(), - FILE_GENERIC_WRITE, - FILE_SHARE_READ | FILE_SHARE_WRITE, + (GENERIC_READ | GENERIC_WRITE), + (FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE), nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr ); - if (g_logFile != INVALID_HANDLE_VALUE) { InitializeSRWLock(&g_logFileLock); @@ -145,6 +144,7 @@ DebugPrint( SetFilePointer(g_logFile, 0, nullptr, FILE_END); WriteFile(g_logFile, strOutput.QueryStr(), strOutput.QueryCB(), &nBytesWritten, nullptr); + FlushFileBuffers(g_logFile); } } } diff --git a/src/AspNetCoreModuleV2/CommonLib/utility.cxx b/src/AspNetCoreModuleV2/CommonLib/utility.cxx index b98cc24359..a82a7d18b3 100644 --- a/src/AspNetCoreModuleV2/CommonLib/utility.cxx +++ b/src/AspNetCoreModuleV2/CommonLib/utility.cxx @@ -572,13 +572,7 @@ UTILITY::LogEvent( ); } - STACK_STRA(converted, 256); - if (converted.CopyW(pstrMsg)) - { - DebugPrintf( - dwEventInfoType == EVENTLOG_ERROR_TYPE ? ASPNETCORE_DEBUG_FLAG_ERROR : ASPNETCORE_DEBUG_FLAG_INFO, - "Event Log: %s", converted.QueryStr()); - } + WDebugPrintf(dwEventInfoType == EVENTLOG_ERROR_TYPE ? ASPNETCORE_DEBUG_FLAG_ERROR : ASPNETCORE_DEBUG_FLAG_INFO, L"Event Log: %s", pstrMsg); } VOID diff --git a/test/CommonLibTests/ConfigUtilityTests.cpp b/test/CommonLibTests/ConfigUtilityTests.cpp index 99ee323f43..dac87240e1 100644 --- a/test/CommonLibTests/ConfigUtilityTests.cpp +++ b/test/CommonLibTests/ConfigUtilityTests.cpp @@ -89,4 +89,18 @@ namespace ConfigUtilityTests EXPECT_EQ(hr, S_OK); EXPECT_STREQ(handlerVersion.QueryStr(), L"value2"); } + + TEST(ConfigUtilityTestSingle, IgnoresFailedGetElement) + { + STRU handlerVersion; + + auto element = std::make_unique>(); + ON_CALL(*element, GetElementByName(_, _)) + .WillByDefault(DoAll(testing::SetArgPointee<1>(nullptr), testing::Return(HRESULT_FROM_WIN32( ERROR_INVALID_INDEX )))); + + HRESULT hr = ConfigUtility::FindHandlerVersion(element.get(), &handlerVersion); + + EXPECT_EQ(hr, S_OK); + EXPECT_STREQ(handlerVersion.QueryStr(), L""); + } } From dfed3d7563726d1fbd2bf528aae2d0012ee63d95 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 29 Jun 2018 11:17:22 -0700 Subject: [PATCH 2/8] Add and use GetEnvironmentVariableValue (#997) --- .../CommonLib/Environment.cpp | 30 ++++++++++++++++++- .../CommonLib/Environment.h | 3 ++ .../CommonLib/debugutil.cpp | 8 ++--- 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/AspNetCoreModuleV2/CommonLib/Environment.cpp b/src/AspNetCoreModuleV2/CommonLib/Environment.cpp index 9746fe2c9b..316319207c 100644 --- a/src/AspNetCoreModuleV2/CommonLib/Environment.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/Environment.cpp @@ -18,7 +18,7 @@ Environment::ExpandEnvironmentVariables(const std::wstring & str) do { expandedStr.resize(requestedSize); - requestedSize = ExpandEnvironmentStringsW(str.c_str(), &expandedStr[0], requestedSize); + requestedSize = ExpandEnvironmentStringsW(str.c_str(), expandedStr.data(), requestedSize); if (requestedSize == 0) { throw std::system_error(GetLastError(), std::system_category(), "ExpandEnvironmentVariables"); @@ -30,3 +30,31 @@ Environment::ExpandEnvironmentVariables(const std::wstring & str) return expandedStr; } + +std::optional +Environment::GetEnvironmentVariableValue(const std::wstring & str) +{ + DWORD requestedSize = GetEnvironmentVariableW(str.c_str(), nullptr, 0); + if (requestedSize == 0) + { + if (GetLastError() == ERROR_ENVVAR_NOT_FOUND) + { + return std::nullopt; + } + + throw std::system_error(GetLastError(), std::system_category(), "GetEnvironmentVariableW"); + } + + std::wstring expandedStr; + do + { + expandedStr.resize(requestedSize); + requestedSize = GetEnvironmentVariableW(str.c_str(), expandedStr.data(), requestedSize); + if (requestedSize == 0) + { + throw std::system_error(GetLastError(), std::system_category(), "ExpandEnvironmentStringsW"); + } + } while (expandedStr.size() != requestedSize + 1); + + return expandedStr; +} diff --git a/src/AspNetCoreModuleV2/CommonLib/Environment.h b/src/AspNetCoreModuleV2/CommonLib/Environment.h index 969833d842..16022e18cf 100644 --- a/src/AspNetCoreModuleV2/CommonLib/Environment.h +++ b/src/AspNetCoreModuleV2/CommonLib/Environment.h @@ -4,6 +4,7 @@ #pragma once #include +#include class Environment { @@ -13,5 +14,7 @@ public: static std::wstring ExpandEnvironmentVariables(const std::wstring & str); + static + std::optional GetEnvironmentVariableValue(const std::wstring & str); }; diff --git a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp index b7fff04a5c..189a5097f9 100644 --- a/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/debugutil.cpp @@ -53,7 +53,7 @@ DebugInitialize() try { - const auto value = std::stoi(Environment::ExpandEnvironmentVariables(L"%ASPNETCORE_MODULE_DEBUG%")); + const auto value = std::stoi(Environment::GetEnvironmentVariableValue(L"ASPNETCORE_MODULE_DEBUG").value_or(L"0")); if (value >= 1) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_ERROR; if (value >= 2) DEBUG_FLAGS_VAR |= ASPNETCORE_DEBUG_FLAG_WARNING; @@ -67,11 +67,11 @@ DebugInitialize() try { - auto debugOutputFile = Environment::ExpandEnvironmentVariables(L"%ASPNETCORE_MODULE_DEBUG_FILE%"); + const auto debugOutputFile = Environment::GetEnvironmentVariableValue(L"ASPNETCORE_MODULE_DEBUG_FILE"); - if (!debugOutputFile.empty()) + if (debugOutputFile.has_value()) { - g_logFile = CreateFileW(debugOutputFile.c_str(), + g_logFile = CreateFileW(debugOutputFile.value().c_str(), (GENERIC_READ | GENERIC_WRITE), (FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE), nullptr, From 65d3787fc483959166d9ecc75d392dcee7e90a2a Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Fri, 29 Jun 2018 12:42:00 -0700 Subject: [PATCH 3/8] Pass parameters to createapplication method (#998) --- .../AspNetCore/Inc/applicationinfo.h | 8 +++++--- .../AspNetCore/src/applicationinfo.cpp | 13 +++++++++++-- .../CommonLib/application.h | 10 ---------- .../CommonLib/iapplication.h | 12 ++++++------ .../InProcessRequestHandler/dllmain.cxx | 16 +++++++++------- .../inprocessapplication.cpp | 17 ++++++++++++++--- .../inprocessapplication.h | 19 ++++--------------- .../OutOfProcessRequestHandler/dllmain.cxx | 4 ++++ .../inprocess_application_tests.cpp | 13 ++++++++----- .../Inprocess/StartupTests.cs | 4 ++++ 10 files changed, 65 insertions(+), 51 deletions(-) diff --git a/src/AspNetCoreModuleV2/AspNetCore/Inc/applicationinfo.h b/src/AspNetCoreModuleV2/AspNetCore/Inc/applicationinfo.h index 3d9073d013..f6f5890b8e 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/Inc/applicationinfo.h +++ b/src/AspNetCoreModuleV2/AspNetCore/Inc/applicationinfo.h @@ -20,9 +20,11 @@ typedef HRESULT (WINAPI * PFN_ASPNETCORE_CREATE_APPLICATION)( - _In_ IHttpServer *pServer, - _In_ IHttpApplication *pHttpApplication, - _Out_ IAPPLICATION **pApplication + _In_ IHttpServer *pServer, + _In_ IHttpApplication *pHttpApplication, + _In_ APPLICATION_PARAMETER *pParameters, + _In_ DWORD nParameters, + _Out_ IAPPLICATION **pApplication ); extern BOOL g_fRecycleProcessCalled; diff --git a/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp b/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp index eca9c56448..ccbd586f3e 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp @@ -3,6 +3,7 @@ #include "applicationinfo.h" +#include #include "proxymodule.h" #include "hostfxr_utility.h" #include "utility.h" @@ -222,8 +223,16 @@ APPLICATION_INFO::EnsureApplicationCreated( FINISHED(HRESULT_FROM_WIN32(ERROR_INVALID_FUNCTION)); } - FINISHED_IF_FAILED(m_pfnAspNetCoreCreateApplication(m_pServer, pHttpContext->GetApplication(), &pApplication)); - pApplication->SetParameter(L"InProcessExeLocation", struExeLocation.QueryStr()); + std::array parameters { + {"InProcessExeLocation", struExeLocation.QueryStr()} + }; + + FINISHED_IF_FAILED(m_pfnAspNetCoreCreateApplication( + m_pServer, + pHttpContext->GetApplication(), + parameters.data(), + static_cast(parameters.size()), + &pApplication)); m_pApplication = pApplication; } diff --git a/src/AspNetCoreModuleV2/CommonLib/application.h b/src/AspNetCoreModuleV2/CommonLib/application.h index 6025fdd0fb..0c0752f0c4 100644 --- a/src/AspNetCoreModuleV2/CommonLib/application.h +++ b/src/AspNetCoreModuleV2/CommonLib/application.h @@ -40,16 +40,6 @@ public: } } - VOID - SetParameter( - _In_ LPCWSTR pzName, - _In_ LPCWSTR pzValue) - override - { - UNREFERENCED_PARAMETER(pzName); - UNREFERENCED_PARAMETER(pzValue); - } - protected: volatile APPLICATION_STATUS m_status = APPLICATION_STATUS::UNKNOWN; diff --git a/src/AspNetCoreModuleV2/CommonLib/iapplication.h b/src/AspNetCoreModuleV2/CommonLib/iapplication.h index f74ebc021c..dcec7a5e3e 100644 --- a/src/AspNetCoreModuleV2/CommonLib/iapplication.h +++ b/src/AspNetCoreModuleV2/CommonLib/iapplication.h @@ -14,6 +14,12 @@ enum APPLICATION_STATUS FAIL }; +struct APPLICATION_PARAMETER +{ + LPCSTR pzName; + PVOID pValue; +}; + class IAPPLICATION { public: @@ -46,10 +52,4 @@ public: CreateHandler( _In_ IHttpContext *pHttpContext, _Out_ IREQUEST_HANDLER **pRequestHandler) = 0; - - virtual - VOID - SetParameter( - _In_ LPCWSTR pzName, - _In_ LPCWSTR pzValue) = 0; }; diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cxx b/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cxx index d0775e1ae7..20314d34a4 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cxx +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/dllmain.cxx @@ -82,9 +82,11 @@ BOOL APIENTRY DllMain(HMODULE hModule, HRESULT __stdcall CreateApplication( - _In_ IHttpServer *pServer, - _In_ IHttpApplication *pHttpApplication, - _Out_ IAPPLICATION **ppApplication + _In_ IHttpServer *pServer, + _In_ IHttpApplication *pHttpApplication, + _In_ APPLICATION_PARAMETER *pParameters, + _In_ DWORD nParameters, + _Out_ IAPPLICATION **ppApplication ) { REQUESTHANDLER_CONFIG *pConfig = NULL; @@ -97,11 +99,11 @@ CreateApplication( auto config = std::unique_ptr(pConfig); - BOOL disableStartupPage = pConfig->QueryDisableStartUpErrorPage(); + const bool disableStartupPage = pConfig->QueryDisableStartUpErrorPage(); - auto pApplication = std::make_unique(pServer, std::move(config)); - - if (FAILED(pApplication->LoadManagedApplication())) + auto pApplication = std::make_unique(pServer, std::move(config), pParameters, nParameters); + + if (FAILED_LOG(pApplication->LoadManagedApplication())) { // Set the currently running application to a fake application that returns startup exceptions. *ppApplication = new StartupExceptionApplication(pServer, disableStartupPage); diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp index 85897d906c..a1cf301054 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp @@ -13,18 +13,22 @@ #include "exceptions.h" #include "LoggingHelpers.h" +const LPCSTR IN_PROCESS_APPLICATION::s_exeLocationParameterName = "InProcessExeLocation"; + IN_PROCESS_APPLICATION* IN_PROCESS_APPLICATION::s_Application = NULL; IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( IHttpServer *pHttpServer, - std::unique_ptr pConfig) : + std::unique_ptr pConfig, + APPLICATION_PARAMETER *pParameters, + DWORD nParameters) : + InProcessApplicationBase(pHttpServer), m_pHttpServer(pHttpServer), m_ProcessExitCode(0), m_fBlockCallbacksIntoManaged(FALSE), - m_fInitialized(FALSE), m_fShutdownCalledFromNative(FALSE), m_fShutdownCalledFromManaged(FALSE), - InProcessApplicationBase(pHttpServer), + m_fInitialized(FALSE), m_pConfig(std::move(pConfig)) { // is it guaranteed that we have already checked app offline at this point? @@ -32,6 +36,13 @@ IN_PROCESS_APPLICATION::IN_PROCESS_APPLICATION( DBG_ASSERT(pHttpServer != NULL); DBG_ASSERT(pConfig != NULL); + for (DWORD i = 0; i < nParameters; i++) + { + if (_stricmp(pParameters[i].pzName, s_exeLocationParameterName) == 0) + { + m_struExeLocation.Copy(reinterpret_cast(pParameters[i].pValue)); + } + } m_status = APPLICATION_STATUS::STARTING; } diff --git a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h index 6b3bfd18a0..034c36fc93 100644 --- a/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h +++ b/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.h @@ -18,7 +18,9 @@ class IN_PROCESS_APPLICATION : public InProcessApplicationBase public: IN_PROCESS_APPLICATION( IHttpServer* pHttpServer, - std::unique_ptr pConfig); + std::unique_ptr pConfig, + APPLICATION_PARAMETER *pParameters, + DWORD nParameters); ~IN_PROCESS_APPLICATION(); @@ -42,19 +44,6 @@ public: _Out_ IREQUEST_HANDLER **pRequestHandler) override; - VOID - SetParameter( - _In_ LPCWSTR pzName, - _In_ LPCWSTR pzValue) - override - { - const auto exeLocationParameterName = L"InProcessExeLocation"; - if (_wcsicmp(pzName, exeLocationParameterName) == 0) - { - m_struExeLocation.Copy(pzValue); - } - } - // Executes the .NET Core process HRESULT ExecuteApplication( @@ -162,7 +151,7 @@ private: IOutputManager* m_pLoggerProvider; std::unique_ptr m_pConfig; - + static const LPCSTR s_exeLocationParameterName; static VOID diff --git a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cxx b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cxx index 0a416a4c7b..c74f3cacaa 100644 --- a/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cxx +++ b/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/dllmain.cxx @@ -272,9 +272,13 @@ __stdcall CreateApplication( _In_ IHttpServer *pServer, _In_ IHttpApplication *pHttpApplication, + _In_ APPLICATION_PARAMETER *pParameters, + _In_ DWORD nParameters, _Out_ IAPPLICATION **ppApplication ) { + UNREFERENCED_PARAMETER(pParameters); + UNREFERENCED_PARAMETER(nParameters); HRESULT hr = S_OK; IAPPLICATION *pApplication = NULL; REQUESTHANDLER_CONFIG *pConfig = NULL; diff --git a/test/CommonLibTests/inprocess_application_tests.cpp b/test/CommonLibTests/inprocess_application_tests.cpp index d362b8f56c..3f1f67e457 100644 --- a/test/CommonLibTests/inprocess_application_tests.cpp +++ b/test/CommonLibTests/inprocess_application_tests.cpp @@ -3,6 +3,7 @@ #include "stdafx.h" +#include #include "inprocessapplication.h" #include "fakeclasses.h" @@ -17,11 +18,13 @@ namespace InprocessTests auto server = new MockHttpServer(); auto requestHandlerConfig = MockRequestHandlerConfig::CreateConfig(); auto config = std::unique_ptr(requestHandlerConfig); - IN_PROCESS_APPLICATION *app = new IN_PROCESS_APPLICATION(server, std::move(config)); - { - std::wstring exePath(L"hello"); - app->SetParameter(L"InProcessExeLocation", exePath.c_str()); - } + + std::wstring exePath(L"hello"); + std::array parameters { + {"InProcessExeLocation", exePath.data()} + }; + + IN_PROCESS_APPLICATION *app = new IN_PROCESS_APPLICATION(server, std::move(config), parameters.data(), 1); ASSERT_STREQ(app->QueryExeLocation(), L"hello"); } } diff --git a/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs b/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs index f848ab910a..051ae6a2dc 100644 --- a/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs +++ b/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Linq; using System.Net; using System.Threading.Tasks; using IISIntegration.FunctionalTests.Utilities; @@ -77,6 +78,9 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests await AssertStarts( deploymentResult => Helpers.ModifyAspNetCoreSectionInWebConfig(deploymentResult, "processPath", path), deploymentParameters => deploymentParameters.EnvironmentVariables["PATH"] = Path.GetDirectoryName(_dotnetLocation)); + + // Verify that in this scenario where.exe was invoked only once by shim and request handler uses cached value + Assert.Equal(1, TestSink.Writes.Count(w => w.Message.Contains("Invoking where.exe to find dotnet.exe"))); } private async Task AssertStarts(Action postDeploy, Action preDeploy = null) From dfd75e939d49733fd8dcb0104a7b121671c467f6 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 2 Jul 2018 12:15:15 -0700 Subject: [PATCH 4/8] Add full IIS tests (#979) --- .appveyor.yml | 1 + .../IIS.Performance/PlaintextBenchmark.cs | 2 +- build/dependencies.props | 4 +- .../CommonLib/hostfxr_utility.cpp | 6 +- .../IISIntegration.FunctionalTests.csproj | 2 + .../Inprocess/IISTests.cs | 40 +++ .../Inprocess/TestServerTest.cs | 4 +- .../Utilities/FunctionalTestsBase.cs | 12 +- .../Utilities/IISApplication.cs | 310 ++++++++++++++++++ .../Utilities/IISDeployer.cs | 95 ++++++ .../Utilities/IISDeploymentResult.cs | 3 + ...pIfHostableWebCoreNotAvailibleAttribute.cs | 2 +- .../Utilities/SkipIfIISCannotRunAttribute.cs | 74 +++++ .../Utilities/TestIISUriHelper.cs | 84 +++++ .../Utilities/TestServer.cs | 2 +- 15 files changed, 629 insertions(+), 12 deletions(-) create mode 100644 test/IISIntegration.FunctionalTests/Inprocess/IISTests.cs create mode 100644 test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs create mode 100644 test/IISIntegration.FunctionalTests/Utilities/IISDeployer.cs create mode 100644 test/IISIntegration.FunctionalTests/Utilities/SkipIfIISCannotRunAttribute.cs create mode 100644 test/IISIntegration.FunctionalTests/Utilities/TestIISUriHelper.cs diff --git a/.appveyor.yml b/.appveyor.yml index 8290b9bacd..f46dcc270f 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -8,6 +8,7 @@ branches: install: - ps: .\tools\update_schema.ps1 - git submodule update --init --recursive + - net start w3svc build_script: - ps: .\run.ps1 default-build clone_depth: 1 diff --git a/benchmarks/IIS.Performance/PlaintextBenchmark.cs b/benchmarks/IIS.Performance/PlaintextBenchmark.cs index ff79705756..8c6b4b2cd3 100644 --- a/benchmarks/IIS.Performance/PlaintextBenchmark.cs +++ b/benchmarks/IIS.Performance/PlaintextBenchmark.cs @@ -6,9 +6,9 @@ using System.Net.Http; using System.Text; using System.Threading.Tasks; using BenchmarkDotNet.Attributes; -using IISIntegration.FunctionalTests.Utilities; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.IIS.Performance diff --git a/build/dependencies.props b/build/dependencies.props index 0736961427..106a216654 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -1,4 +1,4 @@ - + $(MSBuildAllProjects);$(MSBuildThisFileFullPath) @@ -38,8 +38,10 @@ 2.2.0-preview1-26618-02 2.2.0-preview1-34530 15.6.1 + 11.1.0 2.0.3 4.6.0-preview1-26617-01 + 4.6.0-preview1-26617-01 4.6.0-preview1-26617-01 4.6.0-preview1-26617-01 4.6.0-preview1-26617-01 diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp index 2ebc70966e..48eb39e045 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp @@ -334,7 +334,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( const fs::path & requestedPath ) { - WLOG_INFOF(L"Resolving absolute path do dotnet.exe from %s", requestedPath.c_str()); + WLOG_INFOF(L"Resolving absolute path to dotnet.exe from %s", requestedPath.c_str()); // // If we are given an absolute path to dotnet.exe, we are done @@ -368,7 +368,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( const auto dotnetViaWhere = InvokeWhereToFindDotnet(); if (dotnetViaWhere.has_value()) { - WLOG_INFOF(L"Found dotnet.exe wia where.exe invocation at %s", dotnetViaWhere.value().c_str()); + WLOG_INFOF(L"Found dotnet.exe via where.exe invocation at %s", dotnetViaWhere.value().c_str()); return dotnetViaWhere; } @@ -394,7 +394,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToHostFxr( std::vector versionFolders; const auto hostFxrBase = dotnetPath.parent_path() / "host" / "fxr"; - WLOG_INFOF(L"Resolving absolute path do hostfxr.dll from %s", dotnetPath.c_str()); + WLOG_INFOF(L"Resolving absolute path to hostfxr.dll from %s", dotnetPath.c_str()); if (!is_directory(hostFxrBase)) { diff --git a/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj b/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj index 69861d7aa6..544cde26ee 100644 --- a/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj +++ b/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj @@ -36,7 +36,9 @@ + + diff --git a/test/IISIntegration.FunctionalTests/Inprocess/IISTests.cs b/test/IISIntegration.FunctionalTests/Inprocess/IISTests.cs new file mode 100644 index 0000000000..040f03fcad --- /dev/null +++ b/test/IISIntegration.FunctionalTests/Inprocess/IISTests.cs @@ -0,0 +1,40 @@ +// 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. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.IntegrationTesting; +using Microsoft.AspNetCore.Testing.xunit; +using Xunit; + +namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests +{ + [SkipIfIISCannotRun] + public class IISTests : FunctionalTestsBase + { + [ConditionalFact] + public Task HelloWorld_IIS_CoreClr_X64_Standalone() + { + return HelloWorld(RuntimeFlavor.CoreClr, ApplicationType.Standalone); + } + + [ConditionalFact] + public Task HelloWorld_IIS_CoreClr_X64_Portable() + { + return HelloWorld(RuntimeFlavor.CoreClr, ApplicationType.Portable); + } + + private async Task HelloWorld(RuntimeFlavor runtimeFlavor, ApplicationType applicationType) + { + var deploymentParameters = Helpers.GetBaseDeploymentParameters(); + deploymentParameters.ServerType = ServerType.IIS; + deploymentParameters.ApplicationType = applicationType; + + var deploymentResult = await DeployAsync(deploymentParameters); + + var response = await deploymentResult.RetryingHttpClient.GetAsync("HelloWorld"); + var responseText = await response.Content.ReadAsStringAsync(); + + Assert.Equal("Hello World", responseText); + } + } +} diff --git a/test/IISIntegration.FunctionalTests/Inprocess/TestServerTest.cs b/test/IISIntegration.FunctionalTests/Inprocess/TestServerTest.cs index 9c9f9918bb..4b4d3f25c8 100644 --- a/test/IISIntegration.FunctionalTests/Inprocess/TestServerTest.cs +++ b/test/IISIntegration.FunctionalTests/Inprocess/TestServerTest.cs @@ -1,8 +1,6 @@ // 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. -using System.Diagnostics; -using System.Threading; using System.Threading.Tasks; using IISIntegration.FunctionalTests.Utilities; using Microsoft.AspNetCore.Http; @@ -11,7 +9,7 @@ using Microsoft.Extensions.Logging.Testing; using Xunit; using Xunit.Abstractions; -namespace IISIntegration.FunctionalTests.Inprocess +namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { [SkipIfHostableWebCoreNotAvailible] public class TestServerTest: LoggedTest diff --git a/test/IISIntegration.FunctionalTests/Utilities/FunctionalTestsBase.cs b/test/IISIntegration.FunctionalTests/Utilities/FunctionalTestsBase.cs index 1e739b645a..a30387f0dd 100644 --- a/test/IISIntegration.FunctionalTests/Utilities/FunctionalTestsBase.cs +++ b/test/IISIntegration.FunctionalTests/Utilities/FunctionalTestsBase.cs @@ -21,10 +21,18 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting { if (!parameters.EnvironmentVariables.ContainsKey(DebugEnvironmentVariable)) { - // enable debug output parameters.EnvironmentVariables[DebugEnvironmentVariable] = "4"; } - _deployer = ApplicationDeployerFactory.Create(parameters, LoggerFactory); + + // Currently hosting throws if the Servertype = IIS. + if (parameters.ServerType == ServerType.IIS) + { + _deployer = new IISDeployer(parameters, LoggerFactory); + } + else + { + _deployer = ApplicationDeployerFactory.Create(parameters, LoggerFactory); + } var result = await _deployer.DeployAsync(); diff --git a/test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs b/test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs new file mode 100644 index 0000000000..013c24e376 --- /dev/null +++ b/test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs @@ -0,0 +1,310 @@ +// 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. + +using System; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Threading.Tasks; +using System.Xml.Linq; +using Microsoft.Extensions.CommandLineUtils; +using Microsoft.Extensions.Logging; +using Microsoft.Web.Administration; + +namespace Microsoft.AspNetCore.Server.IntegrationTesting +{ + /// + /// Represents the IIS website registered in the global applicationHost.config + /// + internal class IISApplication + { + private static readonly TimeSpan _timeout = TimeSpan.FromSeconds(2); + private static readonly TimeSpan _retryDelay = TimeSpan.FromMilliseconds(100); + private readonly ServerManager _serverManager = new ServerManager(); + private readonly DeploymentParameters _deploymentParameters; + private readonly ILogger _logger; + private readonly string _ancmVersion; + private readonly object _syncLock = new object(); + private readonly string _apphostConfigBackupPath; + private static readonly string _apphostConfigPath = Path.Combine( + Environment.SystemDirectory, + "inetsrv", + "config", + "applicationhost.config"); + + public IISApplication(DeploymentParameters deploymentParameters, ILogger logger) + { + _deploymentParameters = deploymentParameters; + _logger = logger; + _ancmVersion = deploymentParameters.AncmVersion.ToString(); + WebSiteName = CreateTestSiteName(); + AppPoolName = $"{WebSiteName}Pool"; + _apphostConfigBackupPath = Path.Combine( + Environment.SystemDirectory, + "inetsrv", + "config", + $"applicationhost.config.{WebSiteName}backup"); + } + + public string WebSiteName { get; } + + public string AppPoolName { get; } + + public async Task StartIIS(Uri uri, string contentRoot) + { + // Backup currently deployed apphost.config file + using (_logger.BeginScope("StartIIS")) + { + AddTemporaryAppHostConfig(); + var port = uri.Port; + if (port == 0) + { + throw new NotSupportedException("Cannot set port 0 for IIS."); + } + + ConfigureAppPool(contentRoot); + + ConfigureSite(contentRoot, port); + + ConfigureAppHostConfig(contentRoot); + + if (_deploymentParameters.ApplicationType == ApplicationType.Portable) + { + ModifyAspNetCoreSectionInWebConfig("processPath", DotNetMuxer.MuxerPathOrDefault()); + } + + _serverManager.CommitChanges(); + + await WaitUntilSiteStarted(); + } + } + + private void ModifyAspNetCoreSectionInWebConfig(string key, string value) + { + var webConfigFile = Path.Combine(_deploymentParameters.PublishedApplicationRootPath, "web.config"); + var config = XDocument.Load(webConfigFile); + var element = config.Descendants("aspNetCore").FirstOrDefault(); + element.SetAttributeValue(key, value); + config.Save(webConfigFile); + } + + private async Task WaitUntilSiteStarted() + { + var sw = Stopwatch.StartNew(); + + while (sw.Elapsed < _timeout) + { + try + { + var site = _serverManager.Sites.FirstOrDefault(s => s.Name.Equals(WebSiteName)); + if (site.State == ObjectState.Started) + { + _logger.LogInformation($"Site {WebSiteName} has started."); + return; + } + else if (site.State != ObjectState.Starting) + { + _logger.LogInformation($"Site hasn't started with state: {site.State.ToString()}"); + var state = site.Start(); + _logger.LogInformation($"Tried to start site, state: {state.ToString()}"); + } + } + catch (COMException comException) + { + // Accessing the site.State property while the site + // is starting up returns the COMException + // The object identifier does not represent a valid object. + // (Exception from HRESULT: 0x800710D8) + // This also means the site is not started yet, so catch and retry + // after waiting. + _logger.LogWarning($"ComException: {comException.Message}"); + } + + await Task.Delay(_retryDelay); + } + + throw new TimeoutException($"IIS failed to start site."); + } + + public async Task StopAndDeleteAppPool() + { + if (string.IsNullOrEmpty(WebSiteName)) + { + return; + } + + RestoreAppHostConfig(); + + _serverManager.CommitChanges(); + + await WaitUntilSiteStopped(); + } + + private async Task WaitUntilSiteStopped() + { + var site = _serverManager.Sites.Where(element => element.Name == WebSiteName).FirstOrDefault(); + if (site == null) + { + return; + } + + var sw = Stopwatch.StartNew(); + + while (sw.Elapsed < _timeout) + { + try + { + if (site.State == ObjectState.Stopped) + { + _logger.LogInformation($"Site {WebSiteName} has stopped successfully."); + return; + } + } + catch (COMException) + { + // Accessing the site.State property while the site + // is shutdown down returns the COMException + return; + } + + _logger.LogWarning($"IIS has not stopped after {sw.Elapsed.TotalMilliseconds}"); + await Task.Delay(_retryDelay); + } + + throw new TimeoutException($"IIS failed to stop site {site}."); + } + + private void AddTemporaryAppHostConfig() + { + File.Copy(_apphostConfigPath, _apphostConfigBackupPath); + _logger.LogInformation($"Backed up {_apphostConfigPath} to {_apphostConfigBackupPath}"); + } + + private void RestoreAppHostConfig() + { + RetryHelper.RetryOperation( + () => File.Delete(_apphostConfigPath), + e => _logger.LogWarning($"Failed to delete directory : {e.Message}"), + retryCount: 3, + retryDelayMilliseconds: 100); + + File.Move(_apphostConfigBackupPath, _apphostConfigPath); + _logger.LogInformation($"Restored {_apphostConfigPath}."); + } + + private ApplicationPool ConfigureAppPool(string contentRoot) + { + var pool = _serverManager.ApplicationPools.Add(AppPoolName); + pool.ProcessModel.IdentityType = ProcessModelIdentityType.LocalSystem; + pool.ManagedRuntimeVersion = string.Empty; + var envCollection = pool.GetCollection("environmentVariables"); + + AddEnvironmentVariables(contentRoot, envCollection); + + _logger.LogInformation($"Configured AppPool {AppPoolName}"); + return pool; + } + + private void AddEnvironmentVariables(string contentRoot, ConfigurationElementCollection envCollection) + { + foreach (var tuple in _deploymentParameters.EnvironmentVariables) + { + AddEnvironmentVariableToAppPool(envCollection, tuple.Key, tuple.Value); + } + } + + private static void AddEnvironmentVariableToAppPool(ConfigurationElementCollection envCollection, string key, string value) + { + var addElement = envCollection.CreateElement("add"); + addElement["name"] = key; + addElement["value"] = value; + envCollection.Add(addElement); + } + + private Site ConfigureSite(string contentRoot, int port) + { + var site = _serverManager.Sites.Add(WebSiteName, contentRoot, port); + site.Applications.Single().ApplicationPoolName = AppPoolName; + _logger.LogInformation($"Configured Site {WebSiteName} with AppPool {AppPoolName}"); + return site; + } + + private Configuration ConfigureAppHostConfig(string dllRoot) + { + var config = _serverManager.GetApplicationHostConfiguration(); + + SetGlobalModuleSection(config, dllRoot); + + SetModulesSection(config); + + return config; + } + + private void SetGlobalModuleSection(Configuration config, string dllRoot) + { + var ancmFile = GetAncmLocation(dllRoot); + + var globalModulesSection = config.GetSection("system.webServer/globalModules"); + var globalConfigElement = globalModulesSection + .GetCollection() + .Where(element => (string)element["name"] == _ancmVersion) + .FirstOrDefault(); + + if (globalConfigElement == null) + { + _logger.LogInformation($"Could not find {_ancmVersion} section in global modules; creating section."); + var addElement = globalModulesSection.GetCollection().CreateElement("add"); + addElement["name"] = _ancmVersion; + addElement["image"] = ancmFile; + globalModulesSection.GetCollection().Add(addElement); + } + else + { + _logger.LogInformation($"Replacing {_ancmVersion} section in global modules with {ancmFile}"); + globalConfigElement["image"] = ancmFile; + } + } + + private void SetModulesSection(Configuration config) + { + var modulesSection = config.GetSection("system.webServer/modules"); + var moduleConfigElement = modulesSection.GetCollection().Where(element => (string)element["name"] == _ancmVersion).FirstOrDefault(); + if (moduleConfigElement == null) + { + _logger.LogInformation($"Could not find {_ancmVersion} section in modules; creating section."); + var moduleElement = modulesSection.GetCollection().CreateElement("add"); + moduleElement["name"] = _ancmVersion; + modulesSection.GetCollection().Add(moduleElement); + } + } + + private string CreateTestSiteName() + { + if (!string.IsNullOrEmpty(_deploymentParameters.SiteName)) + { + return $"{_deploymentParameters.SiteName}{DateTime.Now.ToString("yyyyMMddHHmmss")}"; + } + else + { + return $"testsite{DateTime.Now.ToString("yyyyMMddHHmmss")}"; + } + } + + private string GetAncmLocation(string dllRoot) + { + var arch = _deploymentParameters.RuntimeArchitecture == RuntimeArchitecture.x64 ? @"x64\aspnetcorev2.dll" : @"x86\aspnetcorev2.dll"; + var ancmFile = Path.Combine(dllRoot, arch); + if (!File.Exists(Environment.ExpandEnvironmentVariables(ancmFile))) + { + ancmFile = Path.Combine(dllRoot, "aspnetcorev2.dll"); + if (!File.Exists(Environment.ExpandEnvironmentVariables(ancmFile))) + { + throw new FileNotFoundException("AspNetCoreModule could not be found.", ancmFile); + } + } + + return ancmFile; + } + } +} diff --git a/test/IISIntegration.FunctionalTests/Utilities/IISDeployer.cs b/test/IISIntegration.FunctionalTests/Utilities/IISDeployer.cs new file mode 100644 index 0000000000..4f0d5252c9 --- /dev/null +++ b/test/IISIntegration.FunctionalTests/Utilities/IISDeployer.cs @@ -0,0 +1,95 @@ +// 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. + +using System; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using System.Xml.Linq; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Server.IntegrationTesting +{ + /// + /// Deployer for IIS. + /// + public partial class IISDeployer : ApplicationDeployer + { + private IISApplication _application; + private CancellationTokenSource _hostShutdownToken = new CancellationTokenSource(); + + public IISDeployer(DeploymentParameters deploymentParameters, ILoggerFactory loggerFactory) + : base(deploymentParameters, loggerFactory) + { + } + + public override void Dispose() + { + if (_application != null) + { + _application.StopAndDeleteAppPool().GetAwaiter().GetResult(); + + TriggerHostShutdown(_hostShutdownToken); + } + + GetLogsFromFile($"{_application.WebSiteName}.txt"); + GetLogsFromFile("web.config"); + + CleanPublishedOutput(); + InvokeUserApplicationCleanup(); + + StopTimer(); + } + + public override async Task DeployAsync() + { + using (Logger.BeginScope("Deployment")) + { + StartTimer(); + + var contentRoot = string.Empty; + _application = new IISApplication(DeploymentParameters, Logger); + + // For now, only support using published output + DeploymentParameters.PublishApplicationBeforeDeployment = true; + + if (DeploymentParameters.PublishApplicationBeforeDeployment) + { + DotnetPublish(); + contentRoot = DeploymentParameters.PublishedApplicationRootPath; + } + + var uri = TestIISUriHelper.BuildTestUri(ServerType.IIS, DeploymentParameters.ApplicationBaseUriHint); + // To prevent modifying the IIS setup concurrently. + await _application.StartIIS(uri, contentRoot); + + // Warm up time for IIS setup. + Logger.LogInformation("Successfully finished IIS application directory setup."); + + return new DeploymentResult( + LoggerFactory, + DeploymentParameters, + applicationBaseUri: uri.ToString(), + contentRoot: contentRoot, + hostShutdownToken: _hostShutdownToken.Token + ); + } + } + + private void GetLogsFromFile(string file) + { + var arr = new string[0]; + + RetryHelper.RetryOperation(() => arr = File.ReadAllLines(Path.Combine(DeploymentParameters.PublishedApplicationRootPath, file)), + (ex) => Logger.LogError("Could not read log file"), + 5, + 200); + foreach (var line in arr) + { + Logger.LogInformation(line); + } + } + + } +} diff --git a/test/IISIntegration.FunctionalTests/Utilities/IISDeploymentResult.cs b/test/IISIntegration.FunctionalTests/Utilities/IISDeploymentResult.cs index 6ea25b89ec..8e39634f2b 100644 --- a/test/IISIntegration.FunctionalTests/Utilities/IISDeploymentResult.cs +++ b/test/IISIntegration.FunctionalTests/Utilities/IISDeploymentResult.cs @@ -1,3 +1,6 @@ +// 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. + using System.Net.Http; using Microsoft.Extensions.Logging; diff --git a/test/IISIntegration.FunctionalTests/Utilities/SkipIfHostableWebCoreNotAvailibleAttribute.cs b/test/IISIntegration.FunctionalTests/Utilities/SkipIfHostableWebCoreNotAvailibleAttribute.cs index 046d430317..b21a087321 100644 --- a/test/IISIntegration.FunctionalTests/Utilities/SkipIfHostableWebCoreNotAvailibleAttribute.cs +++ b/test/IISIntegration.FunctionalTests/Utilities/SkipIfHostableWebCoreNotAvailibleAttribute.cs @@ -5,7 +5,7 @@ using System; using System.IO; using Microsoft.AspNetCore.Testing.xunit; -namespace IISIntegration.FunctionalTests.Utilities +namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { [AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Method)] public sealed class SkipIfHostableWebCoreNotAvailibleAttribute : Attribute, ITestCondition diff --git a/test/IISIntegration.FunctionalTests/Utilities/SkipIfIISCannotRunAttribute.cs b/test/IISIntegration.FunctionalTests/Utilities/SkipIfIISCannotRunAttribute.cs new file mode 100644 index 0000000000..aa6c3abd75 --- /dev/null +++ b/test/IISIntegration.FunctionalTests/Utilities/SkipIfIISCannotRunAttribute.cs @@ -0,0 +1,74 @@ +// 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. + +using System; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Security.Principal; +using System.Xml.Linq; +using Microsoft.AspNetCore.Server.IntegrationTesting; +using Microsoft.AspNetCore.Testing.xunit; + +namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests +{ + [AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Method)] + public sealed class SkipIfIISCannotRunAttribute : Attribute, ITestCondition + { + private static readonly bool _isMet; + public static readonly string _skipReason; + + public bool IsMet => _isMet; + public string SkipReason => _skipReason; + + static SkipIfIISCannotRunAttribute() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + _skipReason = "IIS tests can only be run on Windows"; + return; + } + + var identity = WindowsIdentity.GetCurrent(); + var principal = new WindowsPrincipal(identity); + if (!principal.IsInRole(WindowsBuiltInRole.Administrator)) + { + _skipReason += "The current console is not running as admin."; + return; + } + + if (!File.Exists(Path.Combine(Environment.SystemDirectory, "inetsrv", "w3wp.exe"))) + { + _skipReason += "The machine does not have IIS installed."; + return; + } + + var ancmConfigPath = Path.Combine(Environment.SystemDirectory, "inetsrv", "config", "schema", "aspnetcore_schema_v2.xml"); + + if (!File.Exists(ancmConfigPath)) + { + _skipReason = "IIS Schema is not installed."; + return; + } + + XDocument ancmConfig; + + try + { + ancmConfig = XDocument.Load(ancmConfigPath); + } + catch + { + _skipReason = "Could not read ANCM schema configuration"; + return; + } + + _isMet = ancmConfig + .Root + .Descendants("attribute") + .Any(n => "hostingModel".Equals(n.Attribute("name")?.Value, StringComparison.Ordinal)); + + _skipReason = _isMet ? null : "IIS schema needs to be upgraded to support ANCM."; + } + } +} diff --git a/test/IISIntegration.FunctionalTests/Utilities/TestIISUriHelper.cs b/test/IISIntegration.FunctionalTests/Utilities/TestIISUriHelper.cs new file mode 100644 index 0000000000..b8c37ddd93 --- /dev/null +++ b/test/IISIntegration.FunctionalTests/Utilities/TestIISUriHelper.cs @@ -0,0 +1,84 @@ +// 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. + +using System; +using System.Net; +using System.Net.Sockets; + +namespace Microsoft.AspNetCore.Server.IntegrationTesting +{ + // Copied from Hosting for now https://github.com/aspnet/Hosting/blob/970bc8a30d66dd6894f8f662e5fdab9e68d57777/src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/TestUriHelper.cs + internal static class TestIISUriHelper + { + internal static Uri BuildTestUri(ServerType serverType) + { + return BuildTestUri(serverType, hint: null); + } + + internal static Uri BuildTestUri(ServerType serverType, string hint) + { + // Assume status messages are enabled for Kestrel and disabled for all other servers. + return BuildTestUri(serverType, hint, statusMessagesEnabled: serverType == ServerType.Kestrel); + } + + internal static Uri BuildTestUri(ServerType serverType, string hint, bool statusMessagesEnabled) + { + if (string.IsNullOrEmpty(hint)) + { + if (serverType == ServerType.Kestrel && statusMessagesEnabled) + { + // Most functional tests use this codepath and should directly bind to dynamic port "0" and scrape + // the assigned port from the status message, which should be 100% reliable since the port is bound + // once and never released. Binding to dynamic port "0" on "localhost" (both IPv4 and IPv6) is not + // supported, so the port is only bound on "127.0.0.1" (IPv4). If a test explicitly requires IPv6, + // it should provide a hint URL with "localhost" (IPv4 and IPv6) or "[::1]" (IPv6-only). + return new UriBuilder("http", "127.0.0.1", 0).Uri; + } + else + { + // If the server type is not Kestrel, or status messages are disabled, there is no status message + // from which to scrape the assigned port, so the less reliable GetNextPort() must be used. The + // port is bound on "localhost" (both IPv4 and IPv6), since this is supported when using a specific + // (non-zero) port. + return new UriBuilder("http", "localhost", GetNextPort()).Uri; + } + } + else + { + var uriHint = new Uri(hint); + if (uriHint.Port == 0) + { + // Only a few tests use this codepath, so it's fine to use the less reliable GetNextPort() for simplicity. + // The tests using this codepath will be reviewed to see if they can be changed to directly bind to dynamic + // port "0" on "127.0.0.1" and scrape the assigned port from the status message (the default codepath). + return new UriBuilder(uriHint) { Port = GetNextPort() }.Uri; + } + else + { + // If the hint contains a specific port, return it unchanged. + return uriHint; + } + } + } + + // Copied from https://github.com/aspnet/KestrelHttpServer/blob/47f1db20e063c2da75d9d89653fad4eafe24446c/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs#L508 + // + // This method is an attempt to safely get a free port from the OS. Most of the time, + // when binding to dynamic port "0" the OS increments the assigned port, so it's safe + // to re-use the assigned port in another process. However, occasionally the OS will reuse + // a recently assigned port instead of incrementing, which causes flaky tests with AddressInUse + // exceptions. This method should only be used when the application itself cannot use + // dynamic port "0" (e.g. IISExpress). Most functional tests using raw Kestrel + // (with status messages enabled) should directly bind to dynamic port "0" and scrape + // the assigned port from the status message, which should be 100% reliable since the port + // is bound once and never released. + internal static int GetNextPort() + { + using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp)) + { + socket.Bind(new IPEndPoint(IPAddress.Loopback, 0)); + return ((IPEndPoint)socket.LocalEndPoint).Port; + } + } + } +} diff --git a/test/IISIntegration.FunctionalTests/Utilities/TestServer.cs b/test/IISIntegration.FunctionalTests/Utilities/TestServer.cs index 10f526f860..6cd61b1459 100644 --- a/test/IISIntegration.FunctionalTests/Utilities/TestServer.cs +++ b/test/IISIntegration.FunctionalTests/Utilities/TestServer.cs @@ -15,7 +15,7 @@ using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; -namespace IISIntegration.FunctionalTests.Utilities +namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests { public class TestServer: IDisposable, IStartup { From 9b7ee920289cf18fdcbd4e1b7d89d208835e9385 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 2 Jul 2018 17:18:05 -0700 Subject: [PATCH 5/8] Check thread invalid handle and remove extra dup2 call. (#1008) --- .../RequestHandlerLib/PipeOutputManager.cpp | 12 ++++++----- .../CommonLibTests/PipeOutputManagerTests.cpp | 20 +++++++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/AspNetCoreModuleV2/RequestHandlerLib/PipeOutputManager.cpp b/src/AspNetCoreModuleV2/RequestHandlerLib/PipeOutputManager.cpp index 5620572c6f..e8b61a962b 100644 --- a/src/AspNetCoreModuleV2/RequestHandlerLib/PipeOutputManager.cpp +++ b/src/AspNetCoreModuleV2/RequestHandlerLib/PipeOutputManager.cpp @@ -13,7 +13,7 @@ PipeOutputManager::PipeOutputManager() : m_dwStdErrReadTotal(0), m_hErrReadPipe(INVALID_HANDLE_VALUE), m_hErrWritePipe(INVALID_HANDLE_VALUE), - m_hErrThread(INVALID_HANDLE_VALUE), + m_hErrThread(NULL), m_fDisposed(FALSE) { InitializeSRWLock(&m_srwLock); @@ -54,7 +54,6 @@ PipeOutputManager::StopOutputRedirection() if (m_fdPreviousStdOut >= 0) { - _dup2(m_fdPreviousStdOut, _fileno(stdout)); LOG_IF_DUPFAIL(_dup2(m_fdPreviousStdOut, _fileno(stdout))); } else @@ -79,7 +78,6 @@ PipeOutputManager::StopOutputRedirection() // GetExitCodeThread returns 0 on failure; thread status code is invalid. if (m_hErrThread != NULL && - m_hErrThread != INVALID_HANDLE_VALUE && !LOG_LAST_ERROR_IF(GetExitCodeThread(m_hErrThread, &dwThreadStatus) == 0) && dwThreadStatus == STILL_ACTIVE) { @@ -96,8 +94,12 @@ PipeOutputManager::StopOutputRedirection() } } - CloseHandle(m_hErrThread); - m_hErrThread = NULL; + if (m_hErrThread != NULL) + { + CloseHandle(m_hErrThread); + m_hErrThread = NULL; + } + if (m_hErrReadPipe != INVALID_HANDLE_VALUE) { diff --git a/test/CommonLibTests/PipeOutputManagerTests.cpp b/test/CommonLibTests/PipeOutputManagerTests.cpp index e15fa2868a..67ce9f2f04 100644 --- a/test/CommonLibTests/PipeOutputManagerTests.cpp +++ b/test/CommonLibTests/PipeOutputManagerTests.cpp @@ -32,6 +32,26 @@ namespace PipeOutputManagerTests pManager->NotifyStartupComplete(); + } + + TEST(PipeManagerOutputTest, SetInvalidHandlesForErrAndOut) + { + auto m_fdPreviousStdOut = _dup(_fileno(stdout)); + auto m_fdPreviousStdErr = _dup(_fileno(stderr)); + + SetStdHandle(STD_ERROR_HANDLE, INVALID_HANDLE_VALUE); + SetStdHandle(STD_OUTPUT_HANDLE, INVALID_HANDLE_VALUE); + + PCWSTR expected = L"test"; + + PipeOutputManager* pManager = new PipeOutputManager(); + ASSERT_EQ(S_OK, pManager->Start()); + + pManager->NotifyStartupComplete(); + + _dup2(m_fdPreviousStdOut, _fileno(stdout)); + _dup2(m_fdPreviousStdErr, _fileno(stderr)); + // Test will fail if we didn't redirect stdout back to a file descriptor. // This is because gtest relies on console output to know if a test succeeded or failed. // If the output still points to a file/pipe, the test (and all other tests after it) will fail. From 3efc1eede43ac046f4956838db90095402b26eb2 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 2 Jul 2018 18:21:22 -0700 Subject: [PATCH 6/8] Set correct event providers for ANCM V2 (#1009) --- .../AspNetCore/Inc/aspnetcore_shim_config.h | 7 ------- src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp | 2 +- src/AspNetCoreModuleV2/CommonLib/resources.h | 4 ++-- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/AspNetCoreModuleV2/AspNetCore/Inc/aspnetcore_shim_config.h b/src/AspNetCoreModuleV2/AspNetCore/Inc/aspnetcore_shim_config.h index 3fde28273f..5229f95b79 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/Inc/aspnetcore_shim_config.h +++ b/src/AspNetCoreModuleV2/AspNetCore/Inc/aspnetcore_shim_config.h @@ -38,12 +38,6 @@ public: return &m_struApplicationPhysicalPath; } - STRU* - QueryApplicationPath() - { - return &m_struApplication; - } - STRU* QueryConfigPath() { @@ -83,7 +77,6 @@ private: STRU m_struArguments; STRU m_struProcessPath; - STRU m_struApplication; STRU m_struApplicationPhysicalPath; STRU m_struConfigPath; APP_HOSTING_MODEL m_hostingModel; diff --git a/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp b/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp index ccbd586f3e..b1bcc464c3 100644 --- a/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp +++ b/src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp @@ -153,7 +153,7 @@ APPLICATION_INFO::UpdateAppOfflineFileHandle() STACK_STRU(strEventMsg, 256); if (SUCCEEDED(strEventMsg.SafeSnwprintf( ASPNETCORE_EVENT_RECYCLE_APPOFFLINE_MSG, - m_pConfiguration->QueryApplicationPath()->QueryStr()))) + m_pConfiguration->QueryApplicationPhysicalPath()->QueryStr()))) { UTILITY::LogEvent(g_hEventLog, EVENTLOG_INFORMATION_TYPE, diff --git a/src/AspNetCoreModuleV2/CommonLib/resources.h b/src/AspNetCoreModuleV2/CommonLib/resources.h index a67f753311..153af6d3d0 100644 --- a/src/AspNetCoreModuleV2/CommonLib/resources.h +++ b/src/AspNetCoreModuleV2/CommonLib/resources.h @@ -8,8 +8,8 @@ #define IDS_INVALID_PROPERTY 1000 #define IDS_SERVER_ERROR 1001 -#define ASPNETCORE_EVENT_PROVIDER L"IIS AspNetCore Module" -#define ASPNETCORE_IISEXPRESS_EVENT_PROVIDER L"IIS Express AspNetCore Module" +#define ASPNETCORE_EVENT_PROVIDER L"IIS AspNetCore Module V2" +#define ASPNETCORE_IISEXPRESS_EVENT_PROVIDER L"IIS Express AspNetCore Module V2" #define ASPNETCORE_EVENT_MSG_BUFFER_SIZE 256 #define ASPNETCORE_EVENT_PROCESS_START_SUCCESS_MSG L"Application '%s' started process '%d' successfully and process '%d' is listening on port '%d'." From e7d36a42e6b7b96451e38c6142445fe0371c1f91 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 3 Jul 2018 16:27:28 -0700 Subject: [PATCH 7/8] Only add environment variables if we are running on win8.1 or above. (#1007) --- .../Utilities/IISApplication.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs b/test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs index 013c24e376..faba405c06 100644 --- a/test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs +++ b/test/IISIntegration.FunctionalTests/Utilities/IISApplication.cs @@ -198,19 +198,27 @@ namespace Microsoft.AspNetCore.Server.IntegrationTesting var pool = _serverManager.ApplicationPools.Add(AppPoolName); pool.ProcessModel.IdentityType = ProcessModelIdentityType.LocalSystem; pool.ManagedRuntimeVersion = string.Empty; - var envCollection = pool.GetCollection("environmentVariables"); - AddEnvironmentVariables(contentRoot, envCollection); + AddEnvironmentVariables(contentRoot, pool); _logger.LogInformation($"Configured AppPool {AppPoolName}"); return pool; } - private void AddEnvironmentVariables(string contentRoot, ConfigurationElementCollection envCollection) + private void AddEnvironmentVariables(string contentRoot, ApplicationPool pool) { - foreach (var tuple in _deploymentParameters.EnvironmentVariables) + try { - AddEnvironmentVariableToAppPool(envCollection, tuple.Key, tuple.Value); + var envCollection = pool.GetCollection("environmentVariables"); + + foreach (var tuple in _deploymentParameters.EnvironmentVariables) + { + AddEnvironmentVariableToAppPool(envCollection, tuple.Key, tuple.Value); + } + } + catch (COMException comException) + { + _logger.LogInformation($"Could not add environment variables to worker process: {comException.Message}"); } } From 724cc3ce88233fa37a01fe447afed2c00c6711bf Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Tue, 3 Jul 2018 16:49:24 -0700 Subject: [PATCH 8/8] Only run fallback logic for dotnet and dotnet.exe (#1004) --- .../CommonLib/hostfxr_utility.cpp | 21 ++++++++++++------- .../CommonLib/hostfxr_utility.h | 1 + .../Inprocess/StartupTests.cs | 14 ++++++------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp index 48eb39e045..cf703d34a0 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp @@ -128,11 +128,6 @@ HOSTFXR_UTILITY::GetHostFxrParameters( fs::path processPath = Environment::ExpandEnvironmentVariables(pcwzProcessPath); std::wstring arguments = Environment::ExpandEnvironmentVariables(pcwzArguments); - if (processPath.is_relative()) - { - processPath = applicationPhysicalPath / processPath; - } - // Check if the absolute path is to dotnet or not. if (IsDotnetExecutable(processPath)) { @@ -143,7 +138,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( // Get the absolute path to dotnet. If the path is already an absolute path, it will return that path // // Make sure to append the dotnet.exe path correctly here (pass in regular path)? - auto fullProcessPath = GetAbsolutePathToDotnet(processPath); + auto fullProcessPath = GetAbsolutePathToDotnet(applicationPhysicalPath, processPath); if (!fullProcessPath.has_value()) { return E_FAIL; @@ -172,6 +167,11 @@ HOSTFXR_UTILITY::GetHostFxrParameters( { WLOG_INFOF(L"Process path %s is not dotnet, treating application as standalone", processPath.c_str()); + if (processPath.is_relative()) + { + processPath = applicationPhysicalPath / processPath; + } + // // The processPath is a path to the application executable // like: C:\test\MyApp.Exe or MyApp.Exe @@ -331,11 +331,18 @@ Finished: std::optional HOSTFXR_UTILITY::GetAbsolutePathToDotnet( + const fs::path & applicationPath, const fs::path & requestedPath ) { WLOG_INFOF(L"Resolving absolute path to dotnet.exe from %s", requestedPath.c_str()); + auto processPath = requestedPath; + if (processPath.is_relative()) + { + processPath = applicationPath / processPath; + } + // // If we are given an absolute path to dotnet.exe, we are done // @@ -360,7 +367,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet( // If we encounter any failures, try getting dotnet.exe from the // backup location. // Only do it if no path is specified - if (!requestedPath.has_parent_path()) + if (requestedPath.has_parent_path()) { return std::nullopt; } diff --git a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h index 9b9952f27a..dfdeb08b3a 100644 --- a/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h +++ b/src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.h @@ -91,6 +91,7 @@ public: static std::optional GetAbsolutePathToDotnet( + _In_ const std::experimental::filesystem::path & applicationPath, _In_ const std::experimental::filesystem::path & requestedPath ); }; diff --git a/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs b/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs index 051ae6a2dc..eda231066a 100644 --- a/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs +++ b/test/IISIntegration.FunctionalTests/Inprocess/StartupTests.cs @@ -33,20 +33,18 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests deploymentParameters => deploymentParameters.EnvironmentVariables["DotnetPath"] = _dotnetLocation); } - [ConditionalFact] - public async Task InvalidProcessPath_ExpectServerError() + [ConditionalTheory] + [InlineData("bogus")] + [InlineData("c:\\random files\\dotnet.exe")] + [InlineData(".\\dotnet.exe")] + public async Task InvalidProcessPath_ExpectServerError(string path) { - var dotnetLocation = "bogus"; - var deploymentParameters = GetBaseDeploymentParameters(); - // Point to dotnet installed in user profile. - deploymentParameters.EnvironmentVariables["DotnetPath"] = Environment.ExpandEnvironmentVariables(dotnetLocation); // Path to dotnet. var deploymentResult = await DeployAsync(deploymentParameters); - Helpers.ModifyAspNetCoreSectionInWebConfig(deploymentResult, "processPath", "%DotnetPath%"); + Helpers.ModifyAspNetCoreSectionInWebConfig(deploymentResult, "processPath", path); - // Request to base address and check if various parts of the body are rendered & measure the cold startup time. var response = await deploymentResult.RetryingHttpClient.GetAsync("HelloWorld"); Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);