From 812f2f81459ebbdce783aeaccf7c7d4584233044 Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 11 Jun 2020 10:57:36 -0700 Subject: [PATCH] Fixes NullRef exception in IIS in-proc (#22806) --- .../inprocesshandler.cpp | 9 +- .../IIS/IIS/src/Core/IISHttpContext.cs | 118 +++++++++--------- src/Servers/IIS/IIS/src/Core/IISHttpServer.cs | 4 +- 3 files changed, 70 insertions(+), 61 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.cpp b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.cpp index 883e4ff49d..0328adec4e 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.cpp +++ b/src/Servers/IIS/AspNetCoreModuleV2/InProcessRequestHandler/inprocesshandler.cpp @@ -112,7 +112,7 @@ IN_PROCESS_HANDLER::NotifyDisconnect() if (pManagedHttpContext != nullptr) { - m_pDisconnectHandler(m_pManagedHttpContext); + m_pDisconnectHandler(pManagedHttpContext); } } @@ -139,14 +139,17 @@ IN_PROCESS_HANDLER::SetManagedHttpContext( PVOID pManagedHttpContext ) { + bool disconnectFired = false; + { SRWExclusiveLock lock(m_srwDisconnectLock); m_pManagedHttpContext = pManagedHttpContext; + disconnectFired = m_disconnectFired; } - if (m_disconnectFired && m_pManagedHttpContext != nullptr) + if (disconnectFired && pManagedHttpContext != nullptr) { - m_pDisconnectHandler(m_pManagedHttpContext); + m_pDisconnectHandler(pManagedHttpContext); } } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs index cae357e776..3f7d3c53ce 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs @@ -119,70 +119,76 @@ namespace Microsoft.AspNetCore.Server.IIS.Core protected void InitializeContext() { - _thisHandle = GCHandle.Alloc(this); - - Method = GetVerb(); - - RawTarget = GetRawUrl(); - // TODO version is slow. - HttpVersion = GetVersion(); - Scheme = SslStatus != SslStatus.Insecure ? Constants.HttpsScheme : Constants.HttpScheme; - KnownMethod = VerbId; - StatusCode = 200; - - var originalPath = GetOriginalPath(); - - if (KnownMethod == HttpApiTypes.HTTP_VERB.HttpVerbOPTIONS && string.Equals(RawTarget, "*", StringComparison.Ordinal)) + // create a memory barrier between initialize and disconnect to prevent a possible + // NullRef with disconnect being called before these fields have been written + // disconnect aquires this lock as well + lock (_abortLock) { - PathBase = string.Empty; - Path = string.Empty; - } - else - { - // Path and pathbase are unescaped by RequestUriBuilder - // The UsePathBase middleware will modify the pathbase and path correctly - PathBase = string.Empty; - Path = originalPath; - } + _thisHandle = GCHandle.Alloc(this); - var cookedUrl = GetCookedUrl(); - QueryString = cookedUrl.GetQueryString() ?? string.Empty; + Method = GetVerb(); - RequestHeaders = new RequestHeaders(this); - HttpResponseHeaders = new HeaderCollection(); - ResponseHeaders = HttpResponseHeaders; + RawTarget = GetRawUrl(); + // TODO version is slow. + HttpVersion = GetVersion(); + Scheme = SslStatus != SslStatus.Insecure ? Constants.HttpsScheme : Constants.HttpScheme; + KnownMethod = VerbId; + StatusCode = 200; - if (_options.ForwardWindowsAuthentication) - { - WindowsUser = GetWindowsPrincipal(); - if (_options.AutomaticAuthentication) + var originalPath = GetOriginalPath(); + + if (KnownMethod == HttpApiTypes.HTTP_VERB.HttpVerbOPTIONS && string.Equals(RawTarget, "*", StringComparison.Ordinal)) { - User = WindowsUser; + PathBase = string.Empty; + Path = string.Empty; } + else + { + // Path and pathbase are unescaped by RequestUriBuilder + // The UsePathBase middleware will modify the pathbase and path correctly + PathBase = string.Empty; + Path = originalPath; + } + + var cookedUrl = GetCookedUrl(); + QueryString = cookedUrl.GetQueryString() ?? string.Empty; + + RequestHeaders = new RequestHeaders(this); + HttpResponseHeaders = new HeaderCollection(); + ResponseHeaders = HttpResponseHeaders; + + if (_options.ForwardWindowsAuthentication) + { + WindowsUser = GetWindowsPrincipal(); + if (_options.AutomaticAuthentication) + { + User = WindowsUser; + } + } + + MaxRequestBodySize = _options.MaxRequestBodySize; + + ResetFeatureCollection(); + + if (!_server.IsWebSocketAvailable(_pInProcessHandler)) + { + _currentIHttpUpgradeFeature = null; + } + + _streams = new Streams(this); + + (RequestBody, ResponseBody) = _streams.Start(); + + var pipe = new Pipe( + new PipeOptions( + _memoryPool, + readerScheduler: PipeScheduler.ThreadPool, + pauseWriterThreshold: PauseWriterThreshold, + resumeWriterThreshold: ResumeWriterTheshold, + minimumSegmentSize: MinAllocBufferSize)); + _bodyOutput = new OutputProducer(pipe); } - MaxRequestBodySize = _options.MaxRequestBodySize; - - ResetFeatureCollection(); - - if (!_server.IsWebSocketAvailable(_pInProcessHandler)) - { - _currentIHttpUpgradeFeature = null; - } - - _streams = new Streams(this); - - (RequestBody, ResponseBody) = _streams.Start(); - - var pipe = new Pipe( - new PipeOptions( - _memoryPool, - readerScheduler: PipeScheduler.ThreadPool, - pauseWriterThreshold: PauseWriterThreshold, - resumeWriterThreshold: ResumeWriterTheshold, - minimumSegmentSize: MinAllocBufferSize)); - _bodyOutput = new OutputProducer(pipe); - NativeMethods.HttpSetManagedContext(_pInProcessHandler, (IntPtr)_thisHandle); } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs b/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs index cb6c3db598..ef4c3aabb8 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpServer.cs @@ -170,7 +170,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core try { context = (IISHttpContext)GCHandle.FromIntPtr(pvManagedHttpContext).Target; - context.AbortIO(clientDisconnect: true); + context?.AbortIO(clientDisconnect: true); } catch (Exception ex) { @@ -184,7 +184,7 @@ namespace Microsoft.AspNetCore.Server.IIS.Core try { context = (IISHttpContext)GCHandle.FromIntPtr(pvManagedHttpContext).Target; - context.OnAsyncCompletion(hr, bytes); + context?.OnAsyncCompletion(hr, bytes); return NativeMethods.REQUEST_NOTIFICATION_STATUS.RQ_NOTIFICATION_PENDING; } catch (Exception ex)