From 175ad5f46554ac480ca93abcce175f695314ac98 Mon Sep 17 00:00:00 2001 From: Geoff Kizer Date: Wed, 9 Jun 2021 15:53:03 -0700 Subject: [PATCH] HttpHeaders: remove StoreLocation and AddValue (#53918) * remove StoreLocation and AddValue * make info.ParsedValue etc fields Co-authored-by: Geoffrey Kizer --- .../src/System/Net/Http/Headers/HttpHeaders.cs | 110 ++++++++------------- 1 file changed, 40 insertions(+), 70 deletions(-) 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 6633329..4c67e98 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 @@ -52,13 +52,6 @@ namespace System.Net.Http.Headers /// private bool _forceHeaderStoreItems; - private enum StoreLocation - { - Raw, - Invalid, - Parsed - } - protected HttpHeaders() : this(HttpHeaderType.All, HttpHeaderType.None) { @@ -150,7 +143,7 @@ namespace System.Net.Http.Headers if (currentValue is HeaderStoreItemInfo info) { // The header store already contained a HeaderStoreItemInfo, so add to it. - AddValue(info, value, StoreLocation.Raw); + AddRawValue(info, value); } else { @@ -158,7 +151,7 @@ namespace System.Net.Http.Headers // 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); + AddRawValue(info, value); } } else @@ -191,7 +184,7 @@ namespace System.Net.Http.Headers HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor, parseRawValues: false); do { - AddValue(info, enumerator.Current ?? string.Empty, StoreLocation.Raw); + AddRawValue(info, enumerator.Current ?? string.Empty); } while (enumerator.MoveNext()); } @@ -376,7 +369,7 @@ namespace System.Net.Http.Headers // must not call AddParsedValue(), but SetParsedValue(). E.g. for headers like 'Date', 'Host'. Debug.Assert(descriptor.Parser.SupportsMultipleValues, $"Header '{descriptor.Name}' doesn't support multiple values"); - AddValue(info, value, StoreLocation.Parsed); + AddParsedValue(info, value); } internal void SetParsedValue(HeaderDescriptor descriptor, object value) @@ -392,7 +385,7 @@ namespace System.Net.Http.Headers info.ParsedValue = null; info.RawValue = null; - AddValue(info, value, StoreLocation.Parsed); + AddParsedValue(info, value); } internal void SetOrRemoveParsedValue(HeaderDescriptor descriptor, object? value) @@ -620,12 +613,12 @@ namespace System.Net.Http.Headers // We only have one value. Clone it and assign it to the store. if (source is ICloneable cloneableValue) { - AddValue(destinationInfo, cloneableValue.Clone(), StoreLocation.Parsed); + AddParsedValue(destinationInfo, cloneableValue.Clone()); } else { // If it doesn't implement ICloneable, it's a value type or an immutable type like String/Uri. - AddValue(destinationInfo, source, StoreLocation.Parsed); + AddParsedValue(destinationInfo, source); } } @@ -789,7 +782,7 @@ namespace System.Net.Http.Headers { if (!ContainsInvalidNewLine(rawValue, descriptor.Name)) { - AddValue(info, rawValue, StoreLocation.Parsed); + AddParsedValue(info, rawValue); } } } @@ -814,7 +807,7 @@ namespace System.Net.Http.Headers { if (!ContainsInvalidNewLine(rawValue, descriptor.Name)) { - AddValue(info, rawValue, StoreLocation.Parsed); + AddParsedValue(info, rawValue); } } else @@ -858,11 +851,11 @@ namespace System.Net.Http.Headers // and the current header doesn't support multiple values: e.g. trying to add a date/time value // to the 'Date' header if we already have a date/time value will result in the second value being // added to the 'invalid' header values. - if (!info.CanAddValue(descriptor.Parser)) + if (!info.CanAddParsedValue(descriptor.Parser)) { if (addWhenInvalid) { - AddValue(info, value ?? string.Empty, StoreLocation.Invalid); + AddInvalidValue(info, value ?? string.Empty); } return false; } @@ -876,7 +869,7 @@ namespace System.Net.Http.Headers { if (parsedValue != null) { - AddValue(info, parsedValue, StoreLocation.Parsed); + AddParsedValue(info, parsedValue); } return true; } @@ -903,7 +896,7 @@ namespace System.Net.Http.Headers { if (!ContainsInvalidNewLine(value, descriptor.Name) && addWhenInvalid) { - AddValue(info, value, StoreLocation.Invalid); + AddInvalidValue(info, value); } return false; } @@ -912,7 +905,7 @@ namespace System.Net.Http.Headers // All values were parsed correctly. Copy results to the store. foreach (object item in parsedValues) { - AddValue(info, item, StoreLocation.Parsed); + AddParsedValue(info, item); } return true; } @@ -920,51 +913,31 @@ namespace System.Net.Http.Headers Debug.Assert(value != null); if (!ContainsInvalidNewLine(value, descriptor.Name) && addWhenInvalid) { - AddValue(info, value ?? string.Empty, StoreLocation.Invalid); + AddInvalidValue(info, value ?? string.Empty); } return false; } - private static void AddValue(HeaderStoreItemInfo info, object? value, StoreLocation location) + private static void AddParsedValue(HeaderStoreItemInfo info, object value) { - // Since we have the same pattern for all three store locations (raw, invalid, parsed), we use - // this helper method to deal with adding values: - // - if 'null' just set the store property to 'value' - // - if 'List' append 'value' to the end of the list - // - if 'T', i.e. we have already a value stored (but no list), create a list, add the stored value - // to the list and append 'value' at the end of the newly created list. - - object? currentStoreValue = null; - switch (location) - { - case StoreLocation.Raw: - currentStoreValue = info.RawValue; - AddValueToStoreValue(value, ref currentStoreValue); - info.RawValue = currentStoreValue; - break; - - case StoreLocation.Invalid: - currentStoreValue = info.InvalidValue; - AddValueToStoreValue(value, ref currentStoreValue); - info.InvalidValue = currentStoreValue; - break; - - case StoreLocation.Parsed: - Debug.Assert((value == null) || (!(value is List)), - "Header value types must not derive from List since this type is used internally to store " + - "lists of values. So we would not be able to distinguish between a single value and a list of values."); - currentStoreValue = info.ParsedValue; - AddValueToStoreValue(value, ref currentStoreValue); - info.ParsedValue = currentStoreValue; - break; - - default: - Debug.Fail("Unknown StoreLocation value: " + location.ToString()); - break; - } + Debug.Assert(!(value is List), + "Header value types must not derive from List since this type is used internally to store " + + "lists of values. So we would not be able to distinguish between a single value and a list of values."); + + AddValueToStoreValue(value, ref info.ParsedValue); } - private static void AddValueToStoreValue(object? value, ref object? currentStoreValue) where T : class + private static void AddInvalidValue(HeaderStoreItemInfo info, string value) + { + AddValueToStoreValue(value, ref info.InvalidValue); + } + + private static void AddRawValue(HeaderStoreItemInfo info, string value) + { + AddValueToStoreValue(value, ref info.RawValue); + } + + private static void AddValueToStoreValue(T value, ref object? currentStoreValue) where T : class { // If there is no value set yet, then add current item as value (we don't create a list // if not required). If 'info.Value' is already assigned then make sure 'info.Value' is a @@ -1028,13 +1001,13 @@ namespace System.Net.Http.Headers // If we don't have a parser for the header, we consider the value valid if it doesn't contains // invalid newline characters. We add the values as "parsed value". Note that we allow empty values. CheckInvalidNewLine(value); - AddValue(info, value ?? string.Empty, StoreLocation.Parsed); + AddParsedValue(info, value ?? string.Empty); return; } // If the header only supports 1 value, we can add the current value only if there is no // value already set. - if (!info.CanAddValue(descriptor.Parser)) + if (!info.CanAddParsedValue(descriptor.Parser)) { throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_single_value_header, descriptor.Name)); } @@ -1051,7 +1024,7 @@ namespace System.Net.Http.Headers // but we don't add 'null' to the store either. if (parsedValue != null) { - AddValue(info, parsedValue, StoreLocation.Parsed); + AddParsedValue(info, parsedValue); } return; } @@ -1077,7 +1050,7 @@ namespace System.Net.Http.Headers // All values were parsed correctly. Copy results to the store. foreach (object item in parsedValues) { - AddValue(info, item, StoreLocation.Parsed); + AddParsedValue(info, item); } } @@ -1280,17 +1253,15 @@ namespace System.Net.Http.Headers return value.Equals(storeValue); } - #region Private Classes - internal sealed class HeaderStoreItemInfo { internal HeaderStoreItemInfo() { } - internal object? RawValue { get; set; } - internal object? InvalidValue { get; set; } - internal object? ParsedValue { get; set; } + internal object? RawValue; + internal object? InvalidValue; + internal object? ParsedValue; - internal bool CanAddValue(HttpHeaderParser parser) + internal bool CanAddParsedValue(HttpHeaderParser parser) { Debug.Assert(parser != null, "There should be no reason to call CanAddValue if there is no parser for the current header."); @@ -1308,6 +1279,5 @@ namespace System.Net.Http.Headers internal bool IsEmpty => (RawValue == null) && (InvalidValue == null) && (ParsedValue == null); } - #endregion } } -- 2.7.4