From c06810bf654e09f016fe34dd2b4ac31fb73370a1 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sat, 11 Apr 2020 14:21:37 -0400 Subject: [PATCH] Several allocation reductions in SocketsHttpHandler (#34724) * Remove state machine allocation for SendWithNtConnectionAuthAsync Just manually inline the tiny helper that's only used from one call site. * Remove unnecessary HeaderStoreItemInfo allocations For the common case of a raw string, let the dictionary store the raw string directly rather than always wrapping it in a HeaderStoreItemInfo. * Cache Date and Server response header values For Server, we expect it to be the same for every request on the same connection. For Date, if we're making lots of requests in a high-throughput fashion, we expect many responses on the same connection to have the same value. In both cases, we compare the received value against the last one received, and if it's the same, reuse the same string. --- .../src/System/Net/Http/ByteArrayHelpers.cs | 34 +- .../System/Net/Http/Headers/HttpHeaders.cs | 428 ++++++++++-------- .../Net/Http/Headers/HttpRequestHeaders.cs | 5 + .../src/System/Net/Http/HttpClient.cs | 13 +- .../SocketsHttpHandler/Http2Connection.cs | 2 +- .../Http/SocketsHttpHandler/Http2Stream.cs | 5 +- .../SocketsHttpHandler/Http3RequestStream.cs | 4 +- .../Http/SocketsHttpHandler/HttpConnection.cs | 14 +- .../SocketsHttpHandler/HttpConnectionBase.cs | 26 ++ .../SocketsHttpHandler/HttpConnectionPool.cs | 12 +- .../UnitTests/HPack/HPackRoundtripTests.cs | 2 +- 11 files changed, 321 insertions(+), 224 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/ByteArrayHelpers.cs b/src/libraries/System.Net.Http/src/System/Net/Http/ByteArrayHelpers.cs index 5ee60dab6a5f5..6396c8dfec870 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/ByteArrayHelpers.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/ByteArrayHelpers.cs @@ -8,6 +8,9 @@ namespace System { internal static class ByteArrayHelpers { + // TODO: https://github.com/dotnet/runtime/issues/28230 + // Use Ascii.Equals* when it's available. + internal static bool EqualsOrdinalAsciiIgnoreCase(string left, ReadOnlySpan right) { Debug.Assert(left != null, "Expected non-null string"); @@ -22,16 +25,33 @@ internal static bool EqualsOrdinalAsciiIgnoreCase(string left, ReadOnlySpan right) + { + Debug.Assert(left != null, "Expected non-null string"); + + if (left.Length != right.Length) + { + return false; + } + + for (int i = 0; i < left.Length; i++) + { + if (left[i] != right[i]) { return false; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs index dfe61faa64eca..7ba6b35324084 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs @@ -10,30 +10,47 @@ namespace System.Net.Http.Headers { - // This type is used to store a collection of headers in 'headerStore': - // - A header can have multiple values. - // - A header can have an associated parser which is able to parse the raw string value into a strongly typed object. - // - If a header has an associated parser and the provided raw value can't be parsed, the value is considered - // invalid. Invalid values are stored if added using TryAddWithoutValidation(). If the value was added using Add(), - // Add() will throw FormatException. - // - Since parsing header values is expensive and users usually only care about a few headers, header values are - // lazily initialized. - // - // Given the properties above, a header value can have three states: - // - 'raw': The header value was added using TryAddWithoutValidation() and it wasn't parsed yet. - // - 'parsed': The header value was successfully parsed. It was either added using Add() where the value was parsed - // immediately, or if added using TryAddWithoutValidation() a user already accessed a property/method triggering the - // value to be parsed. - // - 'invalid': The header value was parsed, but parsing failed because the value is invalid. Storing invalid values - // allows users to still retrieve the value (by calling GetValues()), but it will not be exposed as strongly typed - // object. E.g. the client receives a response with the following header: 'Via: 1.1 proxy, invalid' - // - HttpHeaders.GetValues() will return "1.1 proxy", "invalid" - // - HttpResponseHeaders.Via collection will only contain one ViaHeaderValue object with value "1.1 proxy" public abstract class HttpHeaders : IEnumerable>> { - private Dictionary? _headerStore; + // This type is used to store a collection of headers in 'headerStore': + // - A header can have multiple values. + // - A header can have an associated parser which is able to parse the raw string value into a strongly typed object. + // - If a header has an associated parser and the provided raw value can't be parsed, the value is considered + // invalid. Invalid values are stored if added using TryAddWithoutValidation(). If the value was added using Add(), + // Add() will throw FormatException. + // - Since parsing header values is expensive and users usually only care about a few headers, header values are + // lazily initialized. + // + // Given the properties above, a header value can have three states: + // - 'raw': The header value was added using TryAddWithoutValidation() and it wasn't parsed yet. + // - 'parsed': The header value was successfully parsed. It was either added using Add() where the value was parsed + // immediately, or if added using TryAddWithoutValidation() a user already accessed a property/method triggering the + // value to be parsed. + // - 'invalid': The header value was parsed, but parsing failed because the value is invalid. Storing invalid values + // allows users to still retrieve the value (by calling GetValues()), but it will not be exposed as strongly typed + // object. E.g. the client receives a response with the following header: 'Via: 1.1 proxy, invalid' + // - HttpHeaders.GetValues() will return "1.1 proxy", "invalid" + // - HttpResponseHeaders.Via collection will only contain one ViaHeaderValue object with value "1.1 proxy" + + /// Key/value pairs of headers. The value is either a raw or a . + private Dictionary? _headerStore; + private readonly HttpHeaderType _allowedHeaderTypes; private readonly HttpHeaderType _treatAsCustomHeaderTypes; + /// Whether to force values in to be wrapped in objects. + /// + /// In general, header and collection types in System.Net.Http are not thread-safe: it's an error to read/write the same header + /// collection on multiple threads, and even to enumerate a single header collection from multiple threads concurrently; doing + /// so may lazily-initialize various properties and structures. However, there is one collection exempt from this based purely + /// on necessity: HttpClient.DefaultRequestHeaders. DefaultRequestHeaders is enumerated to add all of its headers into each + /// request, and since requests may be sent on the same HttpClient instance concurrently, this collection may be enumerated + /// concurrently. As such, we need to ensure that any mutation performed on DefaultRequestHeaders while enumerating is done in + /// a thread-safe way. This is achieved by locking on the objects in the , + /// but that means the value must be a rather than a raw string, which we prefer to store for + /// unvalidated additions. To work around that, for the HttpClient.DefaultRequestHeaders collection sets + /// to true, which will cause additions to always be wrapped, even if we otherwise wouldn't need them to be. + /// + private bool _forceHeaderStoreItems; private enum StoreLocation { @@ -47,31 +64,26 @@ protected HttpHeaders() { } - internal HttpHeaders(HttpHeaderType allowedHeaderTypes, HttpHeaderType treatAsCustomHeaderTypes) + internal HttpHeaders(HttpHeaderType allowedHeaderTypes, HttpHeaderType treatAsCustomHeaderTypes, bool forceHeaderStoreItems = false) { // Should be no overlap Debug.Assert((allowedHeaderTypes & treatAsCustomHeaderTypes) == 0); _allowedHeaderTypes = allowedHeaderTypes & ~HttpHeaderType.NonTrailing; _treatAsCustomHeaderTypes = treatAsCustomHeaderTypes & ~HttpHeaderType.NonTrailing; + _forceHeaderStoreItems = forceHeaderStoreItems; } - internal Dictionary? HeaderStore => _headerStore; + internal Dictionary? HeaderStore => _headerStore; - public void Add(string name, string? value) - { - Add(GetHeaderDescriptor(name), value); - } + public void Add(string name, string? value) => Add(GetHeaderDescriptor(name), value); internal void Add(HeaderDescriptor descriptor, string? value) { // We don't use GetOrCreateHeaderInfo() here, since this would create a new header in the store. If parsing // the value then throws, we would have to remove the header from the store again. So just get a // HeaderStoreItemInfo object and try to parse the value. If it works, we'll add the header. - HeaderStoreItemInfo info; - bool addToStore; - PrepareHeaderInfoForAdd(descriptor, out info, out addToStore); - + PrepareHeaderInfoForAdd(descriptor, out HeaderStoreItemInfo info, out bool addToStore); ParseAndAddValue(descriptor, info, value); // If we get here, then the value could be parsed correctly. If we created a new HeaderStoreItemInfo, add @@ -82,10 +94,7 @@ internal void Add(HeaderDescriptor descriptor, string? value) } } - public void Add(string name, IEnumerable values) - { - Add(GetHeaderDescriptor(name), values); - } + public void Add(string name, IEnumerable values) => Add(GetHeaderDescriptor(name), values); internal void Add(HeaderDescriptor descriptor, IEnumerable values) { @@ -125,16 +134,35 @@ public bool TryAddWithoutValidation(string name, string? value) => internal bool TryAddWithoutValidation(HeaderDescriptor descriptor, string? value) { - if (value == null) + // Normalize null values to be empty values, which are allowed. If the user adds multiple + // null/empty values, all of them are added to the collection. This will result in delimiter-only + // values, e.g. adding two null-strings (or empty, or whitespace-only) results in "My-Header: ,". + value ??= string.Empty; + + // Ensure the header store dictionary has been created. + _headerStore ??= new Dictionary(); + + if (_headerStore.TryGetValue(descriptor, out object? currentValue)) { - // We allow empty header values. (e.g. "My-Header: "). If the user adds multiple null/empty - // values, we'll just add them to the collection. This will result in delimiter-only values: - // E.g. adding two null-strings (or empty, or whitespace-only) results in "My-Header: ,". - value = string.Empty; + if (currentValue is HeaderStoreItemInfo info) + { + // The header store already contained a HeaderStoreItemInfo, so add to it. + AddValue(info, value, StoreLocation.Raw); + } + else + { + // The header store contained a single raw string value, so promote it + // to being a HeaderStoreItemInfo and add to it. + Debug.Assert(currentValue is string); + _headerStore[descriptor] = info = new HeaderStoreItemInfo() { RawValue = currentValue }; + AddValue(info, value, StoreLocation.Raw); + } + } + else + { + // The header store did not contain the header. Add the raw string. + _headerStore.Add(descriptor, _forceHeaderStoreItems ? new HeaderStoreItemInfo { RawValue = currentValue } : (object)value); } - - HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor, false); - AddValue(info, value, StoreLocation.Raw); return true; } @@ -150,67 +178,54 @@ internal bool TryAddWithoutValidation(HeaderDescriptor descriptor, IEnumerable enumerator = values.GetEnumerator()) { - // We allow empty header values. (e.g. "My-Header: "). If the user adds multiple null/empty - // values, we'll just add them to the collection. This will result in delimiter-only values: - // E.g. adding two null-strings (or empty, or whitespace-only) results in "My-Header: ,". - AddValue(info, value ?? string.Empty, StoreLocation.Raw); + if (enumerator.MoveNext()) + { + TryAddWithoutValidation(descriptor, enumerator.Current); + if (enumerator.MoveNext()) + { + HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor, parseRawValues: false); + do + { + AddValue(info, enumerator.Current ?? string.Empty, StoreLocation.Raw); + } + while (enumerator.MoveNext()); + } + } } return true; } - public void Clear() - { - if (_headerStore != null) - { - _headerStore.Clear(); - } - } - - public bool Remove(string name) - { - return Remove(GetHeaderDescriptor(name)); - } + public void Clear() => _headerStore?.Clear(); - public IEnumerable GetValues(string name) - { - return GetValues(GetHeaderDescriptor(name)); - } + public IEnumerable GetValues(string name) => GetValues(GetHeaderDescriptor(name)); internal IEnumerable GetValues(HeaderDescriptor descriptor) { - if (!TryGetValues(descriptor, out IEnumerable? values)) + if (TryGetValues(descriptor, out IEnumerable? values)) { - throw new InvalidOperationException(SR.net_http_headers_not_found); + return values; } - return values; + throw new InvalidOperationException(SR.net_http_headers_not_found); } public bool TryGetValues(string name, [NotNullWhen(true)] out IEnumerable? values) { - HeaderDescriptor descriptor; - if (!TryGetHeaderDescriptor(name, out descriptor)) + if (TryGetHeaderDescriptor(name, out HeaderDescriptor descriptor)) { - values = null; - return false; + return TryGetValues(descriptor, out values); } - return TryGetValues(descriptor, out values); + values = null; + return false; } internal bool TryGetValues(HeaderDescriptor descriptor, [NotNullWhen(true)] out IEnumerable? values) { - if (_headerStore == null) - { - values = null; - return false; - } - - if (TryGetAndParseHeaderInfo(descriptor, out HeaderStoreItemInfo? info)) + if (_headerStore != null && TryGetAndParseHeaderInfo(descriptor, out HeaderStoreItemInfo? info)) { values = GetValuesAsStrings(descriptor, info); return true; @@ -220,22 +235,14 @@ internal bool TryGetValues(HeaderDescriptor descriptor, [NotNullWhen(true)] out return false; } - public bool Contains(string name) - { - return Contains(GetHeaderDescriptor(name)); - } + public bool Contains(string name) => Contains(GetHeaderDescriptor(name)); internal bool Contains(HeaderDescriptor descriptor) { - if (_headerStore == null) - { - return false; - } - // We can't just call headerStore.ContainsKey() since after parsing the value the header may not exist // anymore (if the value contains invalid newline chars, we remove the header). So try to parse the // header value. - return TryGetAndParseHeaderInfo(descriptor, out HeaderStoreItemInfo? _); + return _headerStore != null && TryGetAndParseHeaderInfo(descriptor, out _); } public override string ToString() @@ -265,7 +272,7 @@ internal IEnumerable> GetHeaderStrings() yield break; } - foreach (KeyValuePair header in _headerStore) + foreach (KeyValuePair header in _headerStore) { string stringValue = GetHeaderString(header.Key, header.Value); @@ -273,73 +280,65 @@ internal IEnumerable> GetHeaderStrings() } } - internal string GetHeaderString(string name) - { - if (!TryGetHeaderDescriptor(name, out HeaderDescriptor descriptor)) - { - return string.Empty; - } + internal string GetHeaderString(HeaderDescriptor descriptor, object? exclude = null) => + TryGetHeaderValue(descriptor, out object? info) ? + GetHeaderString(descriptor, info, exclude) : + string.Empty; - return GetHeaderString(descriptor); - } - - internal string GetHeaderString(HeaderDescriptor descriptor, object? exclude = null) + private string GetHeaderString(HeaderDescriptor descriptor, object info, object? exclude = null) { - if (!TryGetHeaderInfo(descriptor, out HeaderStoreItemInfo? info)) - { - return string.Empty; - } - - return GetHeaderString(descriptor, info, exclude); - } - - private string GetHeaderString(HeaderDescriptor descriptor, HeaderStoreItemInfo info, object? exclude = null) - { - string stringValue; - string[] values = GetValuesAsStrings(descriptor, info, exclude); if (values.Length == 1) { - stringValue = values[0]; + return values[0]; } - else + + // Note that if we get multiple values for a header that doesn't support multiple values, we'll + // just separate the values using a comma (default separator). + string? separator = HttpHeaderParser.DefaultSeparator; + if ((descriptor.Parser != null) && (descriptor.Parser.SupportsMultipleValues)) { - // Note that if we get multiple values for a header that doesn't support multiple values, we'll - // just separate the values using a comma (default separator). - string? separator = HttpHeaderParser.DefaultSeparator; - if ((descriptor.Parser != null) && (descriptor.Parser.SupportsMultipleValues)) - { - separator = descriptor.Parser.Separator; - } - stringValue = string.Join(separator, values); + separator = descriptor.Parser.Separator; } - - return stringValue; + return string.Join(separator, values); } -#region IEnumerable>> Members + #region IEnumerable>> Members - public IEnumerator>> GetEnumerator() - { - return _headerStore != null && _headerStore.Count > 0 ? + public IEnumerator>> GetEnumerator() => _headerStore != null && _headerStore.Count > 0 ? GetEnumeratorCore() : ((IEnumerable>>)Array.Empty>>()).GetEnumerator(); - } private IEnumerator>> GetEnumeratorCore() { - foreach (KeyValuePair header in _headerStore!) + foreach (KeyValuePair header in _headerStore!) { HeaderDescriptor descriptor = header.Key; - HeaderStoreItemInfo info = header.Value; + object value = header.Value; + + HeaderStoreItemInfo? info = value as HeaderStoreItemInfo; + if (info is null) + { + // To retain consistent semantics, we need to upgrade a raw string to a HeaderStoreItemInfo + // during enumeration so that we can parse the raw value in order to a) return + // the correct set of parsed values, and b) update the instance for subsequent enumerations + // to reflect that parsing. It is safe to write back into the dictionary here because + // the only collection that can be enumerated concurrently is HttpClient.DefaultRequestHeaders, + // and all values in it will be HeaderStoreItemInfo. + Debug.Assert(!_forceHeaderStoreItems); + _headerStore[descriptor] = info = new HeaderStoreItemInfo() { RawValue = 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)) + if (!ParseRawHeaderValues(descriptor, info, removeEmptyHeader: false)) { // We have an invalid header value (contains invalid newline chars). Delete it. + // Note that ParseRawHeaderValues locks on the info object, such that only a single + // call to it with the same info will return false, which makes this removal safe to + // do even for HttpClient.DefaultRequestHeaders, which may be enumerated concurrently. _headerStore.Remove(descriptor); } else @@ -354,19 +353,16 @@ private IEnumerator>> GetEnumeratorCore #region IEnumerable Members - Collections.IEnumerator Collections.IEnumerable.GetEnumerator() - { - return GetEnumerator(); - } + Collections.IEnumerator Collections.IEnumerable.GetEnumerator() => GetEnumerator(); -#endregion + #endregion internal void AddParsedValue(HeaderDescriptor descriptor, object value) { Debug.Assert(value != null); Debug.Assert(descriptor.Parser != null, "Can't add parsed value if there is no parser available."); - HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor, true); + HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor, parseRawValues: true); // If the current header has only one value, we can't add another value. The strongly typed property // must not call AddParsedValue(), but SetParsedValue(). E.g. for headers like 'Date', 'Host'. @@ -381,8 +377,8 @@ internal void SetParsedValue(HeaderDescriptor descriptor, object value) Debug.Assert(descriptor.Parser != null, "Can't add parsed value if there is no parser available."); // This method will first clear all values. This is used e.g. when setting the 'Date' or 'Host' header. - // I.e. headers not supporting collections. - HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor, true); + // i.e. headers not supporting collections. + HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor, parseRawValues: true); info.InvalidValue = null; info.ParsedValue = null; @@ -403,15 +399,9 @@ internal void SetOrRemoveParsedValue(HeaderDescriptor descriptor, object? value) } } - internal bool Remove(HeaderDescriptor descriptor) - { - if (_headerStore == null) - { - return false; - } + public bool Remove(string name) => Remove(GetHeaderDescriptor(name)); - return _headerStore.Remove(descriptor); - } + internal bool Remove(HeaderDescriptor descriptor) => _headerStore != null && _headerStore.Remove(descriptor); internal bool RemoveParsedValue(HeaderDescriptor descriptor, object value) { @@ -430,14 +420,13 @@ internal bool RemoveParsedValue(HeaderDescriptor descriptor, object value) Debug.Assert(descriptor.Parser.SupportsMultipleValues, "This method should not be used for single-value headers. Use Remove(string) instead."); - bool result = false; - // If there is no entry, just return. if (info.ParsedValue == null) { return false; } + bool result = false; IEqualityComparer? comparer = descriptor.Parser.Comparer; List? parsedValues = info.ParsedValue as List; @@ -485,6 +474,7 @@ internal bool RemoveParsedValue(HeaderDescriptor descriptor, object value) return result; } + return false; } @@ -534,6 +524,7 @@ internal bool ContainsParsedValue(HeaderDescriptor descriptor, object value) return true; } } + return false; } } @@ -546,32 +537,41 @@ internal virtual void AddHeaders(HttpHeaders sourceHeaders) Debug.Assert(sourceHeaders != null); Debug.Assert(GetType() == sourceHeaders.GetType(), "Can only copy headers from an instance of the same type."); - if (sourceHeaders._headerStore == null) + Dictionary? sourceHeadersStore = sourceHeaders._headerStore; + if (sourceHeadersStore is null || sourceHeadersStore.Count == 0) { return; } - foreach (KeyValuePair header in sourceHeaders._headerStore) + _headerStore ??= new Dictionary(); + + foreach (KeyValuePair header in sourceHeadersStore) { // 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 // header, then we don't try to merge the values. - if ((_headerStore == null) || (!_headerStore.ContainsKey(header.Key))) + if (!_headerStore.ContainsKey(header.Key)) { - HeaderStoreItemInfo sourceInfo = header.Value; - - // If DefaultRequestHeaders values are copied to multiple messages, it is useful to parse these - // default header values only once. This is what we're doing here: By parsing raw headers in - // 'sourceHeaders' before copying values to our header store. - if (!sourceHeaders.ParseRawHeaderValues(header.Key, sourceInfo, false)) + object sourceValue = header.Value; + if (sourceValue is HeaderStoreItemInfo info) { - // If after trying to parse source header values no value is left (i.e. all values contain - // invalid newline chars), delete it and skip to the next header. - sourceHeaders._headerStore.Remove(header.Key); + if (!sourceHeaders.ParseRawHeaderValues(header.Key, info, removeEmptyHeader: false)) + { + // If after trying to parse source header values no value is left (i.e. all values contain + // invalid newline chars), delete it and skip to the next header. ParseRawHeaderValues takes + // a lock, and it'll only ever return false once for one thread, so we don't need to be + // concerned about concurrent removals on the HttpClient.DefaultRequestHeaders source. + sourceHeadersStore.Remove(header.Key); + } + else + { + AddHeaderInfo(header.Key, info); + } } else { - AddHeaderInfo(header.Key, sourceInfo); + Debug.Assert(sourceValue is string); + _headerStore.Add(header.Key, _forceHeaderStoreItems ? new HeaderStoreItemInfo { RawValue = sourceValue } : sourceValue); } } } @@ -655,15 +655,27 @@ private static void CloneAndAddValue(HeaderStoreItemInfo destinationInfo, object private HeaderStoreItemInfo GetOrCreateHeaderInfo(HeaderDescriptor descriptor, bool parseRawValues) { - HeaderStoreItemInfo? result; - bool found = false; + HeaderStoreItemInfo? result = null; + bool found; if (parseRawValues) { found = TryGetAndParseHeaderInfo(descriptor, out result); } else { - found = TryGetHeaderInfo(descriptor, out result); + found = TryGetHeaderValue(descriptor, out object? value); + if (found) + { + if (value is HeaderStoreItemInfo hsti) + { + result = hsti; + } + else + { + Debug.Assert(value is string); + _headerStore![descriptor] = result = new HeaderStoreItemInfo { RawValue = value }; + } + } } if (!found) @@ -688,33 +700,41 @@ private HeaderStoreItemInfo CreateAndAddHeaderToStore(HeaderDescriptor descripto return result; } - private void AddHeaderToStore(HeaderDescriptor descriptor, HeaderStoreItemInfo info) + private void AddHeaderToStore(HeaderDescriptor descriptor, object value) { - if (_headerStore == null) - { - _headerStore = new Dictionary(); - } - _headerStore.Add(descriptor, info); + Debug.Assert(value is string || value is HeaderStoreItemInfo); + (_headerStore ??= new Dictionary()).Add(descriptor, value); } - private bool TryGetHeaderInfo(HeaderDescriptor descriptor, [NotNullWhen(true)] out HeaderStoreItemInfo? info) + private bool TryGetHeaderValue(HeaderDescriptor descriptor, [NotNullWhen(true)] out object? value) { if (_headerStore == null) { - info = null; + value = null; return false; } - return _headerStore.TryGetValue(descriptor, out info); + return _headerStore.TryGetValue(descriptor, out value); } private bool TryGetAndParseHeaderInfo(HeaderDescriptor key, [NotNullWhen(true)] out HeaderStoreItemInfo? info) { - if (TryGetHeaderInfo(key, out info)) + if (TryGetHeaderValue(key, out object? value)) { - return ParseRawHeaderValues(key, info, true); + if (value is HeaderStoreItemInfo hsi) + { + info = hsi; + } + else + { + Debug.Assert(value is string); + _headerStore![key] = info = new HeaderStoreItemInfo() { RawValue = value }; + } + + return ParseRawHeaderValues(key, info, removeEmptyHeader: true); } + info = null; return false; } @@ -1029,7 +1049,7 @@ private void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreItemInfo i // a valid value. If it is (i.e. no exception thrown), we set the parsed value (if any) and return. if ((value == null) || (index == value.Length)) { - // If the returned value is null, then it means the header accepts empty values. I.e. we don't throw + // If the returned value is null, then it means the header accepts empty values. i.e. we don't throw // but we don't add 'null' to the store either. if (parsedValue != null) { @@ -1091,23 +1111,22 @@ private bool TryGetHeaderDescriptor(string name, out HeaderDescriptor descriptor { if (string.IsNullOrEmpty(name)) { - descriptor = default(HeaderDescriptor); + descriptor = default; return false; } - if (!HeaderDescriptor.TryGet(name, out descriptor)) + if (HeaderDescriptor.TryGet(name, out descriptor)) { - return false; - } + if ((descriptor.HeaderType & _allowedHeaderTypes) != 0) + { + return true; + } - if ((descriptor.HeaderType & _allowedHeaderTypes) != 0) - { - return true; - } - else if ((descriptor.HeaderType & _treatAsCustomHeaderTypes) != 0) - { - descriptor = descriptor.AsCustomHeader(); - return true; + if ((descriptor.HeaderType & _treatAsCustomHeaderTypes) != 0) + { + descriptor = descriptor.AsCustomHeader(); + return true; + } } return false; @@ -1136,8 +1155,15 @@ private static bool ContainsInvalidNewLine(string value, string name) return false; } - private static string[] GetValuesAsStrings(HeaderDescriptor descriptor, HeaderStoreItemInfo info, object? exclude = null) + private static string[] GetValuesAsStrings(HeaderDescriptor descriptor, object value, object? exclude = null) { + HeaderStoreItemInfo? info = value as HeaderStoreItemInfo; + if (info is null) + { + Debug.Assert(value is string); + return new string[1] { (string)value }; + } + int length = GetValueCount(info); string[] values; @@ -1168,8 +1194,22 @@ private static string[] GetValuesAsStrings(HeaderDescriptor descriptor, HeaderSt return values; } - internal static int GetValuesAsStrings(HeaderDescriptor descriptor, HeaderStoreItemInfo info, ref string[] values) + internal static int GetValuesAsStrings(HeaderDescriptor descriptor, object sourceValues, ref string[] values) { + HeaderStoreItemInfo? info = sourceValues as HeaderStoreItemInfo; + if (info is null) + { + Debug.Assert(sourceValues is string); + + if (values.Length == 0) + { + values = new string[1]; + } + + values[0] = (string)sourceValues; + return 1; + } + Debug.Assert(values != null); int length = GetValueCount(info); @@ -1267,7 +1307,7 @@ private bool AreEqual(object value, object? storeValue, IEqualityComparer? compa return value.Equals(storeValue); } -#region Private Classes + #region Private Classes internal class HeaderStoreItemInfo { @@ -1295,6 +1335,6 @@ internal bool CanAddValue(HttpHeaderParser parser) internal bool IsEmpty => (RawValue == null) && (InvalidValue == null) && (ParsedValue == null); } -#endregion + #endregion } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs index f62345ab9469c..452dda259a6e0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs @@ -266,6 +266,11 @@ internal HttpRequestHeaders() { } + internal HttpRequestHeaders(bool forceHeaderStoreItems) + : base(HttpHeaderType.General | HttpHeaderType.Request | HttpHeaderType.Custom, HttpHeaderType.Response, forceHeaderStoreItems) + { + } + internal override void AddHeaders(HttpHeaders sourceHeaders) { base.AddHeaders(sourceHeaders); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs index 91d955b3fc96b..c48875d356729 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs @@ -45,17 +45,8 @@ public static IWebProxy DefaultProxy } } - public HttpRequestHeaders DefaultRequestHeaders - { - get - { - if (_defaultRequestHeaders == null) - { - _defaultRequestHeaders = new HttpRequestHeaders(); - } - return _defaultRequestHeaders; - } - } + public HttpRequestHeaders DefaultRequestHeaders => + _defaultRequestHeaders ??= new HttpRequestHeaders(forceHeaderStoreItems: true); public Version DefaultRequestVersion { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index b2fbfd3969102..9246892510802 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1027,7 +1027,7 @@ private void WriteHeaderCollection(HttpHeaders headers) return; } - foreach (KeyValuePair header in headers.HeaderStore) + foreach (KeyValuePair header in headers.HeaderStore) { int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref _headerValues); Debug.Assert(headerValuesCount > 0, "No values for header??"); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index 4ca37b2b702c3..5e4a3839de6d4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -532,23 +532,24 @@ public void OnHeader(ReadOnlySpan name, ReadOnlySpan value) throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_header_name, Encoding.ASCII.GetString(name))); } - string headerValue = descriptor.GetHeaderValue(value); - // Note we ignore the return value from TryAddWithoutValidation; // if the header can't be added, we silently drop it. if (_responseProtocolState == ResponseProtocolState.ExpectingTrailingHeaders) { Debug.Assert(_trailers != null); + string headerValue = descriptor.GetHeaderValue(value); _trailers.Add(KeyValuePair.Create((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue)); } else if ((descriptor.HeaderType & HttpHeaderType.Content) == HttpHeaderType.Content) { Debug.Assert(_response != null && _response.Content != null); + string headerValue = descriptor.GetHeaderValue(value); _response.Content.Headers.TryAddWithoutValidation(descriptor, headerValue); } else { Debug.Assert(_response != null); + string headerValue = _connection.GetResponseHeaderValueWithCaching(descriptor, value); _response.Headers.TryAddWithoutValidation((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index d1be81f307901..394de483e20b1 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -593,7 +593,7 @@ private void BufferHeaderCollection(HttpHeaders headers) return; } - foreach (KeyValuePair header in headers.HeaderStore) + foreach (KeyValuePair header in headers.HeaderStore) { int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref _headerValues); Debug.Assert(headerValuesCount > 0, "No values for header??"); @@ -920,7 +920,7 @@ private void OnHeader(int? staticIndex, HeaderDescriptor descriptor, string? sta } else { - string headerValue = staticValue ?? descriptor.GetHeaderValue(literalValue); + string headerValue = staticValue ?? _connection.GetResponseHeaderValueWithCaching(descriptor, literalValue); switch (_headerState) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 3814e644a5b4c..04b6f4eb4f3d7 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -198,7 +198,7 @@ private async ValueTask WriteHeadersAsync(HttpHeaders headers, string? cookiesFr { if (headers.HeaderStore != null) { - foreach (KeyValuePair header in headers.HeaderStore) + foreach (KeyValuePair header in headers.HeaderStore) { if (header.Key.KnownHeader != null) { @@ -934,21 +934,25 @@ private static void ParseHeaderNameValue(HttpConnection connection, ReadOnlySpan pos++; } - string headerValue = descriptor.GetHeaderValue(line.Slice(pos)); - // Note we ignore the return value from TryAddWithoutValidation. If the header can't be added, we silently drop it. - // Request headers returned on the response must be treated as custom headers. + ReadOnlySpan value = line.Slice(pos); if (isFromTrailer) { + string headerValue = descriptor.GetHeaderValue(value); response.TrailingHeaders.TryAddWithoutValidation((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue); } else if ((descriptor.HeaderType & HttpHeaderType.Content) == HttpHeaderType.Content) { + string headerValue = descriptor.GetHeaderValue(value); response.Content!.Headers.TryAddWithoutValidation(descriptor, headerValue); } else { - response.Headers.TryAddWithoutValidation((descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, headerValue); + // Request headers returned on the response must be treated as custom headers. + string headerValue = connection.GetResponseHeaderValueWithCaching(descriptor, value); + response.Headers.TryAddWithoutValidation( + (descriptor.HeaderType & HttpHeaderType.Request) == HttpHeaderType.Request ? descriptor.AsCustomHeader() : descriptor, + headerValue); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs index 258be9180454f..7104057aa1349 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs @@ -2,7 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Net.Http.Headers; using System.Net.Security; using System.Runtime.CompilerServices; using System.Threading; @@ -12,6 +14,30 @@ namespace System.Net.Http { internal abstract class HttpConnectionBase : IHttpTrace { + /// Cached string for the last Date header received on this connection. + private string? _lastDateHeaderValue; + /// Cached string for the last Server header received on this connection. + private string? _lastServerHeaderValue; + + /// Uses , but first special-cases several known headers for which we can use caching. + public string GetResponseHeaderValueWithCaching(HeaderDescriptor descriptor, ReadOnlySpan value) + { + return + ReferenceEquals(descriptor.KnownHeader, KnownHeaders.Date) ? GetOrAddCachedValue(ref _lastDateHeaderValue, descriptor, value) : + ReferenceEquals(descriptor.KnownHeader, KnownHeaders.Server) ? GetOrAddCachedValue(ref _lastServerHeaderValue, descriptor, value) : + descriptor.GetHeaderValue(value); + + static string GetOrAddCachedValue([NotNull] ref string? cache, HeaderDescriptor descriptor, ReadOnlySpan value) + { + string? lastValue = cache; + if (lastValue is null || !ByteArrayHelpers.EqualsOrdinalAscii(lastValue, value)) + { + cache = lastValue = descriptor.GetHeaderValue(value); + } + return lastValue; + } + } + public abstract Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken); public abstract void Trace(string message, [CallerMemberName] string? memberName = null); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs index 61f59a365025b..dfe0252881b13 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs @@ -732,7 +732,17 @@ public async Task SendWithRetryAsync(HttpRequestMessage req { if (connection is HttpConnection) { - response = await SendWithNtConnectionAuthAsync((HttpConnection)connection, request, doRequestAuth, cancellationToken).ConfigureAwait(false); + ((HttpConnection)connection).Acquire(); + try + { + response = await (doRequestAuth && Settings._credentials != null ? + AuthenticationHelper.SendWithNtConnectionAuthAsync(request, Settings._credentials, (HttpConnection)connection, this, cancellationToken) : + SendWithNtProxyAuthAsync((HttpConnection)connection, request, cancellationToken)).ConfigureAwait(false); + } + finally + { + ((HttpConnection)connection).Release(); + } } else { diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs b/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs index 9459fe57c8908..241d79d5edd8d 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs @@ -51,7 +51,7 @@ private static Memory HPackEncode(HttpHeaders headers) FillAvailableSpaceWithOnes(buffer); string[] headerValues = Array.Empty(); - foreach (KeyValuePair header in headers.HeaderStore) + foreach (KeyValuePair header in headers.HeaderStore) { int headerValuesCount = HttpHeaders.GetValuesAsStrings(header.Key, header.Value, ref headerValues); Assert.InRange(headerValuesCount, 0, int.MaxValue);