From 729a98adfc03442b27b3f7953704e7022a9e1829 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 30 Aug 2018 12:20:53 -0700 Subject: [PATCH] Call AllocConsole (#1351) --- .../CommonLib/FileOutputManager.cpp | 23 ++++++++++++++- .../CommonLib/FileOutputManager.h | 1 + .../CommonLib/LoggingHelpers.cpp | 7 ++++- .../CommonLib/PipeOutputManager.cpp | 23 ++++++++++++++- .../CommonLib/PipeOutputManager.h | 1 + .../Inprocess/StartupExceptionTests.cs | 29 +++++++++++++------ .../StartupExceptionWebSite/Program.cs | 10 +++++++ 7 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp index 15bc54cbe6..66292196a6 100644 --- a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.cpp @@ -20,7 +20,8 @@ FileOutputManager::FileOutputManager(bool fEnableNativeLogging) : m_disposed(false), stdoutWrapper(nullptr), stderrWrapper(nullptr), - m_fEnableNativeRedirection(fEnableNativeLogging) + m_fEnableNativeRedirection(fEnableNativeLogging), + m_fCreatedConsole(false) { InitializeSRWLock(&m_srwLock); } @@ -49,6 +50,21 @@ FileOutputManager::Start() STRU struPath; FILETIME processCreationTime; FILETIME dummyFileTime; + + // To make Console.* functions work, allocate a console + // in the current process. + if (!AllocConsole()) + { + // ERROR_ACCESS_DENIED means there is a console already present. + if (GetLastError() != ERROR_ACCESS_DENIED) + { + RETURN_LAST_ERROR(); + } + } + else + { + m_fCreatedConsole = true; + } // Concatenate the log file name and application path RETURN_IF_FAILED(FILE_UTILITY::ConvertPathToFullPath( @@ -129,6 +145,11 @@ FileOutputManager::Stop() m_disposed = true; + if (m_fCreatedConsole) + { + RETURN_LAST_ERROR_IF(!FreeConsole()); + } + if (m_hLogFileHandle == INVALID_HANDLE_VALUE) { return HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND); diff --git a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h index 94f6e19176..7cb3e452fa 100644 --- a/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h +++ b/src/AspNetCoreModuleV2/CommonLib/FileOutputManager.h @@ -35,6 +35,7 @@ private: STRA m_straFileContent; BOOL m_disposed; BOOL m_fEnableNativeRedirection; + BOOL m_fCreatedConsole; SRWLOCK m_srwLock{}; std::unique_ptr stdoutWrapper; std::unique_ptr stderrWrapper; diff --git a/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp b/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp index 69de017e7f..3d1aacf46d 100644 --- a/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/LoggingHelpers.cpp @@ -27,13 +27,18 @@ LoggingHelpers::CreateLoggingProvider( try { + // Check if there is an existing active console window before redirecting + // Window == IISExpress with active console window, don't redirect to a pipe + // if true. + CONSOLE_SCREEN_BUFFER_INFO dummy; + if (fIsLoggingEnabled) { auto manager = std::make_unique(fEnableNativeLogging); hr = manager->Initialize(pwzStdOutFileName, pwzApplicationPath); outputManager = std::move(manager); } - else if (!GetConsoleWindow()) + else if (!GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &dummy)) { outputManager = std::make_unique(fEnableNativeLogging); } diff --git a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp index 8ddfcb437f..20aafcc17e 100644 --- a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp +++ b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.cpp @@ -24,7 +24,8 @@ PipeOutputManager::PipeOutputManager(bool fEnableNativeLogging) : m_disposed(FALSE), m_fEnableNativeRedirection(fEnableNativeLogging), stdoutWrapper(nullptr), - stderrWrapper(nullptr) + stderrWrapper(nullptr), + m_fCreatedConsole(false) { InitializeSRWLock(&m_srwLock); } @@ -43,6 +44,21 @@ HRESULT PipeOutputManager::Start() HANDLE hStdErrReadPipe; HANDLE hStdErrWritePipe; + // To make Console.* functions work, allocate a console + // in the current process. + if (!AllocConsole()) + { + // ERROR_ACCESS_DENIED means there is a console already present. + if (GetLastError() != ERROR_ACCESS_DENIED) + { + RETURN_LAST_ERROR(); + } + } + else + { + m_fCreatedConsole = true; + } + RETURN_LAST_ERROR_IF(!CreatePipe(&hStdErrReadPipe, &hStdErrWritePipe, &saAttr, 0 /*nSize*/)); m_hErrReadPipe = hStdErrReadPipe; @@ -92,6 +108,11 @@ HRESULT PipeOutputManager::Stop() m_disposed = true; + if (m_fCreatedConsole) + { + FreeConsole(); + } + // Both pipe wrappers duplicate the pipe writer handle // meaning we are fine to close the handle too. if (m_hErrWritePipe != INVALID_HANDLE_VALUE) diff --git a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h index 0b51ff4038..e41bee6d10 100644 --- a/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h +++ b/src/AspNetCoreModuleV2/CommonLib/PipeOutputManager.h @@ -39,6 +39,7 @@ private: SRWLOCK m_srwLock {}; BOOL m_disposed; BOOL m_fEnableNativeRedirection; + BOOL m_fCreatedConsole; std::unique_ptr stdoutWrapper; std::unique_ptr stderrWrapper; }; diff --git a/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs b/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs index 1234d368fc..12eae66d01 100644 --- a/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs +++ b/test/Common.FunctionalTests/Inprocess/StartupExceptionTests.cs @@ -35,13 +35,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_RANDOM_VALUE"] = randomNumberString; - var deploymentResult = await DeployAsync(deploymentParameters); - - var response = await deploymentResult.HttpClient.GetAsync(path); - - Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); - - StopServer(); + await AssertFailsToStart(path, deploymentParameters); Assert.Contains(TestSink.Writes, context => context.Message.Contains($"Random number: {randomNumberString}")); } @@ -56,6 +50,25 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; + await AssertFailsToStart(path, deploymentParameters); + + Assert.Contains(TestSink.Writes, context => context.Message.Contains(new string('a', 4096))); + } + + [ConditionalFact] + public async Task CheckValidConsoleFunctions() + { + var path = "CheckConsoleFunctions"; + var deploymentParameters = _fixture.GetBaseDeploymentParameters(_fixture.StartupExceptionWebsite, publish: true); + deploymentParameters.WebConfigBasedEnvironmentVariables["ASPNETCORE_INPROCESS_STARTUP_VALUE"] = path; + + await AssertFailsToStart(path, deploymentParameters); + + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Is Console redirection: True")); + } + + private async Task AssertFailsToStart(string path, IntegrationTesting.IIS.IISDeploymentParameters deploymentParameters) + { var deploymentResult = await DeployAsync(deploymentParameters); var response = await deploymentResult.HttpClient.GetAsync(path); @@ -63,8 +76,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); StopServer(); - - Assert.Contains(TestSink.Writes, context => context.Message.Contains(new string('a', 4096))); } [ConditionalFact] diff --git a/test/WebSites/StartupExceptionWebSite/Program.cs b/test/WebSites/StartupExceptionWebSite/Program.cs index 283a0136ce..44c8b1f461 100644 --- a/test/WebSites/StartupExceptionWebSite/Program.cs +++ b/test/WebSites/StartupExceptionWebSite/Program.cs @@ -2,6 +2,8 @@ // 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.Text; namespace IISTestSite { @@ -41,6 +43,14 @@ namespace IISTestSite Console.Error.WriteLine(new string('a', 4096)); Console.Error.Flush(); } + else if (envVariable == "CheckConsoleFunctions") + { + // Call a bunch of console functions and make sure none return invalid handle. + Console.OutputEncoding = Encoding.UTF8; + Console.Title = "Test"; + Console.WriteLine($"Is Console redirection: {Console.IsOutputRedirected}"); + Console.BackgroundColor = ConsoleColor.Blue; + } } } }