Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not reorder HTTP header values #65249

Merged
merged 31 commits into from
Mar 28, 2022
Merged

Do not reorder HTTP header values #65249

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
1f717f8
Do not reorder HTTP header values
feiyun0112 Feb 12, 2022
02d168a
Merge branch 'main' into fix-63833
feiyun0112 Feb 12, 2022
04e4718
fix complie error
feiyun0112 Feb 13, 2022
c4f503b
make the changes @ danmoseley recommended
feiyun0112 Feb 15, 2022
f5b1cb0
make the changes @MihaZupan recommended
feiyun0112 Feb 16, 2022
542b448
Merge branch 'dotnet:main' into fix-63833
feiyun0112 Feb 19, 2022
2fd48a6
make the changes @MihaZupan recommended
feiyun0112 Feb 19, 2022
cf3716b
make the changes @MihaZupan recommended
feiyun0112 Feb 21, 2022
afd612a
make the changes @MihaZupan recommended
feiyun0112 Feb 28, 2022
92ac1a3
make the changes @MihaZupan recommended
feiyun0112 Mar 1, 2022
ad7aaf8
check array length
feiyun0112 Mar 1, 2022
6f5c6ac
fix unit test
feiyun0112 Mar 8, 2022
263fe0e
Update src/libraries/System.Net.Http/tests/UnitTests/Headers/HttpHead…
feiyun0112 Mar 9, 2022
7cf1a6b
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Http…
feiyun0112 Mar 9, 2022
dfa81e7
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Http…
feiyun0112 Mar 9, 2022
68a5b36
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Http…
feiyun0112 Mar 9, 2022
017a558
make the changes @MihaZupan recommended
feiyun0112 Mar 9, 2022
f7a4636
Merge branch 'fix-63833' of https://github.com/feiyun0112/runtime int…
feiyun0112 Mar 9, 2022
c69be94
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Cach…
feiyun0112 Mar 9, 2022
39c1a98
chang unit test
feiyun0112 Mar 11, 2022
1dcebc9
Merge branch 'fix-63833' of https://github.com/feiyun0112/runtime int…
feiyun0112 Mar 11, 2022
a969560
GetParsedValueLength
feiyun0112 Mar 11, 2022
3684810
fix build error
feiyun0112 Mar 12, 2022
876d130
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Cach…
feiyun0112 Mar 16, 2022
8904a36
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Http…
feiyun0112 Mar 16, 2022
c3cd0af
unit test
feiyun0112 Mar 16, 2022
ef11a8a
make changes @geoffkizer recommended
feiyun0112 Mar 17, 2022
8f69ba8
CloneAndAddValue for InvalidValues
feiyun0112 Mar 21, 2022
682d146
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Http…
feiyun0112 Mar 22, 2022
e50c36d
Update src/libraries/System.Net.Http/src/System/Net/Http/Headers/Http…
feiyun0112 Mar 22, 2022
5badaae
Final nits
MihaZupan Mar 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 111 additions & 63 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ 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))
{
Debug.Assert(!ContainsKey(descriptor));
AddEntryToStore(new HeaderEntry(descriptor, info));
Expand Down Expand Up @@ -114,7 +114,7 @@ internal void Add(HeaderDescriptor descriptor, IEnumerable<string?> 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))
{
Debug.Assert(!ContainsKey(descriptor));
AddEntryToStore(new HeaderEntry(descriptor, info));
Expand Down Expand Up @@ -385,8 +385,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);
Expand Down Expand Up @@ -419,47 +418,54 @@ 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;
}

bool result = false;
IEqualityComparer? comparer = descriptor.Parser.Comparer;

List<object>? parsedValues = info.ParsedValue as List<object>;
List<object>? parsedValues = parsedValue as List<object>;
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(),
if (parsedValue is not InvalidValue)
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(item.GetType() == value.GetType(),
"One of the stored values does not have the same type as 'value'.");
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved

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 (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.ParsedAndInvalidValues = null;
}
}

Expand Down Expand Up @@ -489,32 +495,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<object>? parsedValues = info.ParsedValue as List<object>;
List<object>? parsedValues = parsedValue as List<object>;

IEqualityComparer? comparer = descriptor.Parser.Comparer;

if (parsedValues == null)
{
Debug.Assert(info.ParsedValue.GetType() == value.GetType(),
if (parsedValue is not InvalidValue)
{
Debug.Assert(parsedValue.GetType() == value.GetType(),
"Stored value does not have the same type as 'value'.");
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved

return AreEqual(value, info.ParsedValue, comparer);
return AreEqual(value, parsedValue, comparer);
}
}
else
{
foreach (object item in parsedValues)
{
Debug.Assert(item.GetType() == value.GetType(),
if (parsedValue is not InvalidValue)
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(item.GetType() == value.GetType(),
"One of the stored values does not have the same type as 'value'.");
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved

if (AreEqual(value, item, comparer))
{
return true;
if (AreEqual(value, item, comparer))
{
return true;
}
}
}

Expand Down Expand Up @@ -585,26 +598,17 @@ private HeaderStoreItemInfo CloneHeaderInfo(HeaderDescriptor descriptor, HeaderS

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);
destinationInfo.ParsedAndInvalidValues = CloneStringHeaderInfoValues(sourceInfo.ParsedAndInvalidValues);
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
}
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)
if (sourceInfo.ParsedAndInvalidValues != null)
{
List<object>? sourceValues = sourceInfo.ParsedValue as List<object>;
List<object>? sourceValues = sourceInfo.ParsedAndInvalidValues as List<object>;
if (sourceValues == null)
{
CloneAndAddValue(destinationInfo, sourceInfo.ParsedValue);
CloneAndAddValue(destinationInfo, sourceInfo.ParsedAndInvalidValues);
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand Down Expand Up @@ -744,7 +748,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);
Expand Down Expand Up @@ -813,7 +817,7 @@ 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))
{
// 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.
Expand Down Expand Up @@ -845,7 +849,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))
Expand All @@ -868,7 +872,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)
{
Expand Down Expand Up @@ -907,12 +911,12 @@ private static void AddParsedValue(HeaderStoreItemInfo info, object value)
"Header value types must not derive from List<object> 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<object>(value, ref info.ParsedValue);
AddValueToStoreValue<object>(value, ref info.ParsedAndInvalidValues);
}

private static void AddInvalidValue(HeaderStoreItemInfo info, string value)
{
AddValueToStoreValue<string>(value, ref info.InvalidValue);
AddValueToStoreValue<object>(new InvalidValue(value), ref info.ParsedAndInvalidValues);
}

private static void AddRawValue(HeaderStoreItemInfo info, string value)
Expand Down Expand Up @@ -955,7 +959,49 @@ private static void AddValueToStoreValue<T>(T value, ref object? currentStoreVal
return null;
}

return info.ParsedValue;
var parsedAndInvalidValues = info.ParsedAndInvalidValues;
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
if (parsedAndInvalidValues is null)
{
return null;
}

if (parsedAndInvalidValues is List<object> list)
{
object? parsedValue = null;
foreach (object value in list)
{
if (value is not InvalidValue)
{
if (parsedValue == null)
{
parsedValue = value;
}
else
{
List<object>? storeValues = parsedValue as List<object>;

if (storeValues == null)
{
storeValues = new List<object>(2);
storeValues.Add(parsedValue);
parsedValue = storeValues;
}
storeValues.Add(value);
}
}
}

return parsedValue;
}
else
{
if (parsedAndInvalidValues is not InvalidValue)
{
return parsedAndInvalidValues;
}

return null;
}
}

internal virtual bool IsAllowedHeaderName(HeaderDescriptor descriptor) => true;
Expand Down Expand Up @@ -996,7 +1042,7 @@ private void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreItemInfo i
}

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
Expand All @@ -1023,7 +1069,7 @@ private void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreItemInfo i

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);
Expand Down Expand Up @@ -1146,8 +1192,8 @@ internal static void GetStoreValuesAsStringOrStringArray(HeaderDescriptor descri

int currentIndex = 0;
ReadStoreValues<string?>(values, info.RawValue, null, ref currentIndex);
ReadStoreValues<object?>(values, info.ParsedValue, descriptor.Parser, ref currentIndex);
ReadStoreValues<string?>(values, info.InvalidValue, null, ref currentIndex);
ReadStoreValues<object?>(values, info.ParsedAndInvalidValues, descriptor.Parser, ref currentIndex);

Debug.Assert(currentIndex == length);
}

Expand Down Expand Up @@ -1180,8 +1226,7 @@ internal static int GetStoreValuesIntoStringArray(HeaderDescriptor descriptor, o

int currentIndex = 0;
ReadStoreValues<string?>(values, info.RawValue, null, ref currentIndex);
ReadStoreValues<object?>(values, info.ParsedValue, descriptor.Parser, ref currentIndex);
ReadStoreValues<string?>(values, info.InvalidValue, null, ref currentIndex);
ReadStoreValues<object?>(values, info.ParsedAndInvalidValues, descriptor.Parser, ref currentIndex);
Debug.Assert(currentIndex == length);
}

Expand All @@ -1193,8 +1238,7 @@ private static int GetValueCount(HeaderStoreItemInfo info)
Debug.Assert(info != null);

int valueCount = Count<string>(info.RawValue);
valueCount += Count<string>(info.InvalidValue);
valueCount += Count<object>(info.ParsedValue);
valueCount += Count<object>(info.ParsedAndInvalidValues);
return valueCount;

static int Count<T>(object? valueStore) =>
Expand All @@ -1211,15 +1255,15 @@ private static void ReadStoreValues<T>(Span<string?> 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
{
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++;
}
}
Expand All @@ -1239,13 +1283,17 @@ private bool AreEqual(object value, object? storeValue, IEqualityComparer? compa
return value.Equals(storeValue);
}

internal record InvalidValue(string value)
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
{
public override string ToString() => value;
}

internal sealed class HeaderStoreItemInfo
{
internal HeaderStoreItemInfo() { }

internal object? RawValue;
internal object? InvalidValue;
internal object? ParsedValue;
internal object? ParsedAndInvalidValues;
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved

internal bool CanAddParsedValue(HttpHeaderParser parser)
{
Expand All @@ -1260,10 +1308,10 @@ 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 == null;
}

internal bool IsEmpty => (RawValue == null) && (InvalidValue == null) && (ParsedValue == null);
internal bool IsEmpty => (RawValue == null) && ParsedAndInvalidValues == null;
feiyun0112 marked this conversation as resolved.
Show resolved Hide resolved
}


Expand Down
Loading