From ed801f0e882b5e32b93856cf465db0f52ce54807 Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Thu, 22 Mar 2018 12:21:10 -0700 Subject: [PATCH] Do not return 200 for exceptions --- .../Properties/launchSettings.json | 1 + .../NativeMethods.cs | 4 +- .../Server/IISHttpContext.cs | 11 ++-- src/RequestHandler/managedexports.cxx | 6 +- .../IISIntegration.FunctionalTests.csproj | 1 + .../Inprocess/ResponseHeaderTests.cs | 20 +++++++ test/IISTestSite/Startup.cs | 55 +++++++++---------- 7 files changed, 58 insertions(+), 40 deletions(-) diff --git a/samples/NativeIISSample/Properties/launchSettings.json b/samples/NativeIISSample/Properties/launchSettings.json index 7d09a120ab..6d5ce43f73 100644 --- a/samples/NativeIISSample/Properties/launchSettings.json +++ b/samples/NativeIISSample/Properties/launchSettings.json @@ -12,6 +12,7 @@ "commandName": "Executable", "executablePath": "$(IISExpressPath)", "commandLineArgs": "$(IISExpressArguments)", + "nativeDebugging": true, "environmentVariables": { "IIS_SITE_PATH": "$(MSBuildThisFileDirectory)", "ANCM_PATH": "$(TargetDir)$(AncmPath)", diff --git a/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs b/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs index 8cfd0b256f..00c9d5beb1 100644 --- a/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs +++ b/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs @@ -68,8 +68,8 @@ namespace Microsoft.AspNetCore.Server.IISIntegration [DllImport(AspNetCoreModuleDll)] internal unsafe static extern HttpApiTypes.HTTP_RESPONSE_V2* http_get_raw_response(IntPtr pInProcessHandler); - [DllImport(AspNetCoreModuleDll)] - public unsafe static extern void http_set_response_status_code(IntPtr pInProcessHandler, ushort statusCode, byte* pszReason); + [DllImport(AspNetCoreModuleDll, CharSet = CharSet.Ansi)] + public unsafe static extern int http_set_response_status_code(IntPtr pInProcessHandler, ushort statusCode, string pszReason); [DllImport(AspNetCoreModuleDll)] public unsafe static extern int http_read_request_bytes(IntPtr pInProcessHandler, byte* pvBuffer, int cbBuffer, out int dwBytesReceived, out bool fCompletionExpected); diff --git a/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs b/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs index f6cbe57255..32afb07033 100644 --- a/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs +++ b/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs @@ -223,7 +223,7 @@ namespace Microsoft.AspNetCore.Server.IISIntegration _reasonPhrase = value; } } - + internal IISHttpServer Server { get { return _server; } @@ -332,13 +332,10 @@ namespace Microsoft.AspNetCore.Server.IISIntegration public unsafe void SendResponseHeaders(bool appCompleted) { // Verifies we have sent the statuscode before writing a header - var reasonPhraseBytes = Encoding.UTF8.GetBytes(ReasonPhrase ?? ReasonPhrases.GetReasonPhrase(StatusCode)); + var reasonPhrase = string.IsNullOrEmpty(ReasonPhrase) ? ReasonPhrases.GetReasonPhrase(StatusCode) : ReasonPhrase; - fixed (byte* pReasonPhrase = reasonPhraseBytes) - { - // This copies data into the underlying buffer - NativeMethods.http_set_response_status_code(_pInProcessHandler, (ushort)StatusCode, pReasonPhrase); - } + // This copies data into the underlying buffer + NativeMethods.http_set_response_status_code(_pInProcessHandler, (ushort)StatusCode, reasonPhrase); HttpResponseHeaders.IsReadOnly = true; foreach (var headerPair in HttpResponseHeaders) diff --git a/src/RequestHandler/managedexports.cxx b/src/RequestHandler/managedexports.cxx index 80672a6667..46510a00b7 100644 --- a/src/RequestHandler/managedexports.cxx +++ b/src/RequestHandler/managedexports.cxx @@ -74,13 +74,15 @@ Finished: return hr; } -EXTERN_C __MIDL_DECLSPEC_DLLEXPORT VOID http_set_response_status_code( +EXTERN_C __MIDL_DECLSPEC_DLLEXPORT +HRESULT +http_set_response_status_code( _In_ IN_PROCESS_HANDLER* pInProcessHandler, _In_ USHORT statusCode, _In_ PCSTR pszReason ) { - pInProcessHandler->QueryHttpContext()->GetResponse()->SetStatus(statusCode, pszReason); + return pInProcessHandler->QueryHttpContext()->GetResponse()->SetStatus(statusCode, pszReason); } EXTERN_C __MIDL_DECLSPEC_DLLEXPORT diff --git a/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj b/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj index 77f3646a70..c09eeee095 100644 --- a/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj +++ b/test/IISIntegration.FunctionalTests/IISIntegration.FunctionalTests.csproj @@ -10,6 +10,7 @@ + diff --git a/test/IISIntegration.FunctionalTests/Inprocess/ResponseHeaderTests.cs b/test/IISIntegration.FunctionalTests/Inprocess/ResponseHeaderTests.cs index 0ab845f1d4..47ca0dc808 100644 --- a/test/IISIntegration.FunctionalTests/Inprocess/ResponseHeaderTests.cs +++ b/test/IISIntegration.FunctionalTests/Inprocess/ResponseHeaderTests.cs @@ -40,5 +40,25 @@ namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests Assert.Equal("1", headerValues.First()); Assert.Equal("2", headerValues.Last()); } + + [ConditionalFact] + public async Task ErrorCodeIsSetForExceptionDuringRequest() + { + var response = await _fixture.Client.GetAsync("Throw"); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + Assert.Equal("Internal Server Error", response.ReasonPhrase); + } + + [ConditionalTheory] + [InlineData(200, "custom", "custom")] + [InlineData(500, "", "Internal Server Error")] + [InlineData(999, "", "")] + public async Task CustomErrorCodeWorks(int code, string reason, string expectedReason) + { + var response = await _fixture.Client.GetAsync($"SetCustomErorCode?code={code}&reason={reason}"); + Assert.Equal((HttpStatusCode)code, response.StatusCode); + Assert.Equal(expectedReason, response.ReasonPhrase); + Assert.Equal("Body", await response.Content.ReadAsStringAsync()); + } } } diff --git a/test/IISTestSite/Startup.cs b/test/IISTestSite/Startup.cs index 24e7e5e457..00b83e769d 100644 --- a/test/IISTestSite/Startup.cs +++ b/test/IISTestSite/Startup.cs @@ -5,6 +5,7 @@ using System; using System.Diagnostics; using System.IO; using System.Net; +using System.Reflection; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -23,35 +24,16 @@ namespace IISTestSite { public void Configure(IApplicationBuilder app) { - app.Map("/ServerVariable", ServerVariable); - app.Map("/AuthenticationAnonymous", AuthenticationAnonymous); - app.Map("/AuthenticationRestricted", AuthenticationRestricted); - app.Map("/AuthenticationForbidden", AuthenticationForbidden); - app.Map("/AuthenticationRestrictedNTLM", AuthenticationRestrictedNTLM); - app.Map("/FeatureCollectionSetRequestFeatures", FeatureCollectionSetRequestFeatures); - app.Map("/FeatureCollectionSetResponseFeatures", FeatureCollectionSetResponseFeatures); - app.Map("/FeatureCollectionSetConnectionFeatures", FeatureCollectionSetConnectionFeatures); - app.Map("/HelloWorld", HelloWorld); - app.Map("/LargeResponseBody", LargeResponseBody); - app.Map("/ResponseHeaders", ResponseHeaders); - app.Map("/ResponseInvalidOrdering", ResponseInvalidOrdering); - app.Map("/CheckEnvironmentVariable", CheckEnvironmentVariable); - app.Map("/CheckEnvironmentLongValueVariable", CheckEnvironmentLongValueVariable); - app.Map("/CheckAppendedEnvironmentVariable", CheckAppendedEnvironmentVariable); - app.Map("/CheckRemoveAuthEnvironmentVariable", CheckRemoveAuthEnvironmentVariable); - app.Map("/ReadAndWriteSynchronously", ReadAndWriteSynchronously); - app.Map("/ReadAndWriteEcho", ReadAndWriteEcho); - app.Map("/ReadAndWriteCopyToAsync", ReadAndWriteCopyToAsync); - app.Map("/ReadAndWriteEchoTwice", ReadAndWriteEchoTwice); - app.Map("/ReadAndWriteSlowConnection", ReadAndWriteSlowConnection); - app.Map("/WebsocketRequest", WebsocketRequest); - app.Map("/UpgradeFeatureDetection", UpgradeFeatureDetection); - app.Map("/TestInvalidReadOperations", TestInvalidReadOperations); - app.Map("/TestValidReadOperations", TestValidReadOperations); - app.Map("/TestInvalidWriteOperations", TestInvalidWriteOperations); - app.Map("/TestValidWriteOperations", TestValidWriteOperations); - app.Map("/TestReadOffsetWorks", TestReadOffsetWorks); - app.Map("/LargeResponseFile", LargeResponseFile); + foreach (var method in GetType().GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) + { + var parameters = method.GetParameters(); + if (method.Name != nameof(Configure) && + parameters.Length == 1 && + parameters[0].ParameterType == typeof(IApplicationBuilder)) + { + app.Map("/" + method.Name, innerAppBuilder => method.Invoke(this, new[] { innerAppBuilder })); + } + } } private void ServerVariable(IApplicationBuilder app) @@ -231,6 +213,21 @@ namespace IISTestSite }); } + private void Throw(IApplicationBuilder app) + { + app.Run(ctx => { throw new Exception(); }); + } + + private void SetCustomErorCode(IApplicationBuilder app) + { + app.Run(async ctx => { + var feature = ctx.Features.Get(); + feature.ReasonPhrase = ctx.Request.Query["reason"]; + feature.StatusCode = int.Parse(ctx.Request.Query["code"]); + await ctx.Response.WriteAsync("Body"); + }); + } + private void HelloWorld(IApplicationBuilder app) { app.Run(async ctx =>