From c749ffc93c4b74abe40b8a58e4f27b35929e31bf Mon Sep 17 00:00:00 2001 From: feiyun0112 Date: Mon, 28 Mar 2022 16:51:11 +0800 Subject: [PATCH] Do not reorder HTTP header values (#65249) * Do not reorder HTTP header values * fix complie error * make the changes @ danmoseley recommended * make the changes @MihaZupan recommended * make the changes @MihaZupan recommended * make the changes @MihaZupan recommended * make the changes @MihaZupan recommended * make the changes @MihaZupan recommended * check array length * fix unit test * Update src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs Co-authored-by: Miha Zupan * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs Co-authored-by: Miha Zupan * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs Co-authored-by: Miha Zupan * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs Co-authored-by: Miha Zupan * make the changes @MihaZupan recommended * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderParser.cs Co-authored-by: Miha Zupan * chang unit test * GetParsedValueLength * fix build error * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderParser.cs Co-authored-by: Miha Zupan * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs Co-authored-by: Miha Zupan * unit test * make changes @geoffkizer recommended * CloneAndAddValue for InvalidValues * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs Co-authored-by: Miha Zupan * Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs Co-authored-by: Miha Zupan * Final nits Co-authored-by: Miha Zupan --- .../Http/Headers/CacheControlHeaderParser.cs | 28 ++- .../Net/Http/Headers/HeaderUtilities.cs | 4 +- .../Net/Http/Headers/HttpContentHeaders.cs | 12 +- .../Net/Http/Headers/HttpGeneralHeaders.cs | 2 +- .../Http/Headers/HttpHeaderValueCollection.cs | 54 +++- .../System/Net/Http/Headers/HttpHeaders.cs | 232 ++++++++++++------ .../Net/Http/Headers/HttpRequestHeaders.cs | 16 +- .../Net/Http/Headers/HttpResponseHeaders.cs | 6 +- .../Headers/HttpContentHeadersTest.cs | 22 +- .../Headers/HttpHeaderValueCollectionTest.cs | 10 +- .../UnitTests/Headers/HttpHeadersTest.cs | 77 +++--- .../Headers/HttpRequestHeadersTest.cs | 60 +++-- .../Headers/HttpResponseHeadersTest.cs | 14 +- 13 files changed, 346 insertions(+), 191 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderParser.cs index 90acab59864eda..69609c8697cde9 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderParser.cs @@ -1,6 +1,7 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Diagnostics; namespace System.Net.Http.Headers @@ -22,8 +23,29 @@ private CacheControlHeaderParser() protected override int GetParsedValueLength(string value, int startIndex, object? storeValue, out object? parsedValue) { - CacheControlHeaderValue? temp = storeValue as CacheControlHeaderValue; - Debug.Assert(storeValue == null || temp != null, "'storeValue' is not of type CacheControlHeaderValue"); + CacheControlHeaderValue? temp = null; + bool isInvalidValue = true; + if (storeValue is List list) + { + foreach (object item in list) + { + if (item is not HttpHeaders.InvalidValue) + { + isInvalidValue = false; + temp = item as CacheControlHeaderValue; + break; + } + } + } + else + { + if (storeValue is not HttpHeaders.InvalidValue) + { + isInvalidValue = false; + temp = storeValue as CacheControlHeaderValue; + } + } + Debug.Assert(isInvalidValue || storeValue == null || temp != null, "'storeValue' is not of type CacheControlHeaderValue"); int resultLength = CacheControlHeaderValue.GetCacheControlLength(value, startIndex, temp, out temp); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs index 6cc82433278f75..12ffbd45dee290 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs @@ -287,7 +287,7 @@ internal static int GetNextNonEmptyOrWhitespaceIndex(string input, int startInde { Debug.Assert(store != null); - object? storedValue = store.GetParsedValues(descriptor); + object? storedValue = store.GetSingleParsedValue(descriptor); if (storedValue != null) { return (DateTimeOffset)storedValue; @@ -304,7 +304,7 @@ internal static int GetNextNonEmptyOrWhitespaceIndex(string input, int startInde { Debug.Assert(store != null); - object? storedValue = store.GetParsedValues(descriptor); + object? storedValue = store.GetSingleParsedValue(descriptor); if (storedValue != null) { return (TimeSpan)storedValue; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpContentHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpContentHeaders.cs index 361e27bd8f0596..73f93e6bfccf4b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpContentHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpContentHeaders.cs @@ -19,7 +19,7 @@ public sealed class HttpContentHeaders : HttpHeaders public ContentDispositionHeaderValue? ContentDisposition { - get { return (ContentDispositionHeaderValue?)GetParsedValues(KnownHeaders.ContentDisposition.Descriptor); } + get { return (ContentDispositionHeaderValue?)GetSingleParsedValue(KnownHeaders.ContentDisposition.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.ContentDisposition.Descriptor, value); } } @@ -36,7 +36,7 @@ public long? ContentLength get { // 'Content-Length' can only hold one value. So either we get 'null' back or a boxed long value. - object? storedValue = GetParsedValues(KnownHeaders.ContentLength.Descriptor); + object? storedValue = GetSingleParsedValue(KnownHeaders.ContentLength.Descriptor); // Only try to calculate the length if the user didn't set the value explicitly using the setter. if (!_contentLengthSet && (storedValue == null)) @@ -72,25 +72,25 @@ public long? ContentLength public Uri? ContentLocation { - get { return (Uri?)GetParsedValues(KnownHeaders.ContentLocation.Descriptor); } + get { return (Uri?)GetSingleParsedValue(KnownHeaders.ContentLocation.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.ContentLocation.Descriptor, value); } } public byte[]? ContentMD5 { - get { return (byte[]?)GetParsedValues(KnownHeaders.ContentMD5.Descriptor); } + get { return (byte[]?)GetSingleParsedValue(KnownHeaders.ContentMD5.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.ContentMD5.Descriptor, value); } } public ContentRangeHeaderValue? ContentRange { - get { return (ContentRangeHeaderValue?)GetParsedValues(KnownHeaders.ContentRange.Descriptor); } + get { return (ContentRangeHeaderValue?)GetSingleParsedValue(KnownHeaders.ContentRange.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.ContentRange.Descriptor, value); } } public MediaTypeHeaderValue? ContentType { - get { return (MediaTypeHeaderValue?)GetParsedValues(KnownHeaders.ContentType.Descriptor); } + get { return (MediaTypeHeaderValue?)GetSingleParsedValue(KnownHeaders.ContentType.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.ContentType.Descriptor, value); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpGeneralHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpGeneralHeaders.cs index 4bd4694f4d5d3d..036834930d820c 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpGeneralHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpGeneralHeaders.cs @@ -22,7 +22,7 @@ internal sealed class HttpGeneralHeaders public CacheControlHeaderValue? CacheControl { - get { return (CacheControlHeaderValue?)_parent.GetParsedValues(KnownHeaders.CacheControl.Descriptor); } + get { return (CacheControlHeaderValue?)_parent.GetSingleParsedValue(KnownHeaders.CacheControl.Descriptor); } set { _parent.SetOrRemoveParsedValue(KnownHeaders.CacheControl.Descriptor, value); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderValueCollection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderValueCollection.cs index 7ae54578ed03db..4d673c25261c33 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderValueCollection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderValueCollection.cs @@ -86,7 +86,7 @@ public void CopyTo(T[] array!!, int arrayIndex) throw new ArgumentOutOfRangeException(nameof(arrayIndex)); } - object? storeValue = _store.GetParsedValues(_descriptor); + object? storeValue = _store.GetParsedAndInvalidValues(_descriptor); if (storeValue == null) { @@ -97,18 +97,30 @@ public void CopyTo(T[] array!!, int arrayIndex) if (storeValues == null) { - // We only have 1 value: If it is the "special value" just return, otherwise add the value to the - // array and return. - Debug.Assert(storeValue is T); - if (arrayIndex == array.Length) + if (storeValue is not HttpHeaders.InvalidValue) { - throw new ArgumentException(SR.net_http_copyto_array_too_small); + Debug.Assert(storeValue is T); + if (arrayIndex == array.Length) + { + throw new ArgumentException(SR.net_http_copyto_array_too_small); + } + array[arrayIndex] = (T)storeValue; } - array[arrayIndex] = (T)storeValue; } else { - storeValues.CopyTo(array, arrayIndex); + foreach (object item in storeValues) + { + if (item is not HttpHeaders.InvalidValue) + { + Debug.Assert(item is T); + if (arrayIndex == array.Length) + { + throw new ArgumentException(SR.net_http_copyto_array_too_small); + } + array[arrayIndex++] = (T)item; + } + } } } @@ -122,8 +134,8 @@ public bool Remove(T item) public IEnumerator GetEnumerator() { - object? storeValue = _store.GetParsedValues(_descriptor); - return storeValue is null ? + object? storeValue = _store.GetParsedAndInvalidValues(_descriptor); + return storeValue is null || storeValue is HttpHeaders.InvalidValue ? ((IEnumerable)Array.Empty()).GetEnumerator() : // use singleton empty array enumerator Iterate(storeValue); @@ -134,6 +146,10 @@ static IEnumerator Iterate(object storeValue) // We have multiple values. Iterate through the values and return them. foreach (object item in storeValues) { + if (item is HttpHeaders.InvalidValue) + { + continue; + } Debug.Assert(item is T); yield return (T)item; } @@ -178,7 +194,7 @@ private int GetCount() { // This is an O(n) operation. - object? storeValue = _store.GetParsedValues(_descriptor); + object? storeValue = _store.GetParsedAndInvalidValues(_descriptor); if (storeValue == null) { @@ -189,11 +205,23 @@ private int GetCount() if (storeValues == null) { - return 1; + if (storeValue is not HttpHeaders.InvalidValue) + { + return 1; + } + return 0; } else { - return storeValues.Count; + int count = 0; + foreach (object item in storeValues) + { + if (item is not HttpHeaders.InvalidValue) + { + count++; + } + } + return count; } } } 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 fbeabc8fbe3fed..c54c97e9b30da5 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 @@ -85,8 +85,9 @@ internal void Add(HeaderDescriptor descriptor, string? value) // If we get here, then the value could be parsed correctly. If we created a new HeaderStoreItemInfo, add // it to the store if we added at least one value. - if (addToStore && (info.ParsedValue != null)) + if (addToStore && (info.ParsedAndInvalidValues != null)) { + info.AssertContainsNoInvalidValues(); Debug.Assert(!ContainsKey(descriptor)); AddEntryToStore(new HeaderEntry(descriptor, info)); } @@ -114,8 +115,9 @@ internal void Add(HeaderDescriptor descriptor, IEnumerable values!!) // get added. Same here: If multiple values get added to a _new_ header, make sure the header gets added // with the valid values. // However, if all values for a _new_ header were invalid, then don't add the header. - if (addToStore && (info.ParsedValue != null)) + if (addToStore && (info.ParsedAndInvalidValues != null)) { + info.AssertContainsNoInvalidValues(); Debug.Assert(!ContainsKey(descriptor)); AddEntryToStore(new HeaderEntry(descriptor, info)); } @@ -385,8 +387,7 @@ internal void SetParsedValue(HeaderDescriptor descriptor, object value) // i.e. headers not supporting collections. HeaderStoreItemInfo info = GetOrCreateHeaderInfo(descriptor); - info.InvalidValue = null; - info.ParsedValue = null; + info.ParsedAndInvalidValues = null; info.RawValue = null; AddParsedValue(info, value); @@ -419,7 +420,8 @@ internal bool RemoveParsedValue(HeaderDescriptor descriptor, object value) "This method should not be used for single-value headers. Use Remove(string) instead."); // If there is no entry, just return. - if (info.ParsedValue == null) + var parsedValue = info.ParsedAndInvalidValues; + if (parsedValue == null) { return false; } @@ -427,39 +429,46 @@ internal bool RemoveParsedValue(HeaderDescriptor descriptor, object value) bool result = false; IEqualityComparer? comparer = descriptor.Parser.Comparer; - List? parsedValues = info.ParsedValue as List; + List? parsedValues = parsedValue as List; if (parsedValues == null) { - Debug.Assert(info.ParsedValue.GetType() == value.GetType(), - "Stored value does not have the same type as 'value'."); - - if (AreEqual(value, info.ParsedValue, comparer)) + if (parsedValue is not InvalidValue) { - info.ParsedValue = null; - result = true; + Debug.Assert(parsedValue.GetType() == value.GetType(), + "Stored value does not have the same type as 'value'."); + + if (AreEqual(value, parsedValue, comparer)) + { + info.ParsedAndInvalidValues = null; + result = true; + } } } else { foreach (object item in parsedValues) { - Debug.Assert(item.GetType() == value.GetType(), - "One of the stored values does not have the same type as 'value'."); - - if (AreEqual(value, item, comparer)) + if (item is not InvalidValue) { - // Remove 'item' rather than 'value', since the 'comparer' may consider two values - // equal even though the default obj.Equals() may not (e.g. if 'comparer' does - // case-insensitive comparison for strings, but string.Equals() is case-sensitive). - result = parsedValues.Remove(item); - break; + Debug.Assert(item.GetType() == value.GetType(), + "One of the stored values does not have the same type as 'value'."); + + if (AreEqual(value, item, comparer)) + { + // Remove 'item' rather than 'value', since the 'comparer' may consider two values + // equal even though the default obj.Equals() may not (e.g. if 'comparer' does + // case-insensitive comparison for strings, but string.Equals() is case-sensitive). + result = parsedValues.Remove(item); + break; + } } } // If we removed the last item in a list, remove the list. if (parsedValues.Count == 0) { - info.ParsedValue = null; + info.AssertContainsNoInvalidValues(); + info.ParsedAndInvalidValues = null; } } @@ -489,32 +498,39 @@ internal bool ContainsParsedValue(HeaderDescriptor descriptor, object value) "This method should not be used for single-value headers. Use equality comparer instead."); // If there is no entry, just return. - if (info.ParsedValue == null) + var parsedValue = info.ParsedAndInvalidValues; + if (parsedValue == null) { return false; } - List? parsedValues = info.ParsedValue as List; + List? parsedValues = parsedValue as List; IEqualityComparer? comparer = descriptor.Parser.Comparer; if (parsedValues == null) { - Debug.Assert(info.ParsedValue.GetType() == value.GetType(), - "Stored value does not have the same type as 'value'."); + if (parsedValue is not InvalidValue) + { + Debug.Assert(parsedValue.GetType() == value.GetType(), + "Stored value does not have the same type as 'value'."); - return AreEqual(value, info.ParsedValue, comparer); + return AreEqual(value, parsedValue, comparer); + } } else { foreach (object item in parsedValues) { - Debug.Assert(item.GetType() == value.GetType(), - "One of the stored values does not have the same type as 'value'."); - - if (AreEqual(value, item, comparer)) + if (item is not InvalidValue) { - return true; + Debug.Assert(item.GetType() == value.GetType(), + "One of the stored values does not have the same type as 'value'."); + + if (AreEqual(value, item, comparer)) + { + return true; + } } } @@ -585,26 +601,18 @@ private static HeaderStoreItemInfo CloneHeaderInfo(HeaderDescriptor descriptor, if (descriptor.Parser == null) { - // We have custom header values. The parsed values are strings. - // Custom header values are always stored as string or list of strings. - Debug.Assert(sourceInfo.InvalidValue == null, "No invalid values expected for custom headers."); - destinationInfo.ParsedValue = CloneStringHeaderInfoValues(sourceInfo.ParsedValue); + sourceInfo.AssertContainsNoInvalidValues(); + destinationInfo.ParsedAndInvalidValues = CloneStringHeaderInfoValues(sourceInfo.ParsedAndInvalidValues); } else { - // We have a parser, so we also have to copy invalid values and clone parsed values. - - // Invalid values are always strings. Strings are immutable. So we only have to clone the - // collection (if there is one). - destinationInfo.InvalidValue = CloneStringHeaderInfoValues(sourceInfo.InvalidValue); - - // Now clone and add parsed values (if any). - if (sourceInfo.ParsedValue != null) + // We have a parser, so we also have to clone invalid values and parsed values. + if (sourceInfo.ParsedAndInvalidValues != null) { - List? sourceValues = sourceInfo.ParsedValue as List; + List? sourceValues = sourceInfo.ParsedAndInvalidValues as List; if (sourceValues == null) { - CloneAndAddValue(destinationInfo, sourceInfo.ParsedValue); + CloneAndAddValue(destinationInfo, sourceInfo.ParsedAndInvalidValues); } else { @@ -624,6 +632,7 @@ private static void CloneAndAddValue(HeaderStoreItemInfo destinationInfo, object // We only have one value. Clone it and assign it to the store. if (source is ICloneable cloneableValue) { + Debug.Assert(source is not InvalidValue); AddParsedValue(destinationInfo, cloneableValue.Clone()); } else @@ -744,7 +753,7 @@ private bool ParseRawHeaderValues(HeaderDescriptor descriptor, HeaderStoreItemIn // During parsing, we removed the value since it contains newline chars. Return false to indicate that // this is an empty header. - if ((info.InvalidValue == null) && (info.ParsedValue == null)) + if (info.ParsedAndInvalidValues == null) { // After parsing the raw value, no value is left because all values contain newline chars. Debug.Assert(_count > 0); @@ -813,8 +822,9 @@ internal bool TryParseAndAddValue(HeaderDescriptor descriptor, string? value) bool result = TryParseAndAddRawHeaderValue(descriptor, info, value, false); - if (result && addToStore && (info.ParsedValue != null)) + if (result && addToStore && (info.ParsedAndInvalidValues != null)) { + info.AssertContainsNoInvalidValues(); // If we get here, then the value could be parsed correctly. If we created a new HeaderStoreItemInfo, add // it to the store if we added at least one value. Debug.Assert(!ContainsKey(descriptor)); @@ -845,7 +855,7 @@ private static bool TryParseAndAddRawHeaderValue(HeaderDescriptor descriptor, He int index = 0; - if (descriptor.Parser.TryParseValue(value, info.ParsedValue, ref index, out object? parsedValue)) + if (descriptor.Parser.TryParseValue(value, info.ParsedAndInvalidValues, ref index, out object? parsedValue)) { // The raw string only represented one value (which was successfully parsed). Add the value and return. if ((value == null) || (index == value.Length)) @@ -868,7 +878,7 @@ private static bool TryParseAndAddRawHeaderValue(HeaderDescriptor descriptor, He while (index < value.Length) { - if (descriptor.Parser.TryParseValue(value, info.ParsedValue, ref index, out parsedValue)) + if (descriptor.Parser.TryParseValue(value, info.ParsedAndInvalidValues, ref index, out parsedValue)) { if (parsedValue != null) { @@ -907,12 +917,12 @@ private static void AddParsedValue(HeaderStoreItemInfo info, object value) "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); + AddValueToStoreValue(value, ref info.ParsedAndInvalidValues); } private static void AddInvalidValue(HeaderStoreItemInfo info, string value) { - AddValueToStoreValue(value, ref info.InvalidValue); + AddValueToStoreValue(new InvalidValue(value), ref info.ParsedAndInvalidValues); } private static void AddRawValue(HeaderStoreItemInfo info, string value) @@ -945,17 +955,24 @@ private static void AddValueToStoreValue(T value, ref object? currentStoreVal } } - // Since most of the time we just have 1 value, we don't create a List for one value, but we change - // the return type to 'object'. The caller has to deal with the return type (object vs. List). This - // is to optimize the most common scenario where a header has only one value. - internal object? GetParsedValues(HeaderDescriptor descriptor) + internal object? GetSingleParsedValue(HeaderDescriptor descriptor) + { + if (!TryGetAndParseHeaderInfo(descriptor, out HeaderStoreItemInfo? info)) + { + return null; + } + + return info.GetSingleParsedValue(); + } + + internal object? GetParsedAndInvalidValues(HeaderDescriptor descriptor) { if (!TryGetAndParseHeaderInfo(descriptor, out HeaderStoreItemInfo? info)) { return null; } - return info.ParsedValue; + return info.ParsedAndInvalidValues; } internal virtual bool IsAllowedHeaderName(HeaderDescriptor descriptor) => true; @@ -996,7 +1013,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte } int index = 0; - object parsedValue = descriptor.Parser.ParseValue(value, info.ParsedValue, ref index); + object parsedValue = descriptor.Parser.ParseValue(value, info.ParsedAndInvalidValues, ref index); // The raw string only represented one value (which was successfully parsed). Add the value and return. // If value is null we still have to first call ParseValue() to allow the parser to decide whether null is @@ -1023,7 +1040,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte while (index < value.Length) { - parsedValue = descriptor.Parser.ParseValue(value, info.ParsedValue, ref index); + parsedValue = descriptor.Parser.ParseValue(value, info.ParsedAndInvalidValues, ref index); if (parsedValue != null) { parsedValues.Add(parsedValue); @@ -1146,8 +1163,8 @@ internal static void GetStoreValuesAsStringOrStringArray(HeaderDescriptor descri int currentIndex = 0; ReadStoreValues(values, info.RawValue, null, ref currentIndex); - ReadStoreValues(values, info.ParsedValue, descriptor.Parser, ref currentIndex); - ReadStoreValues(values, info.InvalidValue, null, ref currentIndex); + ReadStoreValues(values, info.ParsedAndInvalidValues, descriptor.Parser, ref currentIndex); + Debug.Assert(currentIndex == length); } @@ -1180,8 +1197,7 @@ internal static int GetStoreValuesIntoStringArray(HeaderDescriptor descriptor, o int currentIndex = 0; ReadStoreValues(values, info.RawValue, null, ref currentIndex); - ReadStoreValues(values, info.ParsedValue, descriptor.Parser, ref currentIndex); - ReadStoreValues(values, info.InvalidValue, null, ref currentIndex); + ReadStoreValues(values, info.ParsedAndInvalidValues, descriptor.Parser, ref currentIndex); Debug.Assert(currentIndex == length); } @@ -1193,8 +1209,7 @@ private static int GetValueCount(HeaderStoreItemInfo info) Debug.Assert(info != null); int valueCount = Count(info.RawValue); - valueCount += Count(info.InvalidValue); - valueCount += Count(info.ParsedValue); + valueCount += Count(info.ParsedAndInvalidValues); return valueCount; static int Count(object? valueStore) => @@ -1211,7 +1226,7 @@ private static void ReadStoreValues(Span values, object? storeValue, if (storeValues == null) { - values[currentIndex] = parser == null ? storeValue.ToString() : parser.ToString(storeValue); + values[currentIndex] = parser == null || storeValue is InvalidValue ? storeValue.ToString() : parser.ToString(storeValue); currentIndex++; } else @@ -1219,7 +1234,7 @@ private static void ReadStoreValues(Span values, object? storeValue, foreach (object? item in storeValues) { Debug.Assert(item != null); - values[currentIndex] = parser == null ? item.ToString() : parser.ToString(item); + values[currentIndex] = parser == null || item is InvalidValue ? item.ToString() : parser.ToString(item); currentIndex++; } } @@ -1239,15 +1254,27 @@ private static bool AreEqual(object value, object? storeValue, IEqualityComparer return value.Equals(storeValue); } + internal sealed class InvalidValue + { + private readonly string _value; + + public InvalidValue(string value) + { + Debug.Assert(value is not null); + _value = value; + } + + public override string ToString() => _value; + } + internal sealed class HeaderStoreItemInfo { internal HeaderStoreItemInfo() { } internal object? RawValue; - internal object? InvalidValue; - internal object? ParsedValue; + internal object? ParsedAndInvalidValues; - internal bool CanAddParsedValue(HttpHeaderParser parser) + public 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."); @@ -1260,10 +1287,71 @@ internal bool CanAddParsedValue(HttpHeaderParser parser) // 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)); + return parser.SupportsMultipleValues || ParsedAndInvalidValues is null; + } + + [Conditional("DEBUG")] + public void AssertContainsNoInvalidValues() + { + if (ParsedAndInvalidValues is not null) + { + if (ParsedAndInvalidValues is List list) + { + foreach (object item in list) + { + Debug.Assert(item is not InvalidValue); + } + } + else + { + Debug.Assert(ParsedAndInvalidValues is not InvalidValue); + } + } + } + + public object? GetSingleParsedValue() + { + if (ParsedAndInvalidValues is not null) + { + if (ParsedAndInvalidValues is List list) + { + AssertContainsSingleParsedValue(list); + foreach (object item in list) + { + if (item is not InvalidValue) + { + return item; + } + } + } + else + { + if (ParsedAndInvalidValues is not InvalidValue) + { + return ParsedAndInvalidValues; + } + } + } + + return null; + } + + [Conditional("DEBUG")] + private static void AssertContainsSingleParsedValue(List list) + { + int count = 0; + foreach (object item in list) + { + if (item is not InvalidValue) + { + count++; + } + } + + Debug.Assert(count == 1, "Only a single parsed value should be stored for this parser"); } - internal bool IsEmpty => (RawValue == null) && (InvalidValue == null) && (ParsedValue == null); + public bool IsEmpty => RawValue == null && ParsedAndInvalidValues == null; } 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 9a278ff1183196..5da7cfb9844eda 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 @@ -47,7 +47,7 @@ private T GetSpecializedCollection(int slot, Func crea public AuthenticationHeaderValue? Authorization { - get { return (AuthenticationHeaderValue?)GetParsedValues(KnownHeaders.Authorization.Descriptor); } + get { return (AuthenticationHeaderValue?)GetSingleParsedValue(KnownHeaders.Authorization.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.Authorization.Descriptor, value); } } @@ -87,7 +87,7 @@ public bool? ExpectContinue public string? From { - get { return (string?)GetParsedValues(KnownHeaders.From.Descriptor); } + get { return (string?)GetSingleParsedValue(KnownHeaders.From.Descriptor); } set { // Null and empty string are equivalent. In this case it means, remove the From header value (if any). @@ -104,7 +104,7 @@ public string? From public string? Host { - get { return (string?)GetParsedValues(KnownHeaders.Host.Descriptor); } + get { return (string?)GetSingleParsedValue(KnownHeaders.Host.Descriptor); } set { // Null and empty string are equivalent. In this case it means, remove the Host header value (if any). @@ -135,7 +135,7 @@ public DateTimeOffset? IfModifiedSince public RangeConditionHeaderValue? IfRange { - get { return (RangeConditionHeaderValue?)GetParsedValues(KnownHeaders.IfRange.Descriptor); } + get { return (RangeConditionHeaderValue?)GetSingleParsedValue(KnownHeaders.IfRange.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.IfRange.Descriptor, value); } } @@ -149,7 +149,7 @@ public int? MaxForwards { get { - object? storedValue = GetParsedValues(KnownHeaders.MaxForwards.Descriptor); + object? storedValue = GetSingleParsedValue(KnownHeaders.MaxForwards.Descriptor); if (storedValue != null) { return (int)storedValue; @@ -162,19 +162,19 @@ public int? MaxForwards public AuthenticationHeaderValue? ProxyAuthorization { - get { return (AuthenticationHeaderValue?)GetParsedValues(KnownHeaders.ProxyAuthorization.Descriptor); } + get { return (AuthenticationHeaderValue?)GetSingleParsedValue(KnownHeaders.ProxyAuthorization.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.ProxyAuthorization.Descriptor, value); } } public RangeHeaderValue? Range { - get { return (RangeHeaderValue?)GetParsedValues(KnownHeaders.Range.Descriptor); } + get { return (RangeHeaderValue?)GetSingleParsedValue(KnownHeaders.Range.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.Range.Descriptor, value); } } public Uri? Referrer { - get { return (Uri?)GetParsedValues(KnownHeaders.Referer.Descriptor); } + get { return (Uri?)GetSingleParsedValue(KnownHeaders.Referer.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.Referer.Descriptor, value); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpResponseHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpResponseHeaders.cs index 442db3beb2f49b..8795e39f9ea5b4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpResponseHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpResponseHeaders.cs @@ -45,13 +45,13 @@ public TimeSpan? Age public EntityTagHeaderValue? ETag { - get { return (EntityTagHeaderValue?)GetParsedValues(KnownHeaders.ETag.Descriptor); } + get { return (EntityTagHeaderValue?)GetSingleParsedValue(KnownHeaders.ETag.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.ETag.Descriptor, value); } } public Uri? Location { - get { return (Uri?)GetParsedValues(KnownHeaders.Location.Descriptor); } + get { return (Uri?)GetSingleParsedValue(KnownHeaders.Location.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.Location.Descriptor, value); } } @@ -60,7 +60,7 @@ public Uri? Location public RetryConditionHeaderValue? RetryAfter { - get { return (RetryConditionHeaderValue?)GetParsedValues(KnownHeaders.RetryAfter.Descriptor); } + get { return (RetryConditionHeaderValue?)GetSingleParsedValue(KnownHeaders.RetryAfter.Descriptor); } set { SetOrRemoveParsedValue(KnownHeaders.RetryAfter.Descriptor, value); } } diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpContentHeadersTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpContentHeadersTest.cs index b50b28b56fd1ba..752926313ebe16 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpContentHeadersTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpContentHeadersTest.cs @@ -34,7 +34,7 @@ public void ContentLength_ReadValue_TryComputeLengthInvoked() // The delegate is invoked to return the length. Assert.Equal(15, _headers.ContentLength); - Assert.Equal((long)15, _headers.GetParsedValues(KnownHeaders.ContentLength.Descriptor)); + Assert.Equal((long)15, _headers.GetSingleParsedValue(KnownHeaders.ContentLength.Descriptor)); // After getting the calculated content length, set it to null. _headers.ContentLength = null; @@ -43,7 +43,7 @@ public void ContentLength_ReadValue_TryComputeLengthInvoked() _headers.ContentLength = 27; Assert.Equal((long)27, _headers.ContentLength); - Assert.Equal((long)27, _headers.GetParsedValues(KnownHeaders.ContentLength.Descriptor)); + Assert.Equal((long)27, _headers.GetSingleParsedValue(KnownHeaders.ContentLength.Descriptor)); } [Fact] @@ -53,7 +53,7 @@ public void ContentLength_SetCustomValue_TryComputeLengthNotInvoked() _headers.ContentLength = 27; Assert.Equal((long)27, _headers.ContentLength); - Assert.Equal((long)27, _headers.GetParsedValues(KnownHeaders.ContentLength.Descriptor)); + Assert.Equal((long)27, _headers.GetSingleParsedValue(KnownHeaders.ContentLength.Descriptor)); // After explicitly setting the content length, set it to null. _headers.ContentLength = null; @@ -177,13 +177,13 @@ public void ContentLocation_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void ContentLocation_UseAddMethodWithInvalidValue_InvalidValueRecognized() { _headers.TryAddWithoutValidation("Content-Location", " http://example.com http://other"); - Assert.Null(_headers.GetParsedValues(KnownHeaders.ContentLocation.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.ContentLocation.Descriptor)); Assert.Equal(1, _headers.GetValues("Content-Location").Count()); Assert.Equal(" http://example.com http://other", _headers.GetValues("Content-Location").First()); _headers.Clear(); _headers.TryAddWithoutValidation("Content-Location", "http://host /other"); - Assert.Null(_headers.GetParsedValues(KnownHeaders.ContentLocation.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.ContentLocation.Descriptor)); Assert.Equal(1, _headers.GetValues("Content-Location").Count()); Assert.Equal("http://host /other", _headers.GetValues("Content-Location").First()); } @@ -315,13 +315,13 @@ public void ContentMD5_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void ContentMD5_UseAddMethodWithInvalidValue_InvalidValueRecognized() { _headers.TryAddWithoutValidation("Content-MD5", "AQ--"); - Assert.Null(_headers.GetParsedValues(KnownHeaders.ContentMD5.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.ContentMD5.Descriptor)); Assert.Equal(1, _headers.GetValues("Content-MD5").Count()); Assert.Equal("AQ--", _headers.GetValues("Content-MD5").First()); _headers.Clear(); _headers.TryAddWithoutValidation("Content-MD5", "AQ==, CD"); - Assert.Null(_headers.GetParsedValues(KnownHeaders.ContentMD5.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.ContentMD5.Descriptor)); Assert.Equal(1, _headers.GetValues("Content-MD5").Count()); Assert.Equal("AQ==, CD", _headers.GetValues("Content-MD5").First()); } @@ -401,13 +401,13 @@ public void Expires_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void Expires_UseAddMethodWithInvalidValue_InvalidValueRecognized() { _headers.TryAddWithoutValidation("Expires", " Sun, 06 Nov 1994 08:49:37 GMT ,"); - Assert.Null(_headers.GetParsedValues(KnownHeaders.Expires.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.Expires.Descriptor)); Assert.Equal(1, _headers.GetValues("Expires").Count()); Assert.Equal(" Sun, 06 Nov 1994 08:49:37 GMT ,", _headers.GetValues("Expires").First()); _headers.Clear(); _headers.TryAddWithoutValidation("Expires", " Sun, 06 Nov "); - Assert.Null(_headers.GetParsedValues(KnownHeaders.Expires.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.Expires.Descriptor)); Assert.Equal(1, _headers.GetValues("Expires").Count()); Assert.Equal(" Sun, 06 Nov ", _headers.GetValues("Expires").First()); } @@ -441,13 +441,13 @@ public void LastModified_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void LastModified_UseAddMethodWithInvalidValue_InvalidValueRecognized() { _headers.TryAddWithoutValidation("Last-Modified", " Sun, 06 Nov 1994 08:49:37 GMT ,"); - Assert.Null(_headers.GetParsedValues(KnownHeaders.LastModified.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.LastModified.Descriptor)); Assert.Equal(1, _headers.GetValues("Last-Modified").Count()); Assert.Equal(" Sun, 06 Nov 1994 08:49:37 GMT ,", _headers.GetValues("Last-Modified").First()); _headers.Clear(); _headers.TryAddWithoutValidation("Last-Modified", " Sun, 06 Nov "); - Assert.Null(_headers.GetParsedValues(KnownHeaders.LastModified.Descriptor)); + Assert.Null(_headers.GetSingleParsedValue(KnownHeaders.LastModified.Descriptor)); Assert.Equal(1, _headers.GetValues("Last-Modified").Count()); Assert.Equal(" Sun, 06 Nov ", _headers.GetValues("Last-Modified").First()); } diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeaderValueCollectionTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeaderValueCollectionTest.cs index dec7eb661f7cf6..ba9386bba50c6c 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeaderValueCollectionTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeaderValueCollectionTest.cs @@ -262,7 +262,7 @@ public void CopyTo_CallWithStartIndexPlusElementCountGreaterArrayLength_Throw() Uri[] array = new Uri[2]; // startIndex + Count = 1 + 2 > array.Length - AssertExtensions.Throws("destinationArray", "", () => { collection.CopyTo(array, 1); }); + AssertExtensions.Throws(() => { collection.CopyTo(array, 1); }); } [Fact] @@ -346,18 +346,18 @@ public void CopyTo_ArrayTooSmall_Throw() headers.Add(knownStringHeader, "special"); headers.Add(knownStringHeader, "special"); array = new string[1]; - AssertExtensions.Throws("destinationArray", "", () => { collection.CopyTo(array, 0); }); + AssertExtensions.Throws(() => { collection.CopyTo(array, 0); }); headers.Add(knownStringHeader, "value1"); array = new string[0]; - AssertExtensions.Throws("destinationArray", "", () => { collection.CopyTo(array, 0); }); + AssertExtensions.Throws(() => { collection.CopyTo(array, 0); }); headers.Add(knownStringHeader, "value2"); array = new string[1]; - AssertExtensions.Throws("destinationArray", "", () => { collection.CopyTo(array, 0); }); + AssertExtensions.Throws(() => { collection.CopyTo(array, 0); }); array = new string[2]; - AssertExtensions.Throws("destinationArray", "", () => { collection.CopyTo(array, 1); }); + AssertExtensions.Throws(() => { collection.CopyTo(array, 1); }); } [Fact] diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs index 4d4d2598ea3b93..7ec416358208ec 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHeadersTest.cs @@ -35,6 +35,22 @@ static HttpHeadersTest() private const string parsedPrefix = "parsed"; private const string invalidHeaderValue = "invalid"; + [Fact] + public void TryAddWithoutValidation_ValidAndInvalidValues_InsertionOrderIsPreserved() + { + HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, "http://microsoft.com"); + request.Headers.TryAddWithoutValidation("Accept", "text/bar"); + request.Headers.TryAddWithoutValidation("Accept", "invalid"); + request.Headers.TryAddWithoutValidation("Accept", "text/baz"); + + // Force parsing + _ = request.Headers.Accept.Count; + + Assert.Equal(1, request.Headers.NonValidated.Count); + HeaderStringValues accept = request.Headers.NonValidated["Accept"]; + Assert.Equal(new[] { "text/bar", "invalid", "text/baz" }, accept); + } + [Theory] [InlineData(null)] [InlineData("")] @@ -190,21 +206,11 @@ public void TryAddWithoutValidation_AddValidAndInvalidValueString_BothValuesPars Assert.Equal(1, headers.Count()); Assert.Equal(2, headers.First().Value.Count()); - // If you compare this test with the previous one: Note that we reversed the order of adding the invalid - // string and the valid string. However, when enumerating header values the order is still the same as in - // the previous test. - // We don't keep track of the order if we have both invalid & valid values. This would add complexity - // and additional memory to store the information. Given how rare this scenario is we consider this - // by design. Note that this scenario is only an issue if: - // - The header value has an invalid format (very rare for standard headers) AND - // - There are multiple header values (some valid, some invalid) AND - // - The order of the headers matters (e.g. Transfer-Encoding) - Assert.Equal(parsedPrefix, headers.First().Value.ElementAt(0)); - Assert.Equal(invalidHeaderValue, headers.First().Value.ElementAt(1)); - + Assert.Equal(invalidHeaderValue, headers.First().Value.ElementAt(0)); + Assert.Equal(parsedPrefix, headers.First().Value.ElementAt(1)); Assert.Equal(2, headers.Parser.TryParseValueCallCount); - string expected = headers.Descriptor.Name + ": " + parsedPrefix + ", " + invalidHeaderValue + Environment.NewLine; + string expected = headers.Descriptor.Name + ": " + invalidHeaderValue + ", " + parsedPrefix + Environment.NewLine; Assert.Equal(expected, headers.ToString()); } @@ -616,8 +622,8 @@ public void Add_SingleFirstTryAddWithoutValidationForInvalidValueThenAdd_TwoPars Assert.Equal(1, headers.Count()); Assert.Equal(2, headers.First().Value.Count()); - Assert.Equal(parsedPrefix + "1", headers.First().Value.ElementAt(0)); - Assert.Equal(invalidHeaderValue, headers.First().Value.ElementAt(1)); + Assert.Equal(invalidHeaderValue, headers.First().Value.ElementAt(0)); + Assert.Equal(parsedPrefix + "1", headers.First().Value.ElementAt(1)); // Value is already parsed. There shouldn't be additional calls to the parser. Assert.Equal(2, headers.Parser.TryParseValueCallCount); @@ -1265,34 +1271,34 @@ public void GetValues_HeadersWithEmptyValues_ReturnsEmptyArray() } [Fact] - public void GetParsedValues_GetValuesFromUninitializedHeaderStore_ReturnsNull() + public void GetSingleParsedValue_GetValuesFromUninitializedHeaderStore_ReturnsNull() { MockHeaders headers = new MockHeaders(); // Get header values from uninitialized store (store collection is null). - object storeValue = headers.GetParsedValues(customHeader); + object storeValue = headers.GetSingleParsedValue(customHeader); Assert.Null(storeValue); } [Fact] - public void GetParsedValues_GetValuesForNonExistingHeader_ReturnsNull() + public void GetSingleParsedValue_GetValuesForNonExistingHeader_ReturnsNull() { MockHeaders headers = new MockHeaders(); headers.Add("custom1", "customValue1"); // Get header values for non-existing header (but other headers exist in the store). - object storeValue = headers.GetParsedValues(customHeader); + object storeValue = headers.GetSingleParsedValue(customHeader); Assert.Null(storeValue); } [Fact] - public void GetParsedValues_GetSingleValueForExistingHeader_ReturnsAddedValue() + public void GetSingleParsedValue_GetSingleValueForExistingHeader_ReturnsAddedValue() { MockHeaders headers = new MockHeaders(); headers.Add(customHeader.Name, "customValue1"); // Get header values for non-existing header (but other headers exist in the store). - object storeValue = headers.GetParsedValues(customHeader); + object storeValue = headers.GetSingleParsedValue(customHeader); Assert.NotNull(storeValue); // If we only have one value, then GetValues() should return just the value and not wrap it in a List. @@ -1300,17 +1306,17 @@ public void GetParsedValues_GetSingleValueForExistingHeader_ReturnsAddedValue() } [Fact] - public void GetParsedValues_HeaderWithEmptyValues_ReturnsEmpty() + public void GetSingleParsedValue_HeaderWithEmptyValues_ReturnsEmpty() { MockHeaders headers = new MockHeaders(); headers.Add(headers.Descriptor, string.Empty); - object storeValue = headers.GetParsedValues(headers.Descriptor); + object storeValue = headers.GetSingleParsedValue(headers.Descriptor); Assert.Null(storeValue); } [Fact] - public void GetParsedValues_GetMultipleValuesForExistingHeader_ReturnsListOfValues() + public void GetSingleParsedValue_GetMultipleValuesForExistingHeader_ReturnsListOfValues() { MockHeaders headers = new MockHeaders(); headers.TryAddWithoutValidation("custom", rawPrefix + "0"); // this must not influence the result. @@ -1320,7 +1326,7 @@ public void GetParsedValues_GetMultipleValuesForExistingHeader_ReturnsListOfValu // Only 1 value should get parsed (call to Add() with known header value). Assert.Equal(1, headers.Parser.TryParseValueCallCount); - object storeValue = headers.GetParsedValues(headers.Descriptor); + object storeValue = headers.GetParsedAndInvalidValues(headers.Descriptor); Assert.NotNull(storeValue); // GetValues() should trigger parsing of values added with TryAddWithoutValidation() @@ -1336,7 +1342,7 @@ public void GetParsedValues_GetMultipleValuesForExistingHeader_ReturnsListOfValu } [Fact] - public void GetParsedValues_GetValuesForExistingHeaderWithInvalidValues_ReturnsOnlyParsedValues() + public void GetSingleParsedValue_GetValuesForExistingHeaderWithInvalidValues_ReturnsOnlyParsedValues() { MockHeaders headers = new MockHeaders(); headers.Add(headers.Descriptor, rawPrefix); @@ -1349,7 +1355,7 @@ public void GetParsedValues_GetValuesForExistingHeaderWithInvalidValues_ReturnsO // Only 1 value should get parsed (call to Add() with known header value). Assert.Equal(1, headers.Parser.TryParseValueCallCount); - object storeValue = headers.GetParsedValues(headers.Descriptor); + object storeValue = headers.GetSingleParsedValue(headers.Descriptor); Assert.NotNull(storeValue); // GetValues() should trigger parsing of values added with TryAddWithoutValidation() @@ -1360,7 +1366,7 @@ public void GetParsedValues_GetValuesForExistingHeaderWithInvalidValues_ReturnsO } [Fact] - public void GetParsedValues_GetValuesForExistingHeaderWithOnlyInvalidValues_ReturnsEmptyEnumerator() + public void GetSingleParsedValue_GetValuesForExistingHeaderWithOnlyInvalidValues_ReturnsEmptyEnumerator() { MockHeaders headers = new MockHeaders(); @@ -1371,7 +1377,7 @@ public void GetParsedValues_GetValuesForExistingHeaderWithOnlyInvalidValues_Retu Assert.Equal(0, headers.Parser.TryParseValueCallCount); - object storeValue = headers.GetParsedValues(headers.Descriptor); + object storeValue = headers.GetSingleParsedValue(headers.Descriptor); Assert.Null(storeValue); // GetValues() should trigger parsing of values added with TryAddWithoutValidation() @@ -1379,20 +1385,20 @@ public void GetParsedValues_GetValuesForExistingHeaderWithOnlyInvalidValues_Retu } [Fact] - public void GetParsedValues_AddInvalidValueToHeader_HeaderGetsRemovedAndNullReturned() + public void GetSingleParsedValue_AddInvalidValueToHeader_HeaderGetsRemovedAndNullReturned() { MockHeaders headers = new MockHeaders(); headers.TryAddWithoutValidation(headers.Descriptor, invalidHeaderValue + "\r\ninvalid"); Assert.Equal(0, headers.Parser.TryParseValueCallCount); - object storeValue = headers.GetParsedValues(headers.Descriptor); + object storeValue = headers.GetSingleParsedValue(headers.Descriptor); Assert.Null(storeValue); Assert.False(headers.Contains(headers.Descriptor)); } [Fact] - public void GetParsedValues_GetParsedValuesForKnownHeaderWithNewlineChars_ReturnsNull() + public void GetSingleParsedValue_GetSingleParsedValueForKnownHeaderWithNewlineChars_ReturnsNull() { MockHeaders headers = new MockHeaders(); @@ -1400,7 +1406,7 @@ public void GetParsedValues_GetParsedValuesForKnownHeaderWithNewlineChars_Return headers.TryAddWithoutValidation(headers.Descriptor, invalidHeaderValue + "\r\ninvalid"); Assert.Equal(0, headers.Parser.TryParseValueCallCount); - Assert.Null(headers.GetParsedValues(headers.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(headers.Descriptor)); Assert.Equal(0, headers.Count()); Assert.Equal(1, headers.Parser.TryParseValueCallCount); } @@ -2065,9 +2071,8 @@ public void AddHeaders_SourceAndDestinationStoreHaveMultipleHeaders_OnlyHeadersN // invalid value. Assert.Equal(3, destination.GetValues(known2Header).Count()); Assert.Equal(parsedPrefix + "5", destination.GetValues(known2Header).ElementAt(0)); - Assert.Equal(parsedPrefix + "7", destination.GetValues(known2Header).ElementAt(1)); - Assert.Equal(invalidHeaderValue, destination.GetValues(known2Header).ElementAt(2)); - + Assert.Equal(invalidHeaderValue, destination.GetValues(known2Header).ElementAt(1)); + Assert.Equal(parsedPrefix + "7", destination.GetValues(known2Header).ElementAt(2)); // Header 'known3' should not be copied, since it doesn't contain any values. Assert.False(destination.Contains(known3Header), "'known3' header value count."); diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs index 81fa6560bc60c3..b174d077e43060 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpRequestHeadersTest.cs @@ -162,7 +162,7 @@ public void AcceptCharset_AddMultipleValuesAndGetValueString_AllValuesAddedUsing foreach (var header in headers.NonValidated) { Assert.Equal("Accept-Charset", header.Key); - Assert.Equal("utf-8, iso-8859-5; q=0.5, invalid value", header.Value.ToString()); + Assert.Equal("invalid value, utf-8, iso-8859-5; q=0.5", header.Value.ToString()); } } @@ -633,26 +633,26 @@ public void UserAgent_TryGetValuesAndGetValues_Malformed() public void UserAgent_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("User-Agent", "custom\u4F1A"); - Assert.Null(headers.GetParsedValues(KnownHeaders.UserAgent.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.UserAgent.Descriptor)); Assert.Equal(1, headers.GetValues("User-Agent").Count()); Assert.Equal("custom\u4F1A", headers.GetValues("User-Agent").First()); headers.Clear(); // Note that "User-Agent" uses whitespace as separators, so the following is an invalid value headers.TryAddWithoutValidation("User-Agent", "custom1, custom2"); - Assert.Null(headers.GetParsedValues(KnownHeaders.UserAgent.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.UserAgent.Descriptor)); Assert.Equal(1, headers.GetValues("User-Agent").Count()); Assert.Equal("custom1, custom2", headers.GetValues("User-Agent").First()); headers.Clear(); headers.TryAddWithoutValidation("User-Agent", "custom1, "); - Assert.Null(headers.GetParsedValues(KnownHeaders.UserAgent.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.UserAgent.Descriptor)); Assert.Equal(1, headers.GetValues("User-Agent").Count()); Assert.Equal("custom1, ", headers.GetValues("User-Agent").First()); headers.Clear(); headers.TryAddWithoutValidation("User-Agent", ",custom1"); - Assert.Null(headers.GetParsedValues(KnownHeaders.UserAgent.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.UserAgent.Descriptor)); Assert.Equal(1, headers.GetValues("User-Agent").Count()); Assert.Equal(",custom1", headers.GetValues("User-Agent").First()); } @@ -667,7 +667,7 @@ public void UserAgent_AddMultipleValuesAndGetValueString_AllValuesAddedUsingTheC foreach (var header in headers.NonValidated) { Assert.Equal("User-Agent", header.Key); - Assert.Equal("custom2/1.1 (comment) custom\u4F1A", header.Value.ToString()); + Assert.Equal("custom\u4F1A custom2/1.1 (comment)", header.Value.ToString()); } } @@ -704,13 +704,13 @@ public void IfRange_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void IfRange_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("If-Range", "\"tag\"\u4F1A"); - Assert.Null(headers.GetParsedValues(KnownHeaders.IfRange.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.IfRange.Descriptor)); Assert.Equal(1, headers.GetValues("If-Range").Count()); Assert.Equal("\"tag\"\u4F1A", headers.GetValues("If-Range").First()); headers.Clear(); headers.TryAddWithoutValidation("If-Range", " \"tag\", "); - Assert.Null(headers.GetParsedValues(KnownHeaders.IfRange.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.IfRange.Descriptor)); Assert.Equal(1, headers.GetValues("If-Range").Count()); Assert.Equal(" \"tag\", ", headers.GetValues("If-Range").First()); } @@ -759,13 +759,13 @@ public void From_UseAddMethodWithInvalidValue_InvalidValueRecognized() { // values are not validated, so invalid values are accepted headers.TryAddWithoutValidation("From", " info@example.com ,"); - Assert.Equal("info@example.com ,", headers.GetParsedValues(KnownHeaders.From.Descriptor)); + Assert.Equal("info@example.com ,", headers.GetSingleParsedValue(KnownHeaders.From.Descriptor)); Assert.Equal(1, headers.GetValues("From").Count()); Assert.Equal("info@example.com ,", headers.GetValues("From").First()); headers.Clear(); headers.TryAddWithoutValidation("From", "info@"); - Assert.Equal("info@", headers.GetParsedValues(KnownHeaders.From.Descriptor)); + Assert.Equal("info@", headers.GetSingleParsedValue(KnownHeaders.From.Descriptor)); Assert.Equal(1, headers.GetValues("From").Count()); Assert.Equal("info@", headers.GetValues("From").First()); } @@ -807,13 +807,13 @@ public void IfModifiedSince_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void IfModifiedSince_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("If-Modified-Since", " Sun, 06 Nov 1994 08:49:37 GMT ,"); - Assert.Null(headers.GetParsedValues(KnownHeaders.IfModifiedSince.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.IfModifiedSince.Descriptor)); Assert.Equal(1, headers.GetValues("If-Modified-Since").Count()); Assert.Equal(" Sun, 06 Nov 1994 08:49:37 GMT ,", headers.GetValues("If-Modified-Since").First()); headers.Clear(); headers.TryAddWithoutValidation("If-Modified-Since", " Sun, 06 Nov "); - Assert.Null(headers.GetParsedValues(KnownHeaders.IfModifiedSince.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.IfModifiedSince.Descriptor)); Assert.Equal(1, headers.GetValues("If-Modified-Since").Count()); Assert.Equal(" Sun, 06 Nov ", headers.GetValues("If-Modified-Since").First()); } @@ -848,13 +848,13 @@ public void IfUnmodifiedSince_UseAddMethod_AddedValueCanBeRetrievedUsingProperty public void IfUnmodifiedSince_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("If-Unmodified-Since", " Sun, 06 Nov 1994 08:49:37 GMT ,"); - Assert.Null(headers.GetParsedValues(KnownHeaders.IfUnmodifiedSince.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.IfUnmodifiedSince.Descriptor)); Assert.Equal(1, headers.GetValues("If-Unmodified-Since").Count()); Assert.Equal(" Sun, 06 Nov 1994 08:49:37 GMT ,", headers.GetValues("If-Unmodified-Since").First()); headers.Clear(); headers.TryAddWithoutValidation("If-Unmodified-Since", " Sun, 06 Nov "); - Assert.Null(headers.GetParsedValues(KnownHeaders.IfUnmodifiedSince.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.IfUnmodifiedSince.Descriptor)); Assert.Equal(1, headers.GetValues("If-Unmodified-Since").Count()); Assert.Equal(" Sun, 06 Nov ", headers.GetValues("If-Unmodified-Since").First()); } @@ -889,13 +889,13 @@ public void Referrer_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void Referrer_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Referer", " http://example.com http://other"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Referer.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Referer.Descriptor)); Assert.Equal(1, headers.GetValues("Referer").Count()); Assert.Equal(" http://example.com http://other", headers.GetValues("Referer").First()); headers.Clear(); headers.TryAddWithoutValidation("Referer", "http://host /other"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Referer.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Referer.Descriptor)); Assert.Equal(1, headers.GetValues("Referer").Count()); Assert.Equal("http://host /other", headers.GetValues("Referer").First()); } @@ -933,13 +933,13 @@ public void MaxForwards_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void MaxForwards_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Max-Forwards", "15,"); - Assert.Null(headers.GetParsedValues(KnownHeaders.MaxForwards.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.MaxForwards.Descriptor)); Assert.Equal(1, headers.GetValues("Max-Forwards").Count()); Assert.Equal("15,", headers.GetValues("Max-Forwards").First()); headers.Clear(); headers.TryAddWithoutValidation("Max-Forwards", "1.0"); - Assert.Null(headers.GetParsedValues(KnownHeaders.MaxForwards.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.MaxForwards.Descriptor)); Assert.Equal(1, headers.GetValues("Max-Forwards").Count()); Assert.Equal("1.0", headers.GetValues("Max-Forwards").First()); } @@ -1209,13 +1209,13 @@ public void TransferEncoding_UseAddMethod_AddedValueCanBeRetrievedUsingProperty( public void TransferEncoding_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Transfer-Encoding", "custom\u4F1A"); - Assert.Null(headers.GetParsedValues(KnownHeaders.TransferEncoding.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.TransferEncoding.Descriptor)); Assert.Equal(1, headers.GetValues("Transfer-Encoding").Count()); Assert.Equal("custom\u4F1A", headers.GetValues("Transfer-Encoding").First()); headers.Clear(); headers.TryAddWithoutValidation("Transfer-Encoding", "custom1 custom2"); - Assert.Null(headers.GetParsedValues(KnownHeaders.TransferEncoding.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.TransferEncoding.Descriptor)); Assert.Equal(1, headers.GetValues("Transfer-Encoding").Count()); Assert.Equal("custom1 custom2", headers.GetValues("Transfer-Encoding").First()); @@ -1263,13 +1263,13 @@ public void Upgrade_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void Upgrade_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Upgrade", "custom\u4F1A"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Upgrade.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Upgrade.Descriptor)); Assert.Equal(1, headers.GetValues("Upgrade").Count()); Assert.Equal("custom\u4F1A", headers.GetValues("Upgrade").First()); headers.Clear(); headers.TryAddWithoutValidation("Upgrade", "custom1 custom2"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Upgrade.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Upgrade.Descriptor)); Assert.Equal(1, headers.GetValues("Upgrade").Count()); Assert.Equal("custom1 custom2", headers.GetValues("Upgrade").First()); } @@ -1308,13 +1308,13 @@ public void Date_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void Date_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Date", " Sun, 06 Nov 1994 08:49:37 GMT ,"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Date.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Date.Descriptor)); Assert.Equal(1, headers.GetValues("Date").Count()); Assert.Equal(" Sun, 06 Nov 1994 08:49:37 GMT ,", headers.GetValues("Date").First()); headers.Clear(); headers.TryAddWithoutValidation("Date", " Sun, 06 Nov "); - Assert.Null(headers.GetParsedValues(KnownHeaders.Date.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Date.Descriptor)); Assert.Equal(1, headers.GetValues("Date").Count()); Assert.Equal(" Sun, 06 Nov ", headers.GetValues("Date").First()); } @@ -1455,6 +1455,18 @@ public void CacheControl_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() Assert.Equal(value, headers.CacheControl); } + [Fact] + public void CacheControl_ValidAndInvalidValues_ReturnValidValue() + { + headers.TryAddWithoutValidation("Cache-Control", ""); + headers.TryAddWithoutValidation("Cache-Control", "no-cache=\"token1\", must-revalidate, max-age=3"); + headers.TryAddWithoutValidation("Cache-Control", "public, s-maxage=15"); + Assert.True(headers.CacheControl.NoCache); + Assert.True(headers.NonValidated["Cache-Control"].Count == 2); + Assert.Equal("", headers.NonValidated["Cache-Control"].ElementAt(0)); + Assert.Equal("public, must-revalidate, no-cache=\"token1\", max-age=3, s-maxage=15", headers.NonValidated["Cache-Control"].ElementAt(1)); + } + [Fact] public void Trailer_ReadAndWriteProperty_ValueMatchesPriorSetValue() { diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpResponseHeadersTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpResponseHeadersTest.cs index 696e3e656511f3..b0222a76bc6c25 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpResponseHeadersTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpResponseHeadersTest.cs @@ -94,7 +94,7 @@ public void Location_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void Location_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Location", " http://example.com http://other"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Location.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Location.Descriptor)); Assert.Equal(1, headers.GetValues("Location").Count()); Assert.Equal(" http://example.com http://other", headers.GetValues("Location").First()); } @@ -348,26 +348,26 @@ public void Server_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void Server_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Server", "custom\u4F1A"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Server.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Server.Descriptor)); Assert.Equal(1, headers.GetValues("Server").Count()); Assert.Equal("custom\u4F1A", headers.GetValues("Server").First()); headers.Clear(); // Note that "Server" uses whitespace as separators, so the following is an invalid value headers.TryAddWithoutValidation("Server", "custom1, custom2"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Server.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Server.Descriptor)); Assert.Equal(1, headers.GetValues("Server").Count()); Assert.Equal("custom1, custom2", headers.GetValues("Server").First()); headers.Clear(); headers.TryAddWithoutValidation("Server", "custom1, "); - Assert.Null(headers.GetParsedValues(KnownHeaders.Server.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Server.Descriptor)); Assert.Equal(1, headers.GetValues("Server").Count()); Assert.Equal("custom1, ", headers.GetValues("Server").First()); headers.Clear(); headers.TryAddWithoutValidation("Server", ",custom1"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Server.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Server.Descriptor)); Assert.Equal(1, headers.GetValues("Server").Count()); Assert.Equal(",custom1", headers.GetValues("Server").First()); } @@ -491,13 +491,13 @@ public void Age_UseAddMethod_AddedValueCanBeRetrievedUsingProperty() public void Age_UseAddMethodWithInvalidValue_InvalidValueRecognized() { headers.TryAddWithoutValidation("Age", "10,"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Age.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Age.Descriptor)); Assert.Equal(1, headers.GetValues("Age").Count()); Assert.Equal("10,", headers.GetValues("Age").First()); headers.Clear(); headers.TryAddWithoutValidation("Age", "1.1"); - Assert.Null(headers.GetParsedValues(KnownHeaders.Age.Descriptor)); + Assert.Null(headers.GetSingleParsedValue(KnownHeaders.Age.Descriptor)); Assert.Equal(1, headers.GetValues("Age").Count()); Assert.Equal("1.1", headers.GetValues("Age").First()); }