Remove header serialization in favor of setting IIS response directly. (#465)

korebuild-lock.txt

@@ -1,2 +1,2 @@
-version:2.1.0-preview1-15550
+version:2.1.0-preview1-15552
-commithash:0dd080d0d87b4d1966ec0af9961dc8bacc04f84f
+commithash:526c2d8d521343e5a29c2a1323925528cb94e873

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;
         }
+
     }
 }

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<GCHandle> _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<GCHandle> 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 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;
+                    var headerNameBytes = Encoding.UTF8.GetBytes(headerPair.Key);
-                }
+                    for (var i = 0; i < headerValues.Count; i++)
-                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)
                     {
-                        continue;
+                        var headerValueBytes = Encoding.UTF8.GetBytes(headerValues[i]);
-                    }
+                        fixed (byte* pHeaderName = headerNameBytes)
-                    headerName = headerPair.Key;
-                    StringValues headerValues = headerPair.Value;
-                    lookup = HttpApiTypes.HTTP_RESPONSE_HEADER_ID.IndexOfKnownHeader(headerName);
-                    if (lookup == -1)
-                    {
-                        if (unknownHeaders == null)
                         {
-                            unknownHeaders = new HttpApiTypes.HTTP_UNKNOWN_HEADER[numUnknownHeaders];
+                            fixed (byte* pHeaderValue = headerValueBytes)
-                            gcHandle = GCHandle.Alloc(unknownHeaders, GCHandleType.Pinned);
+                            {
-                            pinnedHeaders.Add(gcHandle);
+                                NativeMethods.http_response_set_unknown_header(_pHttpContext, pHeaderName, pHeaderValue, (ushort)headerValueBytes.Length, fReplace: false);
-                            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++;
                         }
                     }
-                    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<HttpApiTypes.HTTP_MULTIPLE_KNOWN_HEADERS>();
-
-                        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++;
-                    }
                 }
-            }
+                else
-            catch (Exception)
-            {
-                FreePinnedHeaders(pinnedHeaders);
-                throw;
-            }
-            return pinnedHeaders;
-        }
-
-        private static void FreePinnedHeaders(List<GCHandle> pinnedHeaders)
-        {
-            if (pinnedHeaders != null)
-            {
-                foreach (GCHandle gcHandle in pinnedHeaders)
                 {
-                    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;
             }

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;
+                    }
+                }
+            }
+        }
+    }
+}

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");
+                }
+            });
+        }
+    }
+}