diff --git a/korebuild-lock.txt b/korebuild-lock.txt index 36d8056037..39c6b28034 100644 --- a/korebuild-lock.txt +++ b/korebuild-lock.txt @@ -1,2 +1,2 @@ -version:2.1.0-preview1-15550 -commithash:0dd080d0d87b4d1966ec0af9961dc8bacc04f84f +version:2.1.0-preview1-15552 +commithash:526c2d8d521343e5a29c2a1323925528cb94e873 diff --git a/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs b/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs index 169985495f..8015f9c004 100644 --- a/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs +++ b/src/Microsoft.AspNetCore.Server.IISIntegration/NativeMethods.cs @@ -92,6 +92,12 @@ namespace Microsoft.AspNetCore.Server.IISIntegration [DllImport(AspNetCoreModuleDll)] public unsafe static extern int http_cancel_io(IntPtr pHttpContext); + [DllImport(AspNetCoreModuleDll)] + public unsafe static extern int http_response_set_unknown_header(IntPtr pHttpContext, byte* pszHeaderName, byte* pszHeaderValue, ushort usHeaderValueLength, bool fReplace); + + [DllImport(AspNetCoreModuleDll)] + internal unsafe static extern int http_response_set_known_header(IntPtr pHttpContext, int headerId, byte* pHeaderValue, ushort length, bool fReplace); + [DllImport("kernel32.dll")] public static extern IntPtr GetModuleHandle(string lpModuleName); @@ -99,5 +105,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration { return GetModuleHandle(AspNetCoreModuleDll) != IntPtr.Zero; } + } } diff --git a/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs b/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs index 7dcec90f46..a6520425e2 100644 --- a/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs +++ b/src/Microsoft.AspNetCore.Server.IISIntegration/Server/IISHttpContext.cs @@ -40,8 +40,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration protected Exception _applicationException; private readonly PipeFactory _pipeFactory; - private List _pinnedHeaders; - private GCHandle _thisHandle; private BufferHandle _inputHandle; private IISAwaitable _operation = new IISAwaitable(); @@ -193,8 +191,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration hr = NativeMethods.http_flush_response_bytes(_pHttpContext, out var fCompletionExpected); if (!fCompletionExpected) { - FreePinnedHeaders(_pinnedHeaders); - _pinnedHeaders = null; _operation.Complete(hr, 0); } return _operation; @@ -372,161 +368,35 @@ namespace Microsoft.AspNetCore.Server.IISIntegration NativeMethods.http_set_response_status_code(_pHttpContext, (ushort)StatusCode, pReasonPhrase); } - HttpApiTypes.HTTP_RESPONSE_V2* pHttpResponse = NativeMethods.http_get_raw_response(_pHttpContext); - - // We should be sending the headers here as there are many responses that don't have status codes. - _pinnedHeaders = SerializeHeaders(pHttpResponse); - } - - // From HttpSys, except does not write to response - private unsafe List SerializeHeaders(HttpApiTypes.HTTP_RESPONSE_V2* pHttpResponse) - { HttpResponseHeaders.IsReadOnly = true; - HttpApiTypes.HTTP_UNKNOWN_HEADER[] unknownHeaders = null; - HttpApiTypes.HTTP_RESPONSE_INFO[] knownHeaderInfo = null; - var pinnedHeaders = new List(); - GCHandle gcHandle; - - if (HttpResponseHeaders.Count == 0) - { - return null; - } - string headerName; - string headerValue; - int lookup; - var numUnknownHeaders = 0; - int numKnownMultiHeaders = 0; - byte[] bytes = null; - foreach (var headerPair in HttpResponseHeaders) { - if (headerPair.Value.Count == 0) + var headerValues = headerPair.Value; + var knownHeaderIndex = HttpApiTypes.HTTP_RESPONSE_HEADER_ID.IndexOfKnownHeader(headerPair.Key); + if (knownHeaderIndex == -1) { - continue; - } - lookup = HttpApiTypes.HTTP_RESPONSE_HEADER_ID.IndexOfKnownHeader(headerPair.Key); - if (lookup == -1) // TODO handle opaque stream upgrade? - { - numUnknownHeaders++; - } - else if (headerPair.Value.Count > 1) - { - numKnownMultiHeaders++; - } - } - - try - { - var pKnownHeaders = &pHttpResponse->Response_V1.Headers.KnownHeaders; - foreach (var headerPair in HttpResponseHeaders) - { - if (headerPair.Value.Count == 0) + var headerNameBytes = Encoding.UTF8.GetBytes(headerPair.Key); + for (var i = 0; i < headerValues.Count; i++) { - continue; - } - headerName = headerPair.Key; - StringValues headerValues = headerPair.Value; - lookup = HttpApiTypes.HTTP_RESPONSE_HEADER_ID.IndexOfKnownHeader(headerName); - if (lookup == -1) - { - if (unknownHeaders == null) + var headerValueBytes = Encoding.UTF8.GetBytes(headerValues[i]); + fixed (byte* pHeaderName = headerNameBytes) { - unknownHeaders = new HttpApiTypes.HTTP_UNKNOWN_HEADER[numUnknownHeaders]; - gcHandle = GCHandle.Alloc(unknownHeaders, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - pHttpResponse->Response_V1.Headers.pUnknownHeaders = (HttpApiTypes.HTTP_UNKNOWN_HEADER*)gcHandle.AddrOfPinnedObject(); - pHttpResponse->Response_V1.Headers.UnknownHeaderCount = 0; // to remove the iis header for server=... - } - - for (var headerValueIndex = 0; headerValueIndex < headerValues.Count; headerValueIndex++) - { - // Add Name - bytes = HeaderEncoding.GetBytes(headerName); - unknownHeaders[pHttpResponse->Response_V1.Headers.UnknownHeaderCount].NameLength = (ushort)bytes.Length; - gcHandle = GCHandle.Alloc(bytes, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - unknownHeaders[pHttpResponse->Response_V1.Headers.UnknownHeaderCount].pName = (byte*)gcHandle.AddrOfPinnedObject(); - - // Add Value - headerValue = headerValues[headerValueIndex] ?? string.Empty; - bytes = HeaderEncoding.GetBytes(headerValue); - unknownHeaders[pHttpResponse->Response_V1.Headers.UnknownHeaderCount].RawValueLength = (ushort)bytes.Length; - gcHandle = GCHandle.Alloc(bytes, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - unknownHeaders[pHttpResponse->Response_V1.Headers.UnknownHeaderCount].pRawValue = (byte*)gcHandle.AddrOfPinnedObject(); - pHttpResponse->Response_V1.Headers.UnknownHeaderCount++; + fixed (byte* pHeaderValue = headerValueBytes) + { + NativeMethods.http_response_set_unknown_header(_pHttpContext, pHeaderName, pHeaderValue, (ushort)headerValueBytes.Length, fReplace: false); + } } } - else if (headerPair.Value.Count == 1) - { - headerValue = headerValues[0] ?? string.Empty; - bytes = HeaderEncoding.GetBytes(headerValue); - pKnownHeaders[lookup].RawValueLength = (ushort)bytes.Length; - gcHandle = GCHandle.Alloc(bytes, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - pKnownHeaders[lookup].pRawValue = (byte*)gcHandle.AddrOfPinnedObject(); - } - else - { - if (knownHeaderInfo == null) - { - knownHeaderInfo = new HttpApiTypes.HTTP_RESPONSE_INFO[numKnownMultiHeaders]; - gcHandle = GCHandle.Alloc(knownHeaderInfo, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - pHttpResponse->pResponseInfo = (HttpApiTypes.HTTP_RESPONSE_INFO*)gcHandle.AddrOfPinnedObject(); - } - - knownHeaderInfo[pHttpResponse->ResponseInfoCount].Type = HttpApiTypes.HTTP_RESPONSE_INFO_TYPE.HttpResponseInfoTypeMultipleKnownHeaders; - knownHeaderInfo[pHttpResponse->ResponseInfoCount].Length = (uint)Marshal.SizeOf(); - - var header = new HttpApiTypes.HTTP_MULTIPLE_KNOWN_HEADERS(); - - header.HeaderId = (HttpApiTypes.HTTP_RESPONSE_HEADER_ID.Enum)lookup; - header.Flags = HttpApiTypes.HTTP_RESPONSE_INFO_FLAGS.PreserveOrder; // TODO: The docs say this is for www-auth only. - - var nativeHeaderValues = new HttpApiTypes.HTTP_KNOWN_HEADER[headerValues.Count]; - gcHandle = GCHandle.Alloc(nativeHeaderValues, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - header.KnownHeaders = (HttpApiTypes.HTTP_KNOWN_HEADER*)gcHandle.AddrOfPinnedObject(); - - for (int headerValueIndex = 0; headerValueIndex < headerValues.Count; headerValueIndex++) - { - // Add Value - headerValue = headerValues[headerValueIndex] ?? string.Empty; - bytes = HeaderEncoding.GetBytes(headerValue); - nativeHeaderValues[header.KnownHeaderCount].RawValueLength = (ushort)bytes.Length; - gcHandle = GCHandle.Alloc(bytes, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - nativeHeaderValues[header.KnownHeaderCount].pRawValue = (byte*)gcHandle.AddrOfPinnedObject(); - header.KnownHeaderCount++; - } - - // This type is a struct, not an object, so pinning it causes a boxed copy to be created. We can't do that until after all the fields are set. - gcHandle = GCHandle.Alloc(header, GCHandleType.Pinned); - pinnedHeaders.Add(gcHandle); - knownHeaderInfo[pHttpResponse->ResponseInfoCount].pInfo = (HttpApiTypes.HTTP_MULTIPLE_KNOWN_HEADERS*)gcHandle.AddrOfPinnedObject(); - - pHttpResponse->ResponseInfoCount++; - } } - } - catch (Exception) - { - FreePinnedHeaders(pinnedHeaders); - throw; - } - return pinnedHeaders; - } - - private static void FreePinnedHeaders(List pinnedHeaders) - { - if (pinnedHeaders != null) - { - foreach (GCHandle gcHandle in pinnedHeaders) + else { - if (gcHandle.IsAllocated) + for (var i = 0; i < headerValues.Count; i++) { - gcHandle.Free(); + var headerValueBytes = Encoding.UTF8.GetBytes(headerValues[i]); + fixed (byte* pHeaderValue = headerValueBytes) + { + NativeMethods.http_response_set_known_header(_pHttpContext, knownHeaderIndex, pHeaderValue, (ushort)headerValueBytes.Length, fReplace: false); + } } } } @@ -931,8 +801,6 @@ namespace Microsoft.AspNetCore.Server.IISIntegration _operation.Complete(hr, cbBytes); break; case CurrentOperationType.Flush: - FreePinnedHeaders(_pinnedHeaders); - _pinnedHeaders = null; _operation.Complete(hr, cbBytes); break; } diff --git a/test/IISIntegration.IISServerFunctionalTests/ResponseHeaderTests.cs b/test/IISIntegration.IISServerFunctionalTests/ResponseHeaderTests.cs new file mode 100644 index 0000000000..19a73c8004 --- /dev/null +++ b/test/IISIntegration.IISServerFunctionalTests/ResponseHeaderTests.cs @@ -0,0 +1,90 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Net; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.IntegrationTesting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; +using Microsoft.Net.Http.Headers; +using Xunit; +using Xunit.Abstractions; +using Xunit.Sdk; + +namespace Microsoft.AspNetCore.Server.IIS.FunctionalTests +{ + public class ResponseHeaders : LoggedTest + { + public ResponseHeaders(ITestOutputHelper output) : base(output) + { + } + + [Fact(Skip = "See https://github.com/aspnet/IISIntegration/issues/424")] + public Task AddResponseHeaders_HeaderValuesAreSetCorrectly() + { + return RunResponseHeaders(ApplicationType.Portable); + } + + private async Task RunResponseHeaders(ApplicationType applicationType) + { + var runtimeFlavor = RuntimeFlavor.CoreClr; + var serverType = ServerType.IISExpress; + var architecture = RuntimeArchitecture.x64; + var testName = $"ResponseHeaders_{runtimeFlavor}"; + using (StartLog(out var loggerFactory, testName)) + { + var logger = loggerFactory.CreateLogger("HelloWorldTest"); + + var deploymentParameters = new DeploymentParameters(Helpers.GetTestSitesPath(), serverType, runtimeFlavor, architecture) + { + EnvironmentName = "ResponseHeaders", // Will pick the Start class named 'StartupHelloWorld', + ServerConfigTemplateContent = (serverType == ServerType.IISExpress) ? File.ReadAllText("Http.config") : null, + SiteName = "HttpTestSite", // This is configured in the Http.config + TargetFramework = "netcoreapp2.0", + ApplicationType = applicationType + }; + + using (var deployer = ApplicationDeployerFactory.Create(deploymentParameters, loggerFactory)) + { + var deploymentResult = await deployer.DeployAsync(); + deploymentResult.HttpClient.Timeout = TimeSpan.FromSeconds(5); + + // Request to base address and check if various parts of the body are rendered & measure the cold startup time. + var response = await RetryHelper.RetryRequest(() => + { + return deploymentResult.HttpClient.GetAsync("/ResponseHeaders"); + }, logger, deploymentResult.HostShutdownToken, retryCount: 30); + + var responseText = await response.Content.ReadAsStringAsync(); + try + { + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("Request Complete", responseText); + + Assert.True(response.Headers.TryGetValues("UnknownHeader", out var headerValues)); + Assert.Equal("test123=foo", headerValues.First()); + + Assert.True(response.Content.Headers.TryGetValues(HeaderNames.ContentType, out headerValues)); + Assert.Equal("text/plain", headerValues.First()); + + Assert.True(response.Headers.TryGetValues("MultiHeader", out headerValues)); + Assert.Equal(2, headerValues.Count()); + Assert.Equal("1", headerValues.First()); + Assert.Equal("2", headerValues.Last()); + } + catch (XunitException) + { + logger.LogWarning(response.ToString()); + logger.LogWarning(responseText); + throw; + } + } + } + } + } +} diff --git a/test/IISTestSite/StartupResponseHeaders.cs b/test/IISTestSite/StartupResponseHeaders.cs new file mode 100644 index 0000000000..b6ed1bec4b --- /dev/null +++ b/test/IISTestSite/StartupResponseHeaders.cs @@ -0,0 +1,33 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; +using Microsoft.Net.Http.Headers; + +namespace IISTestSite +{ + public class StartupResponseHeaders + { + public void Configure(IApplicationBuilder app, ILoggerFactory loggerFactory) + { + app.Run(async context => + { + if (context.Request.Path.Equals("/ResponseHeaders")) + { + context.Response.Headers["UnknownHeader"] = "test123=foo"; + context.Response.ContentType = "text/plain"; + context.Response.Headers["MultiHeader"] = new StringValues(new string[] { "1", "2" }); + await context.Response.WriteAsync("Request Complete"); + } + }); + } + } +}