From ad82bd31d8fc0b2e3d3993ee8108af270a779c4d Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 8 Mar 2018 11:53:12 -0800 Subject: [PATCH] Fix argument parsing for hostfxr, add native unit tests. (#635) --- .gitignore | 2 + IISIntegration.sln | 13 ++ build/build.msbuild | 1 + build/repo.targets | 2 + src/CommonLib/hostfxr_utility.cpp | 96 +++++---- src/CommonLib/hostfxr_utility.h | 8 +- .../AspNetCoreModuleTests.vcxproj | 182 ++++++++++++++++++ .../hostfxr_utility_tests.cpp | 94 +++++++++ test/AspNetCoreModuleTests/stdafx.cpp | 4 + test/AspNetCoreModuleTests/stdafx.h | 56 ++++++ test/AspNetCoreModuleTests/targetver.h | 6 + 11 files changed, 410 insertions(+), 54 deletions(-) create mode 100644 test/AspNetCoreModuleTests/AspNetCoreModuleTests.vcxproj create mode 100644 test/AspNetCoreModuleTests/hostfxr_utility_tests.cpp create mode 100644 test/AspNetCoreModuleTests/stdafx.cpp create mode 100644 test/AspNetCoreModuleTests/stdafx.h create mode 100644 test/AspNetCoreModuleTests/targetver.h diff --git a/.gitignore b/.gitignore index ead7cde62a..ab6f1a6257 100644 --- a/.gitignore +++ b/.gitignore @@ -55,6 +55,8 @@ src/AspNetCore/version.h src/RequestHandler/version.h src/CommonLib/aspnetcore_msg.h src/CommonLib/aspnetcore_msg.rc +test/AspNetCoreModuleTests/Debug +test/AspNetCoreModuleTests/Release .build *.VC.*db diff --git a/IISIntegration.sln b/IISIntegration.sln index d93cac5243..e4e486e24b 100644 --- a/IISIntegration.sln +++ b/IISIntegration.sln @@ -54,6 +54,8 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "RequestHandler", "src\Reque EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Server.IIS", "src\Microsoft.AspNetCore.Server.IIS\Microsoft.AspNetCore.Server.IIS.csproj", "{46A8612B-418B-4D70-B3A7-A21DD0627473}" EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "AspNetCoreModuleTests", "test\AspNetCoreModuleTests\AspNetCoreModuleTests.vcxproj", "{0692D963-DB10-4387-B3EA-460FBB9BD9A3}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -200,6 +202,16 @@ Global {46A8612B-418B-4D70-B3A7-A21DD0627473}.Release|x64.Build.0 = Release|Any CPU {46A8612B-418B-4D70-B3A7-A21DD0627473}.Release|x86.ActiveCfg = Release|Any CPU {46A8612B-418B-4D70-B3A7-A21DD0627473}.Release|x86.Build.0 = Release|Any CPU + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Debug|Any CPU.ActiveCfg = Debug|Win32 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Debug|x64.ActiveCfg = Debug|x64 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Debug|x64.Build.0 = Debug|x64 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Debug|x86.ActiveCfg = Debug|Win32 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Debug|x86.Build.0 = Debug|Win32 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Release|Any CPU.ActiveCfg = Release|Win32 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Release|x64.ActiveCfg = Release|x64 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Release|x64.Build.0 = Release|x64 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Release|x86.ActiveCfg = Release|Win32 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3}.Release|x86.Build.0 = Release|Win32 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -217,6 +229,7 @@ Global {55494E58-E061-4C4C-A0A8-837008E72F85} = {04B1EDB6-E967-4D25-89B9-E6F8304038CD} {D57EA297-6DC2-4BC0-8C91-334863327863} = {04B1EDB6-E967-4D25-89B9-E6F8304038CD} {46A8612B-418B-4D70-B3A7-A21DD0627473} = {04B1EDB6-E967-4D25-89B9-E6F8304038CD} + {0692D963-DB10-4387-B3EA-460FBB9BD9A3} = {EF30B533-D715-421A-92B7-92FEF460AC9C} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {DB4F868D-E1AE-4FD7-9333-69FA15B268C5} diff --git a/build/build.msbuild b/build/build.msbuild index c731cd9ff8..791fc36b34 100644 --- a/build/build.msbuild +++ b/build/build.msbuild @@ -4,6 +4,7 @@ + diff --git a/build/repo.targets b/build/repo.targets index fe6a0b4c50..c7f0916c3b 100644 --- a/build/repo.targets +++ b/build/repo.targets @@ -24,6 +24,8 @@ Condition="'$(VisualStudioMSBuildx86Path)' != ''" /> + diff --git a/src/CommonLib/hostfxr_utility.cpp b/src/CommonLib/hostfxr_utility.cpp index 2331d11f64..b7fe5a1eea 100644 --- a/src/CommonLib/hostfxr_utility.cpp +++ b/src/CommonLib/hostfxr_utility.cpp @@ -159,7 +159,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( PCWSTR pcwzArguments, _Inout_ STRU* struHostFxrDllLocation, _Out_ DWORD* pdwArgCount, - _Out_ PWSTR** ppwzArgv + _Out_ BSTR** pbstrArgv ) { HRESULT hr = S_OK; @@ -198,7 +198,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( hEventLog, struHostFxrDllLocation, pdwArgCount, - ppwzArgv); + pbstrArgv); goto Finished; } } @@ -322,7 +322,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters( pcwzApplicationPhysicalPath, hEventLog, pdwArgCount, - ppwzArgv))) + pbstrArgv))) { goto Finished; } @@ -352,7 +352,7 @@ HOSTFXR_UTILITY::ParseHostfxrArguments( PCWSTR pcwzApplicationPhysicalPath, HANDLE hEventLog, _Out_ DWORD* pdwArgCount, - _Out_ PWSTR** ppwzArgv + _Out_ BSTR** pbstrArgv ) { UNREFERENCED_PARAMETER( hEventLog ); // TODO use event log to set errors. @@ -362,10 +362,18 @@ HOSTFXR_UTILITY::ParseHostfxrArguments( HRESULT hr = S_OK; INT argc = 0; - PWSTR* argv = NULL; + BSTR* argv = NULL; LPWSTR* pwzArgs = NULL; STRU struTempPath; - DWORD dwArgsProcessed = 0; + INT intArgsProcessed = 0; + + // If we call CommandLineToArgvW with an empty string, argc is 5 for some interesting reason. + // Protectively guard against this by check if the string is null or empty. + if (pwzArgumentsFromConfig == NULL || wcscmp(pwzArgumentsFromConfig, L"") == 0) + { + hr = E_INVALIDARG; + goto Finished; + } pwzArgs = CommandLineToArgvW(pwzArgumentsFromConfig, &argc); @@ -375,14 +383,7 @@ HOSTFXR_UTILITY::ParseHostfxrArguments( goto Failure; } - if (argc < 1) - { - // Invalid arguments - hr = E_INVALIDARG; - goto Failure; - } - - argv = new PWSTR[argc + 2]; + argv = new PWSTR[argc + 1]; if (argv == NULL) { hr = E_OUTOFMEMORY; @@ -396,55 +397,52 @@ HOSTFXR_UTILITY::ParseHostfxrArguments( hr = E_OUTOFMEMORY; goto Failure; } - dwArgsProcessed++; - - argv[1] = SysAllocString(L"exec"); - if (argv[1] == NULL) - { - hr = E_OUTOFMEMORY; - goto Failure; - } - dwArgsProcessed++; // Try to convert the application dll from a relative to an absolute path // Don't record this failure as pwzArgs[0] may already be an absolute path to the dll. - if (SUCCEEDED(UTILITY::ConvertPathToFullPath(pwzArgs[0], pcwzApplicationPhysicalPath, &struTempPath))) + for (intArgsProcessed = 0; intArgsProcessed < argc; intArgsProcessed++) { - argv[2] = SysAllocString(struTempPath.QueryStr()); - } - else - { - argv[2] = SysAllocString(pwzArgs[0]); - } - if (argv[2] == NULL) - { - hr = E_OUTOFMEMORY; - goto Failure; - } - dwArgsProcessed++; - - for (INT i = 1; i < argc; i++) - { - argv[i + 2] = SysAllocString(pwzArgs[i]); - if (argv[i + 2] == NULL) + struTempPath.Copy(pwzArgs[intArgsProcessed]); + if (struTempPath.EndsWith(L".dll")) { - hr = E_OUTOFMEMORY; - goto Failure; + if (SUCCEEDED(UTILITY::ConvertPathToFullPath(pwzArgs[intArgsProcessed], pcwzApplicationPhysicalPath, &struTempPath))) + { + argv[intArgsProcessed + 1] = SysAllocString(struTempPath.QueryStr()); + } + else + { + argv[intArgsProcessed + 1] = SysAllocString(pwzArgs[intArgsProcessed]); + } + if (argv[intArgsProcessed + 1] == NULL) + { + hr = E_OUTOFMEMORY; + goto Failure; + } + } + else + { + argv[intArgsProcessed + 1] = SysAllocString(pwzArgs[intArgsProcessed]); + if (argv[intArgsProcessed + 1] == NULL) + { + hr = E_OUTOFMEMORY; + goto Failure; + } } - dwArgsProcessed++; } - *ppwzArgv = argv; - *pdwArgCount = dwArgsProcessed; + *pbstrArgv = argv; + *pdwArgCount = argc + 1; goto Finished; Failure: if (argv != NULL) { - for (DWORD i = 0; i < dwArgsProcessed; i++) + // intArgsProcess - 1 here as if we fail to allocated the ith string + // we don't want to free it. + for (INT i = 0; i < intArgsProcessed - 1; i++) { - SysFreeString((BSTR)argv[i]); + SysFreeString(argv[i]); } } @@ -458,11 +456,11 @@ Finished: } return hr; } + // // Invoke where.exe to find the location of dotnet.exe // Copies contents of dotnet.exe to a temp file // Respects path ordering. - HRESULT HOSTFXR_UTILITY::FindDotnetExePath( _Out_ STRU* struDotnetPath diff --git a/src/CommonLib/hostfxr_utility.h b/src/CommonLib/hostfxr_utility.h index 556e833705..2effba31f1 100644 --- a/src/CommonLib/hostfxr_utility.h +++ b/src/CommonLib/hostfxr_utility.h @@ -23,10 +23,9 @@ public: PCWSTR pcwzArguments, _Inout_ STRU* struHostFxrDllLocation, _Out_ DWORD* pdwArgCount, - _Out_ PWSTR** ppwzArgv + _Out_ BSTR** ppwzArgv ); -private: static HRESULT GetStandaloneHostfxrParameters( @@ -36,7 +35,7 @@ private: HANDLE hEventLog, _Inout_ STRU* struHostFxrDllLocation, _Out_ DWORD* pdwArgCount, - _Out_ PWSTR** ppwzArgv + _Out_ BSTR** ppwzArgv ); static @@ -47,10 +46,9 @@ private: PCWSTR pcwzApplicationPhysicalPath, HANDLE hEventLog, _Out_ DWORD* pdwArgCount, - _Out_ PWSTR** ppwzArgv + _Out_ BSTR** ppwzArgv ); - static HRESULT FindDotnetExePath( diff --git a/test/AspNetCoreModuleTests/AspNetCoreModuleTests.vcxproj b/test/AspNetCoreModuleTests/AspNetCoreModuleTests.vcxproj new file mode 100644 index 0000000000..f4f7c8217e --- /dev/null +++ b/test/AspNetCoreModuleTests/AspNetCoreModuleTests.vcxproj @@ -0,0 +1,182 @@ + + + + + Debug + Win32 + + + Release + Win32 + + + Debug + x64 + + + Release + x64 + + + + 15.0 + {0692D963-DB10-4387-B3EA-460FBB9BD9A3} + Win32Proj + AspNetCoreModuleTests + 10.0.16299.0 + NativeUnitTestProject + + + + DynamicLibrary + true + v141 + Unicode + false + + + DynamicLibrary + false + v141 + true + Unicode + false + + + DynamicLibrary + true + v141 + Unicode + false + + + DynamicLibrary + false + v141 + true + Unicode + false + + + + + + + + + + + + + + + + + + + + + true + + + true + + + true + + + true + + + + Use + Level3 + Disabled + $(VCInstallDir)UnitTest\include;%(AdditionalIncludeDirectories);..\..\src\CommonLib;..\..\src\IISLib + _DEBUG;%(PreprocessorDefinitions) + true + MultiThreadedDebug + + + Windows + $(VCInstallDir)UnitTest\lib;%(AdditionalLibraryDirectories) + + + + + Use + Level3 + Disabled + $(VCInstallDir)UnitTest\include;%(AdditionalIncludeDirectories);..\..\src\CommonLib;..\..\src\IISLib + WIN32;_DEBUG;%(PreprocessorDefinitions) + true + MultiThreadedDebug + + + Windows + $(VCInstallDir)UnitTest\lib;%(AdditionalLibraryDirectories) + + + + + Level3 + Use + MaxSpeed + true + true + $(VCInstallDir)UnitTest\include;%(AdditionalIncludeDirectories);..\..\src\CommonLib;..\..\src\IISLib + WIN32;NDEBUG;%(PreprocessorDefinitions) + true + MultiThreaded + + + Windows + true + true + $(VCInstallDir)UnitTest\lib;%(AdditionalLibraryDirectories) + + + + + Level3 + Use + MaxSpeed + true + true + $(VCInstallDir)UnitTest\include;%(AdditionalIncludeDirectories);..\..\src\CommonLib;..\..\src\IISLib + NDEBUG;%(PreprocessorDefinitions) + true + MultiThreaded + + + Windows + true + true + $(VCInstallDir)UnitTest\lib;%(AdditionalLibraryDirectories) + + + + + + + + + Create + Create + Create + Create + + + + + + {55494e58-e061-4c4c-a0a8-837008e72f85} + + + {4787a64f-9a3e-4867-a55a-70cb4b2b2ffe} + + + + + + \ No newline at end of file diff --git a/test/AspNetCoreModuleTests/hostfxr_utility_tests.cpp b/test/AspNetCoreModuleTests/hostfxr_utility_tests.cpp new file mode 100644 index 0000000000..dfa0e3c4b9 --- /dev/null +++ b/test/AspNetCoreModuleTests/hostfxr_utility_tests.cpp @@ -0,0 +1,94 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#include "stdafx.h" +#include "CppUnitTest.h" + +using namespace Microsoft::VisualStudio::CppUnitTestFramework; + +namespace AspNetCoreModuleTests +{ + TEST_CLASS(HOSTFXR_UTILITY_TESTS) + { + public: + + TEST_METHOD(ParseHostfxrArguments_BasicHostFxrArguments) + { + DWORD retVal = 0; + BSTR* bstrArray; + PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; + + HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( + L"exec \"test.dll\"", // args + exeStr, // exe path + L"invalid", // physical path to application + NULL, // event log + &retVal, // arg count + &bstrArray); // args array. + + Assert::AreEqual(hr, S_OK); + Assert::AreEqual(DWORD(3), retVal); + Assert::AreEqual(exeStr, bstrArray[0]); + Assert::AreEqual(L"exec", bstrArray[1]); + Assert::AreEqual(L"test.dll", bstrArray[2]); + } + + TEST_METHOD(ParseHostfxrArguments_NoExecProvided) + { + DWORD retVal = 0; + BSTR* bstrArray; + PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; + + HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( + L"test.dll", // args + exeStr, // exe path + L"ignored", // physical path to application + NULL, // event log + &retVal, // arg count + &bstrArray); // args array. + + Assert::AreEqual(hr, S_OK); + Assert::AreEqual(DWORD(2), retVal); + Assert::AreEqual(exeStr, bstrArray[0]); + Assert::AreEqual(L"test.dll", bstrArray[1]); + } + + TEST_METHOD(ParseHostfxrArguments_ConvertDllToAbsolutePath) + { + DWORD retVal = 0; + BSTR* bstrArray; + PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; + + HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( + L"exec \"test.dll\"", // args + exeStr, // exe path + L"C:/test", // physical path to application + NULL, // event log + &retVal, // arg count + &bstrArray); // args array. + + Assert::AreEqual(hr, S_OK); + Assert::AreEqual(DWORD(3), retVal); + Assert::AreEqual(exeStr, bstrArray[0]); + Assert::AreEqual(L"exec", bstrArray[1]); + Assert::AreEqual(L"\\\\?\\C:\\test\\test.dll", bstrArray[2]); + } + + TEST_METHOD(ParseHostfxrArguments_ProvideNoArgs_InvalidArgs) + { + DWORD retVal = 0; + BSTR* bstrArray; + PCWSTR exeStr = L"C:/Program Files/dotnet.exe"; + + HRESULT hr = HOSTFXR_UTILITY::ParseHostfxrArguments( + L"", // args + exeStr, // exe path + L"ignored", // physical path to application + NULL, // event log + &retVal, // arg count + &bstrArray); // args array. + + Assert::AreEqual(E_INVALIDARG, hr); + } + }; +} diff --git a/test/AspNetCoreModuleTests/stdafx.cpp b/test/AspNetCoreModuleTests/stdafx.cpp new file mode 100644 index 0000000000..0351feb240 --- /dev/null +++ b/test/AspNetCoreModuleTests/stdafx.cpp @@ -0,0 +1,4 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#include "stdafx.h" diff --git a/test/AspNetCoreModuleTests/stdafx.h b/test/AspNetCoreModuleTests/stdafx.h new file mode 100644 index 0000000000..4b67a04738 --- /dev/null +++ b/test/AspNetCoreModuleTests/stdafx.h @@ -0,0 +1,56 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#pragma once + +#include "targetver.h" + +#define WIN32_LEAN_AND_MEAN + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include "stringa.h" +#include "stringu.h" +#include "dbgutil.h" +#include "ahutil.h" +#include "multisz.h" +#include "multisza.h" +#include "base64.h" +#include +#include +#include +#include +#include + +#include "..\..\src\IISLib\hashtable.h" +#include "..\..\src\IISLib\stringu.h" +#include "..\..\src\IISLib\stringa.h" +#include "..\..\src\IISLib\multisz.h" +#include "..\..\src\IISLib\dbgutil.h" +#include "..\..\src\IISLib\ahutil.h" +#include "..\..\src\IISLib\hashfn.h" + +#include "..\..\src\CommonLib\hostfxr_utility.h" +#include "..\..\src\CommonLib\environmentvariablehash.h" +#include "..\..\src\CommonLib\aspnetcoreconfig.h" +#include "..\..\src\CommonLib\application.h" +#include "..\..\src\CommonLib\utility.h" +#include "..\..\src\CommonLib\debugutil.h" +#include "..\..\src\CommonLib\requesthandler.h" +#include "..\..\src\CommonLib\resources.h" +#include "..\..\src\CommonLib\aspnetcore_msg.h" + +#include "CppUnitTest.h" diff --git a/test/AspNetCoreModuleTests/targetver.h b/test/AspNetCoreModuleTests/targetver.h new file mode 100644 index 0000000000..0617356416 --- /dev/null +++ b/test/AspNetCoreModuleTests/targetver.h @@ -0,0 +1,6 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +#pragma once + +#include