Skip to content

Commit a7b7837

Browse files
pakrympakrym
authored
Fix ANCM environment variables bugs (#6083)
1 parent a25c7d9 commit a7b7837

File tree

20 files changed

+239
-585
lines changed

20 files changed

+239
-585
lines changed

‎.azure/pipelines/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ jobs:
3737
- template: jobs/iisintegration-job.yml
3838
parameters:
3939
TestGroupName: IISExpress
40-
skipArgs: "/p:SkipIISBackwardsCompatibilityTests=false /p:SkipIISTests=true /p:SkipIISExpressTests=false /p:SkipIISForwardsCompatibilityTests=true"
40+
skipArgs: "/p:SkipIISBackwardsCompatibilityTests=true /p:SkipIISTests=true /p:SkipIISExpressTests=false /p:SkipIISForwardsCompatibilityTests=true"
4141
- template: jobs/iisintegration-job.yml
4242
parameters:
4343
TestGroupName: IISForwardCompat

‎src/Servers/IIS/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include "StringHelpers.h"
77
#include "ConfigurationLoadException.h"
8+
#include <map>
89

910
std::wstring ConfigurationSection::GetRequiredString(const std::wstring& name) const
1011
{
@@ -63,6 +64,17 @@ std::vector<std::pair<std::wstring, std::wstring>> ConfigurationSection::GetKeyV
6364
return pairs;
6465
}
6566

67+
std::map<std::wstring, std::wstring, ignore_case_comparer> ConfigurationSection::GetMap(const std::wstring& name) const
68+
{
69+
std::map<std::wstring, std::wstring, ignore_case_comparer> pairs;
70+
71+
for (auto const element : GetRequiredSection(name)->GetCollection())
72+
{
73+
pairs.insert_or_assign(element->GetRequiredString(CS_ASPNETCORE_COLLECTION_ITEM_NAME), element->GetString(CS_ASPNETCORE_COLLECTION_ITEM_VALUE).value_or(L""));
74+
}
75+
return pairs;
76+
}
77+
6678
std::shared_ptr<ConfigurationSection> ConfigurationSection::GetRequiredSection(const std::wstring& name) const
6779
{
6880
auto section = GetSection(name);

‎src/Servers/IIS/src/AspNetCoreModuleV2/CommonLib/ConfigurationSection.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
#include <string>
77
#include <optional>
88
#include <vector>
9+
#include <map>
910

1011
#include "NonCopyable.h"
12+
#include "StringHelpers.h"
1113

1214
#define CS_ASPNETCORE_COLLECTION_ITEM_NAME L"name"
1315
#define CS_ASPNETCORE_COLLECTION_ITEM_VALUE L"value"
@@ -46,6 +48,7 @@ class ConfigurationSection: NonCopyable
4648
DWORD GetRequiredTimespan(const std::wstring& name) const;
4749

4850
virtual std::vector<std::pair<std::wstring, std::wstring>> GetKeyValuePairs(const std::wstring& name) const;
51+
virtual std::map<std::wstring, std::wstring, ignore_case_comparer> GetMap(const std::wstring& name) const;
4952

5053
virtual std::shared_ptr<ConfigurationSection> GetRequiredSection(const std::wstring & name) const;
5154

‎src/Servers/IIS/src/AspNetCoreModuleV2/CommonLib/StringHelpers.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,18 @@ bool ends_with(const std::wstring &source, const std::wstring &suffix, bool igno
1717

1818
bool equals_ignore_case(const std::wstring& s1, const std::wstring& s2)
1919
{
20-
return CSTR_EQUAL == CompareStringOrdinal(s1.c_str(), static_cast<int>(s1.length()), s2.c_str(), static_cast<int>(s2.length()), true);
20+
return compare_ignore_case(s1, s2) == 0;
21+
}
22+
23+
int compare_ignore_case(const std::wstring& s1, const std::wstring& s2)
24+
{
25+
return CompareStringOrdinal(s1.c_str(), static_cast<int>(s1.length()), s2.c_str(), static_cast<int>(s2.length()), true) - CSTR_EQUAL;
2126
}
2227

2328
std::wstring to_wide_string(const std::string &source, const unsigned int codePage)
2429
{
2530
// MultiByteToWideChar returns 0 on failure, which is also the same return value
26-
// for empty strings. Preemptive return.
31+
// for empty strings. Preemptive return.
2732
if (source.length() == 0)
2833
{
2934
return L"";

‎src/Servers/IIS/src/AspNetCoreModuleV2/CommonLib/StringHelpers.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ bool ends_with(const std::wstring &source, const std::wstring &suffix, bool igno
1111
[[nodiscard]]
1212
bool equals_ignore_case(const std::wstring& s1, const std::wstring& s2);
1313

14+
[[nodiscard]]
15+
int compare_ignore_case(const std::wstring& s1, const std::wstring& s2);
16+
1417
[[nodiscard]]
1518
std::wstring to_wide_string(const std::string &source, const unsigned int codePage);
1619

@@ -48,3 +51,9 @@ std::string format(const std::string& format, Args ... args)
4851
return result;
4952
}
5053

54+
struct ignore_case_comparer
55+
{
56+
bool operator() (const std::wstring & s1, const std::wstring & s2) const {
57+
return compare_ignore_case(s1, s2) == -1;
58+
}
59+
};

‎src/Servers/IIS/src/AspNetCoreModuleV2/CommonLib/exceptions.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ class ResultException: public std::runtime_error
134134
return condition;
135135
}
136136

137+
__declspec(noinline) inline VOID ReportException(LOCATION_ARGUMENTS const InvalidOperationException& exception)
138+
{
139+
TraceException(LOCATION_CALL exception);
140+
DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, "InvalidOperationException '%ls' caught at " LOCATION_FORMAT, exception.as_wstring().c_str(), LOCATION_CALL_ONLY);
141+
}
142+
137143
__declspec(noinline) inline VOID ReportException(LOCATION_ARGUMENTS const std::exception& exception)
138144
{
139145
TraceException(LOCATION_CALL exception);
@@ -165,6 +171,11 @@ __declspec(noinline) inline HRESULT CaughtExceptionHResult(LOCATION_ARGUMENTS_ON
165171
ReportException(LOCATION_CALL exception);
166172
return exception.GetResult();
167173
}
174+
catch (const InvalidOperationException& exception)
175+
{
176+
ReportException(LOCATION_CALL exception);
177+
return HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION);
178+
}
168179
catch (const std::exception& exception)
169180
{
170181
ReportException(LOCATION_CALL exception);

‎src/Servers/IIS/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ InProcessOptions::InProcessOptions(const ConfigurationSource &configurationSourc
5353
m_fStdoutLogEnabled = aspNetCoreSection->GetRequiredBool(CS_ASPNETCORE_STDOUT_LOG_ENABLED);
5454
m_struStdoutLogFile = aspNetCoreSection->GetRequiredString(CS_ASPNETCORE_STDOUT_LOG_FILE);
5555
m_fDisableStartUpErrorPage = aspNetCoreSection->GetRequiredBool(CS_ASPNETCORE_DISABLE_START_UP_ERROR_PAGE);
56-
m_environmentVariables = aspNetCoreSection->GetKeyValuePairs(CS_ASPNETCORE_ENVIRONMENT_VARIABLES);
56+
m_environmentVariables = aspNetCoreSection->GetMap(CS_ASPNETCORE_ENVIRONMENT_VARIABLES);
5757

5858
const auto handlerSettings = aspNetCoreSection->GetKeyValuePairs(CS_ASPNETCORE_HANDLER_SETTINGS);
5959
m_fSetCurrentDirectory = equals_ignore_case(find_element(handlerSettings, CS_ASPNETCORE_HANDLER_SET_CURRENT_DIRECTORY).value_or(L"true"), L"true");

‎src/Servers/IIS/src/AspNetCoreModuleV2/InProcessRequestHandler/InProcessOptions.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "BindingInformation.h"
88
#include "ConfigurationSource.h"
99
#include "WebConfigConfigurationSource.h"
10+
#include <map>
1011

1112
class InProcessOptions: NonCopyable
1213
{
@@ -87,7 +88,7 @@ class InProcessOptions: NonCopyable
8788
return m_dwShutdownTimeLimitInMS;
8889
}
8990

90-
const std::vector<std::pair<std::wstring, std::wstring>>&
91+
const std::map<std::wstring, std::wstring, ignore_case_comparer>&
9192
QueryEnvironmentVariables() const
9293
{
9394
return m_environmentVariables;
@@ -120,7 +121,7 @@ class InProcessOptions: NonCopyable
120121
bool m_fAnonymousAuthEnabled;
121122
DWORD m_dwStartupTimeLimitInMS;
122123
DWORD m_dwShutdownTimeLimitInMS;
123-
std::vector<std::pair<std::wstring, std::wstring>> m_environmentVariables;
124+
std::map<std::wstring, std::wstring, ignore_case_comparer> m_environmentVariables;
124125
std::vector<BindingInformation> m_bindingInformation;
125126

126127
protected:

‎src/Servers/IIS/src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -425,37 +425,20 @@ IN_PROCESS_APPLICATION::ClrThreadEntryPoint(const std::shared_ptr<ExecuteClrCont
425425
HRESULT
426426
IN_PROCESS_APPLICATION::SetEnvironmentVariablesOnWorkerProcess()
427427
{
428-
auto variables = m_pConfig->QueryEnvironmentVariables();
429-
430-
auto inputTable = std::unique_ptr<ENVIRONMENT_VAR_HASH, ENVIRONMENT_VAR_HASH_DELETER>(new ENVIRONMENT_VAR_HASH());
431-
RETURN_IF_FAILED(inputTable->Initialize(37 /*prime*/));
432-
// Copy environment variables to old style hash table
433-
for (auto & variable : variables)
434-
{
435-
auto pNewEntry = std::unique_ptr<ENVIRONMENT_VAR_ENTRY, ENVIRONMENT_VAR_ENTRY_DELETER>(new ENVIRONMENT_VAR_ENTRY());
436-
RETURN_IF_FAILED(pNewEntry->Initialize((variable.first + L"=").c_str(), variable.second.c_str()));
437-
RETURN_IF_FAILED(inputTable->InsertRecord(pNewEntry.get()));
438-
}
439-
440-
ENVIRONMENT_VAR_HASH* pHashTable = NULL;
441-
std::unique_ptr<ENVIRONMENT_VAR_HASH, ENVIRONMENT_VAR_HASH_DELETER> table;
442-
RETURN_IF_FAILED(ENVIRONMENT_VAR_HELPERS::InitEnvironmentVariablesTable(
443-
inputTable.get(),
428+
auto variables = ENVIRONMENT_VAR_HELPERS::InitEnvironmentVariablesTable(
429+
m_pConfig->QueryEnvironmentVariables(),
444430
m_pConfig->QueryWindowsAuthEnabled(),
445431
m_pConfig->QueryBasicAuthEnabled(),
446432
m_pConfig->QueryAnonymousAuthEnabled(),
433+
false, // fAddHostingStartup
447434
QueryApplicationPhysicalPath().c_str(),
448-
nullptr, /* pHttpsPort */
449-
&pHashTable));
435+
nullptr);
450436

451-
table.reset(pHashTable);
452-
453-
HRESULT hr = S_OK;
454-
table->Apply(ENVIRONMENT_VAR_HELPERS::AppendEnvironmentVariables, &hr);
455-
RETURN_IF_FAILED(hr);
456-
457-
table->Apply(ENVIRONMENT_VAR_HELPERS::SetEnvironmentVariables, &hr);
458-
RETURN_IF_FAILED(hr);
437+
for (const auto & variable : variables)
438+
{
439+
LOG_INFOF(L"Setting environment variable %ls=%ls", variable.first.c_str(), variable.second.c_str());
440+
SetEnvironmentVariable(variable.first.c_str(), variable.second.c_str());
441+
}
459442

460443
return S_OK;
461444
}

‎src/Servers/IIS/src/AspNetCoreModuleV2/OutOfProcessRequestHandler/serverprocess.cpp

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ SERVER_PROCESS::Initialize(
2424
BOOL fWindowsAuthEnabled,
2525
BOOL fBasicAuthEnabled,
2626
BOOL fAnonymousAuthEnabled,
27-
ENVIRONMENT_VAR_HASH *pEnvironmentVariables,
27+
std::map<std::wstring, std::wstring, ignore_case_comparer>& pEnvironmentVariables,
2828
BOOL fStdoutLogEnabled,
2929
BOOL fWebSocketSupported,
3030
STRU *pstruStdoutLogFile,
@@ -761,6 +761,8 @@ SERVER_PROCESS::StartProcess(
761761
ENVIRONMENT_VAR_HASH *pHashTable = NULL;
762762
PWSTR pStrStage = NULL;
763763
BOOL fCriticalError = FALSE;
764+
std::map<std::wstring, std::wstring, ignore_case_comparer> variables;
765+
764766
GetStartupInfoW(&startupInfo);
765767

766768
//
@@ -782,27 +784,30 @@ SERVER_PROCESS::StartProcess(
782784
goto Failure;
783785
}
784786

785-
if (FAILED_LOG(hr = ENVIRONMENT_VAR_HELPERS::InitEnvironmentVariablesTable(
786-
m_pEnvironmentVarTable,
787-
m_fWindowsAuthEnabled,
788-
m_fBasicAuthEnabled,
789-
m_fAnonymousAuthEnabled,
790-
m_struAppFullPath.QueryStr(),
791-
m_struHttpsPort.QueryStr(),
792-
&pHashTable)))
787+
try
793788
{
794-
pStrStage = L"InitEnvironmentVariablesTable";
795-
goto Failure;
789+
variables = ENVIRONMENT_VAR_HELPERS::InitEnvironmentVariablesTable(
790+
m_pEnvironmentVarTable,
791+
m_fWindowsAuthEnabled,
792+
m_fBasicAuthEnabled,
793+
m_fAnonymousAuthEnabled,
794+
true, // fAddHostingStartup
795+
m_struAppFullPath.QueryStr(),
796+
m_struHttpsPort.QueryStr());
797+
798+
variables = ENVIRONMENT_VAR_HELPERS::AddWebsocketEnabledToEnvironmentVariables(variables, m_fWebSocketSupported);
796799
}
800+
CATCH_RETURN();
797801

798-
if (FAILED_LOG(hr = ENVIRONMENT_VAR_HELPERS::AddWebsocketEnabledToEnvironmentVariables(
799-
pHashTable,
800-
m_fWebSocketSupported
801-
)))
802-
{
803-
pStrStage = L"AddWebsocketEnabledToEnvironmentVariables";
804-
goto Failure;
805802

803+
pHashTable = new ENVIRONMENT_VAR_HASH();
804+
RETURN_IF_FAILED(pHashTable->Initialize(37 /*prime*/));
805+
// Copy environment variables to old style hash table
806+
for (auto & variable : variables)
807+
{
808+
auto pNewEntry = std::unique_ptr<ENVIRONMENT_VAR_ENTRY, ENVIRONMENT_VAR_ENTRY_DELETER>(new ENVIRONMENT_VAR_ENTRY());
809+
RETURN_IF_FAILED(pNewEntry->Initialize((variable.first + L"=").c_str(), variable.second.c_str()));
810+
RETURN_IF_FAILED(pHashTable->InsertRecord(pNewEntry.get()));
806811
}
807812

808813
//
@@ -1793,7 +1798,6 @@ SERVER_PROCESS::~SERVER_PROCESS()
17931798

17941799
CleanUp();
17951800

1796-
m_pEnvironmentVarTable = NULL;
17971801
// no need to free m_pEnvironmentVarTable, as it references to
17981802
// the same hash table hold by configuration.
17991803
// the hashtable memory will be freed once onfiguration got recycled

0 commit comments

Comments
 (0)