Several improvements to SocketsHttpHandler perf (dotnet/corefx#41640)
authorStephen Toub <stoub@microsoft.com>
Wed, 9 Oct 2019 01:22:47 +0000 (21:22 -0400)
committerGitHub <noreply@github.com>
Wed, 9 Oct 2019 01:22:47 +0000 (21:22 -0400)
* Do not force-parse TryAddWithoutValidation headers in SocketsHttpHandler

When enumerating headers to write them out, do not force them to be parsed if the user explicitly asked for them not to be with "WithoutValidation".  If the key/values are incorrectly formatted, the request may end up being written incorrectly on the wire, but that's up to the developer explicitly choosing it.

* Revert HttpRequestHeaders.ExpectContinue optimization

Several releases ago, when we weren't paying attention to ExpectContinue, we optimized away the backing field for it into a lazily-initialized collection; that made it cheaper when not accessed but more expensive when accessed, which was fine, as we wouldn't access it from the implementation and developers would rarely set it.  But now SocketsHttpHandler checks it on every request, which means we're paying for the more expensive thing always. So, revert the optimization for this field.

* Avoid allocating a string[] per header

When we enumerate the headers to write them out, we currently allocate a string[] for each.  We can instead just fill the same array over and over and over.

Commit migrated from https://github.com/dotnet/corefx/commit/4d346a9ab7b6e619dae3784283acb87bb7a825c9

src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs
src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs

index 8439e74..958e663 100644 (file)
@@ -58,6 +58,8 @@ namespace System.Net.Http.Headers
             _treatAsCustomHeaderTypes = treatAsCustomHeaderTypes;
         }
 
+        internal Dictionary<HeaderDescriptor, HeaderStoreItemInfo> HeaderStore => _headerStore;
+
         public void Add(string name, string value)
         {
             Add(GetHeaderDescriptor(name), value);
@@ -254,13 +256,10 @@ namespace System.Net.Http.Headers
             // HeaderName1: Value1, Value2
             // HeaderName2: Value1
             // ...
-            StringBuilder sb = new StringBuilder();
-            foreach (var header in GetHeaderStrings())
+            var sb = new StringBuilder();
+            foreach (KeyValuePair<string, string> header in GetHeaderStrings())
             {
-                sb.Append(header.Key);
-                sb.Append(": ");
-
-                sb.AppendLine(header.Value);
+                sb.Append(header.Key).Append(": ").AppendLine(header.Value);
             }
 
             return sb.ToString();
@@ -273,7 +272,7 @@ namespace System.Net.Http.Headers
                 yield break;
             }
 
-            foreach (var header in _headerStore)
+            foreach (KeyValuePair<HeaderDescriptor, HeaderStoreItemInfo> header in _headerStore)
             {
                 string stringValue = GetHeaderString(header.Key, header.Value);
 
@@ -338,7 +337,7 @@ namespace System.Net.Http.Headers
 
         private IEnumerator<KeyValuePair<string, IEnumerable<string>>> GetEnumeratorCore()
         {
-            foreach (var header in _headerStore)
+            foreach (KeyValuePair<HeaderDescriptor, HeaderStoreItemInfo> header in _headerStore)
             {
                 HeaderDescriptor descriptor = header.Key;
                 HeaderStoreItemInfo info = header.Value;
@@ -359,39 +358,6 @@ namespace System.Net.Http.Headers
             }
         }
 
-        // The following is the same general code as the above GetEnumerator, but returning the
-        // HeaderDescriptor and values string[], rather than the key name and a values enumerable.
-
-        internal IEnumerable<KeyValuePair<HeaderDescriptor, string[]>> GetHeaderDescriptorsAndValues()
-        {
-            return _headerStore != null && _headerStore.Count > 0 ?
-                GetHeaderDescriptorsAndValuesCore() :
-                Array.Empty<KeyValuePair<HeaderDescriptor, string[]>>();
-        }
-
-        private IEnumerable<KeyValuePair<HeaderDescriptor, string[]>> GetHeaderDescriptorsAndValuesCore()
-        {
-            foreach (var header in _headerStore)
-            {
-                HeaderDescriptor descriptor = header.Key;
-                HeaderStoreItemInfo info = header.Value;
-
-                // Make sure we parse all raw values before returning the result. Note that this has to be
-                // done before we calculate the array length (next line): A raw value may contain a list of
-                // values.
-                if (!ParseRawHeaderValues(descriptor, info, false))
-                {
-                    // We have an invalid header value (contains invalid newline chars). Delete it.
-                    _headerStore.Remove(descriptor);
-                }
-                else
-                {
-                    string[] values = GetValuesAsStrings(descriptor, info);
-                    yield return new KeyValuePair<HeaderDescriptor, string[]>(descriptor, values);
-                }
-            }
-        }
-
         #endregion
 
         #region IEnumerable Members
@@ -595,7 +561,7 @@ namespace System.Net.Http.Headers
                 return;
             }
 
-            foreach (var header in sourceHeaders._headerStore)
+            foreach (KeyValuePair<HeaderDescriptor, HeaderStoreItemInfo> header in sourceHeaders._headerStore)
             {
                 // Only add header values if they're not already set on the message. Note that we don't merge
                 // collections: If both the default headers and the message have set some values for a certain
@@ -1197,9 +1163,7 @@ namespace System.Net.Http.Headers
                 // The values array may not be full because some values were excluded
                 if (currentIndex < length)
                 {
-                    string[] trimmedValues = new string[currentIndex];
-                    Array.Copy(values, 0, trimmedValues, 0, currentIndex);
-                    values = trimmedValues;
+                    values = values.AsSpan(0, currentIndex).ToArray();
                 }
             }
             else
@@ -1211,34 +1175,41 @@ namespace System.Net.Http.Headers
             return values;
         }
 
-        private static int GetValueCount(HeaderStoreItemInfo info)
+        internal static int GetValuesAsStrings(HeaderDescriptor descriptor, HeaderStoreItemInfo info, ref string[] values)
         {
-            Debug.Assert(info != null);
+            Debug.Assert(values != null);
+            int length = GetValueCount(info);
+
+            if (length > 0)
+            {
+                if (values.Length < length)
+                {
+                    values = new string[length];
+                }
 
-            int valueCount = 0;
-            UpdateValueCount<string>(info.RawValue, ref valueCount);
-            UpdateValueCount<string>(info.InvalidValue, ref valueCount);
-            UpdateValueCount<object>(info.ParsedValue, ref valueCount);
+                int currentIndex = 0;
+                ReadStoreValues<string>(values, info.RawValue, null, null, ref currentIndex);
+                ReadStoreValues<object>(values, info.ParsedValue, descriptor.Parser, null, ref currentIndex);
+                ReadStoreValues<string>(values, info.InvalidValue, null, null, ref currentIndex);
+                Debug.Assert(currentIndex == length);
+            }
 
-            return valueCount;
+            return length;
         }
 
-        private static void UpdateValueCount<T>(object valueStore, ref int valueCount)
+        private static int GetValueCount(HeaderStoreItemInfo info)
         {
-            if (valueStore == null)
-            {
-                return;
-            }
+            Debug.Assert(info != null);
 
-            List<T> values = valueStore as List<T>;
-            if (values != null)
-            {
-                valueCount += values.Count;
-            }
-            else
-            {
-                valueCount++;
-            }
+            int valueCount = Count<string>(info.RawValue);
+            valueCount += Count<string>(info.InvalidValue);
+            valueCount += Count<object>(info.ParsedValue);
+            return valueCount;
+
+            static int Count<T>(object valueStore) =>
+                valueStore is null ? 0 :
+                valueStore is List<T> list ? list.Count :
+                1;
         }
 
         private static void ReadStoreValues<T>(string[] values, object storeValue, HttpHeaderParser parser,
@@ -1304,34 +1275,17 @@ namespace System.Net.Http.Headers
 
 #region Private Classes
 
-        private class HeaderStoreItemInfo
+        internal class HeaderStoreItemInfo
         {
-            private object _rawValue;
-            private object _invalidValue;
-            private object _parsedValue;
-
-            internal object RawValue
-            {
-                get { return _rawValue; }
-                set { _rawValue = value; }
-            }
-
-            internal object InvalidValue
-            {
-                get { return _invalidValue; }
-                set { _invalidValue = value; }
-            }
+            internal HeaderStoreItemInfo() { }
 
-            internal object ParsedValue
-            {
-                get { return _parsedValue; }
-                set { _parsedValue = value; }
-            }
+            internal object RawValue { get; set; }
+            internal object InvalidValue { get; set; }
+            internal object ParsedValue { get; set; }
 
             internal bool CanAddValue(HttpHeaderParser parser)
             {
-                Debug.Assert(parser != null,
-                    "There should be no reason to call CanAddValue if there is no parser for the current header.");
+                Debug.Assert(parser != null, "There should be no reason to call CanAddValue if there is no parser for the current header.");
 
                 // If the header only supports one value, and we have already a value set, then we can't add
                 // another value. E.g. the 'Date' header only supports one value. We can't add multiple timestamps
@@ -1342,17 +1296,10 @@ namespace System.Net.Http.Headers
                 // supporting 1 value. When the first value gets parsed, CanAddValue returns true and we add the
                 // parsed value to ParsedValue. When the second value is parsed, CanAddValue returns false, because
                 // we have already a parsed value.
-                return ((parser.SupportsMultipleValues) || ((_invalidValue == null) && (_parsedValue == null)));
-            }
-
-            internal bool IsEmpty
-            {
-                get { return ((_rawValue == null) && (_invalidValue == null) && (_parsedValue == null)); }
+                return parser.SupportsMultipleValues || ((InvalidValue == null) && (ParsedValue == null));
             }
 
-            internal HeaderStoreItemInfo()
-            {
-            }
+            internal bool IsEmpty => (RawValue == null) && (InvalidValue == null) && (ParsedValue == null);
         }
 #endregion
     }
index c49e36a..cd564e8 100644 (file)
@@ -15,31 +15,26 @@ namespace System.Net.Http.Headers
         private const int AcceptCharsetSlot = 1;
         private const int AcceptEncodingSlot = 2;
         private const int AcceptLanguageSlot = 3;
-        private const int ExpectSlot = 4;
-        private const int IfMatchSlot = 5;
-        private const int IfNoneMatchSlot = 6;
-        private const int TransferEncodingSlot = 7;
-        private const int UserAgentSlot = 8;
-        private const int NumCollectionsSlots = 9;
+        private const int IfMatchSlot = 4;
+        private const int IfNoneMatchSlot = 5;
+        private const int TransferEncodingSlot = 6;
+        private const int UserAgentSlot = 7;
+        private const int NumCollectionsSlots = 8;
 
         private object[] _specialCollectionsSlots;
         private HttpGeneralHeaders _generalHeaders;
+        private HttpHeaderValueCollection<NameValueWithParametersHeaderValue> _expect;
         private bool _expectContinueSet;
 
         #region Request Headers
 
         private T GetSpecializedCollection<T>(int slot, Func<HttpRequestHeaders, T> creationFunc)
         {
-            // 9 properties each lazily allocate a collection to store the value(s) for that property.
+            // 8 properties each lazily allocate a collection to store the value(s) for that property.
             // Rather than having a field for each of these, store them untyped in an array that's lazily
-            // allocated.  Then we only pay for the 72 bytes for those fields when any is actually accessed.
-            object[] collections = _specialCollectionsSlots ?? (_specialCollectionsSlots = new object[NumCollectionsSlots]);
-            object result = collections[slot];
-            if (result == null)
-            {
-                collections[slot] = result = creationFunc(this);
-            }
-            return (T)result;
+            // allocated.  Then we only pay for the 64 bytes for those fields when any is actually accessed.
+            _specialCollectionsSlots ??= new object[NumCollectionsSlots];
+            return (T)(_specialCollectionsSlots[slot] ??= creationFunc(this));
         }
 
         public HttpHeaderValueCollection<MediaTypeWithQualityHeaderValue> Accept =>
@@ -198,7 +193,7 @@ namespace System.Net.Http.Headers
             GetSpecializedCollection(UserAgentSlot, thisRef => new HttpHeaderValueCollection<ProductInfoHeaderValue>(KnownHeaders.UserAgent.Descriptor, thisRef));
 
         private HttpHeaderValueCollection<NameValueWithParametersHeaderValue> ExpectCore =>
-            GetSpecializedCollection(ExpectSlot, thisRef => new HttpHeaderValueCollection<NameValueWithParametersHeaderValue>(KnownHeaders.Expect.Descriptor, thisRef, HeaderUtilities.ExpectContinue));
+            _expect ??= new HttpHeaderValueCollection<NameValueWithParametersHeaderValue>(KnownHeaders.Expect.Descriptor, this, HeaderUtilities.ExpectContinue);
 
         #endregion
 
index cc9b1f7..0472d3c 100644 (file)
@@ -262,7 +262,7 @@ namespace System.Net.Http.HPack
         }
 
         /// <summary>Encodes a "Literal Header Field without Indexing - New Name".</summary>
-        public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, string[] values, string separator, Span<byte> destination, out int bytesWritten)
+        public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, ReadOnlySpan<string> values, string separator, Span<byte> destination, out int bytesWritten)
         {
             // From https://tools.ietf.org/html/rfc7541#section-6.2.2
             // ------------------------------------------------------
@@ -422,7 +422,7 @@ namespace System.Net.Http.HPack
             return false;
         }
 
-        public static bool EncodeStringLiterals(string[] values, string separator, Span<byte> destination, out int bytesWritten)
+        public static bool EncodeStringLiterals(ReadOnlySpan<string> values, string separator, Span<byte> destination, out int bytesWritten)
         {
             bytesWritten = 0;
 
index 4d1bc25..e73667b 100644 (file)
@@ -26,6 +26,9 @@ namespace System.Net.Http
         private ArrayBuffer _outgoingBuffer;
         private ArrayBuffer _headerBuffer;
 
+        /// <summary>Reusable array used to get the values for each header being written to the wire.</summary>
+        private string[] _headerValues = Array.Empty<string>();
+
         private int _currentWriteSize;      // as passed to StartWriteAsync
 
         private readonly HPackDecoder _hpackDecoder;
@@ -893,9 +896,9 @@ namespace System.Net.Http
             _headerBuffer.Commit(bytesWritten);
         }
 
-        private void WriteLiteralHeader(string name, string[] values)
+        private void WriteLiteralHeader(string name, ReadOnlySpan<string> values)
         {
-            if (NetEventSource.IsEnabled) Trace($"{nameof(name)}={name}, {nameof(values)}={string.Join(", ", values)}");
+            if (NetEventSource.IsEnabled) Trace($"{nameof(name)}={name}, {nameof(values)}={string.Join(", ", values.ToArray())}");
 
             int bytesWritten;
             while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparator, _headerBuffer.AvailableSpan, out bytesWritten))
@@ -906,9 +909,9 @@ namespace System.Net.Http
             _headerBuffer.Commit(bytesWritten);
         }
 
-        private void WriteLiteralHeaderValues(string[] values, string separator)
+        private void WriteLiteralHeaderValues(ReadOnlySpan<string> values, string separator)
         {
-            if (NetEventSource.IsEnabled) Trace($"{nameof(values)}={string.Join(separator, values)}");
+            if (NetEventSource.IsEnabled) Trace($"{nameof(values)}={string.Join(separator, values.ToArray())}");
 
             int bytesWritten;
             while (!HPackEncoder.EncodeStringLiterals(values, separator, _headerBuffer.AvailableSpan, out bytesWritten))
@@ -949,9 +952,16 @@ namespace System.Net.Http
         {
             if (NetEventSource.IsEnabled) Trace("");
 
-            foreach (KeyValuePair<HeaderDescriptor, string[]> header in headers.GetHeaderDescriptorsAndValues())
+            if (headers.HeaderStore is null)
+            {
+                return;
+            }
+
+            foreach (KeyValuePair<HeaderDescriptor, HttpHeaders.HeaderStoreItemInfo> header in headers.HeaderStore)
             {
-                Debug.Assert(header.Value.Length > 0, "No values for header??");
+                int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref _headerValues);
+                Debug.Assert(headerValuesCount > 0, "No values for header??");
+                ReadOnlySpan<string> headerValues = _headerValues.AsSpan(0, headerValuesCount);
 
                 KnownHeader knownHeader = header.Key.KnownHeader;
                 if (knownHeader != null)
@@ -964,7 +974,7 @@ namespace System.Net.Http
                         if (header.Key.KnownHeader == KnownHeaders.TE)
                         {
                             // HTTP/2 allows only 'trailers' TE header. rfc7540 8.1.2.2
-                            foreach (string value in header.Value)
+                            foreach (string value in headerValues)
                             {
                                 if (string.Equals(value, "trailers", StringComparison.OrdinalIgnoreCase))
                                 {
@@ -979,7 +989,7 @@ namespace System.Net.Http
                         // For all other known headers, send them via their pre-encoded name and the associated value.
                         WriteBytes(knownHeader.Http2EncodedName);
                         string separator = null;
-                        if (header.Value.Length > 1)
+                        if (headerValues.Length > 1)
                         {
                             HttpHeaderParser parser = header.Key.Parser;
                             if (parser != null && parser.SupportsMultipleValues)
@@ -992,13 +1002,13 @@ namespace System.Net.Http
                             }
                         }
 
-                        WriteLiteralHeaderValues(header.Value, separator);
+                        WriteLiteralHeaderValues(headerValues, separator);
                     }
                 }
                 else
                 {
                     // The header is not known: fall back to just encoding the header name and value(s).
-                    WriteLiteralHeader(header.Key.Name, header.Value);
+                    WriteLiteralHeader(header.Key.Name, headerValues);
                 }
             }
         }
index 0e1a252..438903c 100644 (file)
@@ -78,6 +78,8 @@ namespace System.Net.Http
         private readonly byte[] _writeBuffer;
         private int _writeOffset;
         private int _allowedReadLineBytes;
+        /// <summary>Reusable array used to get the values for each header being written to the wire.</summary>
+        private string[] _headerValues = Array.Empty<string>();
 
         private ValueTask<int>? _readAheadTask;
         private int _readAheadTaskLock = 0; // 0 == free, 1 == held
@@ -217,50 +219,54 @@ namespace System.Net.Http
 
         private async Task WriteHeadersAsync(HttpHeaders headers, string cookiesFromContainer)
         {
-            foreach (KeyValuePair<HeaderDescriptor, string[]> header in headers.GetHeaderDescriptorsAndValues())
+            if (headers.HeaderStore != null)
             {
-                if (header.Key.KnownHeader != null)
+                foreach (KeyValuePair<HeaderDescriptor, HttpHeaders.HeaderStoreItemInfo> header in headers.HeaderStore)
                 {
-                    await WriteBytesAsync(header.Key.KnownHeader.AsciiBytesWithColonSpace).ConfigureAwait(false);
-                }
-                else
-                {
-                    await WriteAsciiStringAsync(header.Key.Name).ConfigureAwait(false);
-                    await WriteTwoBytesAsync((byte)':', (byte)' ').ConfigureAwait(false);
-                }
-
-                Debug.Assert(header.Value.Length > 0, "No values for header??");
-                if (header.Value.Length > 0)
-                {
-                    await WriteStringAsync(header.Value[0]).ConfigureAwait(false);
-
-                    if (cookiesFromContainer != null && header.Key.KnownHeader == KnownHeaders.Cookie)
+                    if (header.Key.KnownHeader != null)
                     {
-                        await WriteTwoBytesAsync((byte)';', (byte)' ').ConfigureAwait(false);
-                        await WriteStringAsync(cookiesFromContainer).ConfigureAwait(false);
-
-                        cookiesFromContainer = null;
+                        await WriteBytesAsync(header.Key.KnownHeader.AsciiBytesWithColonSpace).ConfigureAwait(false);
+                    }
+                    else
+                    {
+                        await WriteAsciiStringAsync(header.Key.Name).ConfigureAwait(false);
+                        await WriteTwoBytesAsync((byte)':', (byte)' ').ConfigureAwait(false);
                     }
 
-                    // Some headers such as User-Agent and Server use space as a separator (see: ProductInfoHeaderParser)
-                    if (header.Value.Length > 1)
+                    int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref _headerValues);
+                    Debug.Assert(headerValuesCount > 0, "No values for header??");
+                    if (headerValuesCount > 0)
                     {
-                        HttpHeaderParser parser = header.Key.Parser;
-                        string separator = HttpHeaderParser.DefaultSeparator;
-                        if (parser != null && parser.SupportsMultipleValues)
+                        await WriteStringAsync(_headerValues[0]).ConfigureAwait(false);
+
+                        if (cookiesFromContainer != null && header.Key.KnownHeader == KnownHeaders.Cookie)
                         {
-                            separator = parser.Separator;
+                            await WriteTwoBytesAsync((byte)';', (byte)' ').ConfigureAwait(false);
+                            await WriteStringAsync(cookiesFromContainer).ConfigureAwait(false);
+
+                            cookiesFromContainer = null;
                         }
 
-                        for (int i = 1; i < header.Value.Length; i++)
+                        // Some headers such as User-Agent and Server use space as a separator (see: ProductInfoHeaderParser)
+                        if (headerValuesCount > 1)
                         {
-                            await WriteAsciiStringAsync(separator).ConfigureAwait(false);
-                            await WriteStringAsync(header.Value[i]).ConfigureAwait(false);
+                            HttpHeaderParser parser = header.Key.Parser;
+                            string separator = HttpHeaderParser.DefaultSeparator;
+                            if (parser != null && parser.SupportsMultipleValues)
+                            {
+                                separator = parser.Separator;
+                            }
+
+                            for (int i = 1; i < headerValuesCount; i++)
+                            {
+                                await WriteAsciiStringAsync(separator).ConfigureAwait(false);
+                                await WriteStringAsync(_headerValues[i]).ConfigureAwait(false);
+                            }
                         }
                     }
-                }
 
-                await WriteTwoBytesAsync((byte)'\r', (byte)'\n').ConfigureAwait(false);
+                    await WriteTwoBytesAsync((byte)'\r', (byte)'\n').ConfigureAwait(false);
+                }
             }
 
             if (cookiesFromContainer != null)
index 6d85a2e..be82404 100644 (file)
@@ -48,7 +48,7 @@ namespace System.Net.Http.Functional.Tests
         [Theory]
         [InlineData("\u05D1\u05F1")]
         [InlineData("jp\u30A5")]
-        public async Task SendAsync_InvalidHeader_Throw(string value)
+        public async Task SendAsync_InvalidCharactersInHeader_Throw(string value)
         {
             await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
             {
@@ -73,25 +73,45 @@ namespace System.Net.Http.Functional.Tests
             });
         }
 
-        [Fact]
-        public async Task SendAsync_SpecialCharacterHeader_Success()
+        [Theory]
+        [InlineData("x-Special_name", "header name with underscore", true)] // underscores in header
+        [InlineData("Date", "invaliddateformat", false)] // invalid format for header but added with TryAddWithoutValidation
+        [InlineData("Accept-CharSet", "text/plain, text/json", false)] // invalid format for header but added with TryAddWithoutValidation
+        [InlineData("Content-Location", "", false)] // invalid format for header but added with TryAddWithoutValidation
+        [InlineData("Max-Forwards", "NotAnInteger", false)] // invalid format for header but added with TryAddWithoutValidation
+        public async Task SendAsync_SpecialHeaderKeyOrValue_Success(string key, string value, bool parsable)
         {
-            string headerValue = "header name with underscore";
             await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
             {
+                bool contentHeader = false;
                 using (HttpClient client = CreateHttpClient())
                 {
                     var message = new HttpRequestMessage(HttpMethod.Get, uri) { Version = VersionFromUseHttp2 };
-                    message.Headers.TryAddWithoutValidation("x-Special_name", "header name with underscore");
+                    if (!message.Headers.TryAddWithoutValidation(key, value))
+                    {
+                        message.Content = new StringContent("");
+                        contentHeader = message.Content.Headers.TryAddWithoutValidation(key, value);
+                    }
                     (await client.SendAsync(message).ConfigureAwait(false)).Dispose();
                 }
+
+                // Validate our test by validating our understanding of a header's parsability.
+                HttpHeaders headers = contentHeader ? (HttpHeaders)
+                    new StringContent("").Headers :
+                    new HttpRequestMessage().Headers;
+                if (parsable)
+                {
+                    headers.Add(key, value);
+                }
+                else
+                {
+                    Assert.Throws<FormatException>(() => headers.Add(key, value));
+                }
             },
             async server =>
             {
                 HttpRequestData requestData = await server.HandleRequestAsync(HttpStatusCode.OK);
-
-                string header = requestData.GetSingleHeaderValue("x-Special_name");
-                Assert.Equal(header, headerValue);
+                Assert.Equal(value, requestData.GetSingleHeaderValue(key));
             });
         }
 
index cc3a1f2..3b06dd0 100644 (file)
@@ -49,16 +49,21 @@ namespace System.Net.Http.Unit.Tests.HPack
         {
             var buffer = new ArrayBuffer(4);
             FillAvailableSpaceWithOnes(buffer);
+            string[] headerValues = Array.Empty<string>();
 
-            foreach (KeyValuePair<HeaderDescriptor, string[]> header in headers.GetHeaderDescriptorsAndValues())
+            foreach (KeyValuePair<HeaderDescriptor, HttpHeaders.HeaderStoreItemInfo> header in headers.HeaderStore)
             {
+                int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref headerValues);
+                Assert.InRange(headerValuesCount, 0, int.MaxValue);
+                ReadOnlySpan<string> headerValuesSpan = headerValues.AsSpan(0, headerValuesCount);
+
                 KnownHeader knownHeader = header.Key.KnownHeader;
                 if (knownHeader != null)
                 {
                     // For all other known headers, send them via their pre-encoded name and the associated value.
                     WriteBytes(knownHeader.Http2EncodedName);
                     string separator = null;
-                    if (header.Value.Length > 1)
+                    if (headerValuesSpan.Length > 1)
                     {
                         HttpHeaderParser parser = header.Key.Parser;
                         if (parser != null && parser.SupportsMultipleValues)
@@ -71,12 +76,12 @@ namespace System.Net.Http.Unit.Tests.HPack
                         }
                     }
 
-                    WriteLiteralHeaderValues(header.Value, separator);
+                    WriteLiteralHeaderValues(headerValuesSpan, separator);
                 }
                 else
                 {
                     // The header is not known: fall back to just encoding the header name and value(s).
-                    WriteLiteralHeader(header.Key.Name, header.Value);
+                    WriteLiteralHeader(header.Key.Name, headerValuesSpan);
                 }
             }
 
@@ -94,7 +99,7 @@ namespace System.Net.Http.Unit.Tests.HPack
                 buffer.Commit(bytes.Length);
             }
 
-            void WriteLiteralHeaderValues(string[] values, string separator)
+            void WriteLiteralHeaderValues(ReadOnlySpan<string> values, string separator)
             {
                 int bytesWritten;
                 while (!HPackEncoder.EncodeStringLiterals(values, separator, buffer.AvailableSpan, out bytesWritten))
@@ -106,7 +111,7 @@ namespace System.Net.Http.Unit.Tests.HPack
                 buffer.Commit(bytesWritten);
             }
 
-            void WriteLiteralHeader(string name, string[] values)
+            void WriteLiteralHeader(string name, ReadOnlySpan<string> values)
             {
                 int bytesWritten;
                 while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparator, buffer.AvailableSpan, out bytesWritten))