Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
eb8fe9f
Change HttpHeaders backing store to an array
MihaZupan Dec 18, 2021
d23ad18
Reduce the size of HeaderDescriptor to 1 object
MihaZupan Dec 18, 2021
18c2eaf
Update UnitTests, fix GetOrCreateHeaderInfo
MihaZupan Dec 18, 2021
59be8e5
Switch to a dictionary after ArrayThreshold headers
MihaZupan Dec 20, 2021
7c06284
Add unit tests
MihaZupan Dec 20, 2021
344a48d
Use storeValueRef naming consistently
MihaZupan Dec 20, 2021
e692089
Workaround field layout regression (#63005)
MihaZupan Dec 20, 2021
2e62a23
Mark _descriptor on HeaderDescriptor as nullable
MihaZupan Dec 20, 2021
36626b7
Remove HeaderDescriptor.Descriptor and add HasValue, IsKnownHeader, E…
MihaZupan Dec 21, 2021
0a70036
Simplify HttpHeaderParser.Separator logic
MihaZupan Dec 21, 2021
3f1275b
Add comments on HasValue checks
MihaZupan Dec 21, 2021
af68113
Lazily group headers by name
MihaZupan Jan 4, 2022
2787afc
Add a header ordering+grouping test
MihaZupan Jan 4, 2022
1716411
Make use of the _count field
MihaZupan Jan 6, 2022
58fb231
Revert all HeaderDescriptor changes from PR
MihaZupan Jan 6, 2022
2e4b79a
Switch back to always grouping by name
MihaZupan Jan 12, 2022
1864797
Assert that the collection is not empty in GetEnumeratorCore
MihaZupan Jan 19, 2022
076b219
Optimize AddHeaders for empty collections
MihaZupan Jan 19, 2022
5d681d6
Reference the Roslyn bug issue
MihaZupan Jan 19, 2022
5babbda
Assert that multiValues are never empty
MihaZupan Jan 19, 2022
468cf3d
Don't preserve a Dictionary across Clear
MihaZupan Jan 19, 2022
b3aee48
Add comment about why a custom HeaderEntry type is used
MihaZupan Jan 19, 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
574 changes: 413 additions & 161 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ namespace System.Net.Http.Headers

/// <summary>Gets the number of headers stored in the collection.</summary>
/// <remarks>Multiple header values associated with the same header name are considered to be one header as far as this count is concerned.</remarks>
public int Count => _headers?.HeaderStore?.Count ?? 0;
public int Count => _headers?.Count ?? 0;

/// <summary>Gets whether the collection contains the specified header.</summary>
/// <param name="headerName">The name of the header.</param>
/// <returns>true if the collection contains the header; otherwise, false.</returns>
public bool Contains(string headerName) =>
_headers is HttpHeaders headers &&
headers.TryGetHeaderDescriptor(headerName, out HeaderDescriptor descriptor) &&
headers.TryGetHeaderValue(descriptor, out _);
headers.ContainsKey(descriptor);

/// <summary>Gets the values for the specified header name.</summary>
/// <param name="headerName">The name of the header.</param>
Expand Down Expand Up @@ -83,8 +83,8 @@ public bool TryGetValues(string headerName, out HeaderStringValues values)
/// <summary>Gets an enumerator that iterates through the <see cref="HttpHeadersNonValidated"/>.</summary>
/// <returns>An enumerator that iterates through the <see cref="HttpHeadersNonValidated"/>.</returns>
public Enumerator GetEnumerator() =>
_headers is HttpHeaders headers && headers.HeaderStore is Dictionary<HeaderDescriptor, object> store ?
new Enumerator(store.GetEnumerator()) :
_headers is HttpHeaders headers && headers.GetEntriesArray() is HeaderEntry[] entries ?
new Enumerator(entries, headers.Count) :
default;

/// <inheritdoc/>
Expand Down Expand Up @@ -120,35 +120,34 @@ IEnumerable<HeaderStringValues> IReadOnlyDictionary<string, HeaderStringValues>.
/// <summary>Enumerates the elements of a <see cref="HttpHeadersNonValidated"/>.</summary>
public struct Enumerator : IEnumerator<KeyValuePair<string, HeaderStringValues>>
{
/// <summary>The wrapped enumerator for the underlying headers dictionary.</summary>
private Dictionary<HeaderDescriptor, object>.Enumerator _headerStoreEnumerator;
/// <summary>The current value.</summary>
private readonly HeaderEntry[] _entries;
private readonly int _numberOfEntries;
private int _index;
private KeyValuePair<string, HeaderStringValues> _current;
/// <summary>true if the enumerator was constructed via the ctor; otherwise, false./</summary>
private bool _valid;

/// <summary>Initializes the enumerator.</summary>
/// <param name="headerStoreEnumerator">The underlying dictionary enumerator.</param>
internal Enumerator(Dictionary<HeaderDescriptor, object>.Enumerator headerStoreEnumerator)
internal Enumerator(HeaderEntry[] entries, int numberOfEntries)
{
_headerStoreEnumerator = headerStoreEnumerator;
_entries = entries;
_numberOfEntries = numberOfEntries;
_index = 0;
_current = default;
_valid = true;
}

/// <inheritdoc/>
public bool MoveNext()
{
if (_valid && _headerStoreEnumerator.MoveNext())
int index = _index;
if (_entries is HeaderEntry[] entries && index < _numberOfEntries && (uint)index < (uint)entries.Length)
{
KeyValuePair<HeaderDescriptor, object> current = _headerStoreEnumerator.Current;
HeaderEntry entry = entries[index];
_index++;

HttpHeaders.GetStoreValuesAsStringOrStringArray(current.Key, current.Value, out string? singleValue, out string[]? multiValue);
HttpHeaders.GetStoreValuesAsStringOrStringArray(entry.Key, entry.Value, out string? singleValue, out string[]? multiValue);
Debug.Assert(singleValue is not null ^ multiValue is not null);

_current = new KeyValuePair<string, HeaderStringValues>(
current.Key.Name,
singleValue is not null ? new HeaderStringValues(current.Key, singleValue) : new HeaderStringValues(current.Key, multiValue!));
entry.Key.Name,
singleValue is not null ? new HeaderStringValues(entry.Key, singleValue) : new HeaderStringValues(entry.Key, multiValue!));
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1337,15 +1337,10 @@ private void WriteHeaderCollection(HttpRequestMessage request, HttpHeaders heade
{
if (NetEventSource.Log.IsEnabled()) Trace("");

if (headers.HeaderStore is null)
{
return;
}

HeaderEncodingSelector<HttpRequestMessage>? encodingSelector = _pool.Settings._requestHeaderEncodingSelector;

ref string[]? tmpHeaderValuesArray = ref t_headerValues;
foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
foreach (HeaderEntry header in headers.GetEntries())
{
int headerValuesCount = HttpHeaders.GetStoreValuesIntoStringArray(header.Key, header.Value, ref tmpHeaderValuesArray);
Debug.Assert(headerValuesCount > 0, "No values for header??");
Expand All @@ -1361,7 +1356,7 @@ private void WriteHeaderCollection(HttpRequestMessage request, HttpHeaders heade
// The Connection, Upgrade and ProxyConnection headers are also not supported in HTTP2.
if (knownHeader != KnownHeaders.Host && knownHeader != KnownHeaders.Connection && knownHeader != KnownHeaders.Upgrade && knownHeader != KnownHeaders.ProxyConnection)
{
if (header.Key.KnownHeader == KnownHeaders.TE)
if (knownHeader == KnownHeaders.TE)
{
// HTTP/2 allows only 'trailers' TE header. rfc7540 8.1.2.2
foreach (string value in headerValues)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,14 +622,9 @@ private void BufferHeaders(HttpRequestMessage request)
// TODO: special-case Content-Type for static table values values?
private void BufferHeaderCollection(HttpHeaders headers)
{
if (headers.HeaderStore == null)
{
return;
}

HeaderEncodingSelector<HttpRequestMessage>? encodingSelector = _connection.Pool.Settings._requestHeaderEncodingSelector;

foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
foreach (HeaderEntry header in headers.GetEntries())
{
int headerValuesCount = HttpHeaders.GetStoreValuesIntoStringArray(header.Key, header.Value, ref _headerValues);
Debug.Assert(headerValuesCount > 0, "No values for header??");
Expand All @@ -645,7 +640,7 @@ private void BufferHeaderCollection(HttpHeaders headers)
// The Connection, Upgrade and ProxyConnection headers are also not supported in HTTP/3.
if (knownHeader != KnownHeaders.Host && knownHeader != KnownHeaders.Connection && knownHeader != KnownHeaders.Upgrade && knownHeader != KnownHeaders.ProxyConnection)
{
if (header.Key.KnownHeader == KnownHeaders.TE)
if (knownHeader == KnownHeaders.TE)
{
// HTTP/2 allows only 'trailers' TE header. rfc7540 8.1.2.2
// HTTP/3 does not mention this one way or another; assume it has the same rule.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,12 @@ private async ValueTask WriteHeadersAsync(HttpHeaders headers, string? cookiesFr
{
Debug.Assert(_currentRequest != null);

if (headers.HeaderStore != null)
if (headers.GetEntriesArray() is HeaderEntry[] entries)
{
foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
for (int i = 0; i < headers.Count; i++)
{
HeaderEntry header = entries[i];

if (header.Key.KnownHeader != null)
{
await WriteBytesAsync(header.Key.KnownHeader.AsciiBytesWithColonSpace, async).ConfigureAwait(false);
Expand Down Expand Up @@ -298,10 +300,10 @@ private async ValueTask WriteHeadersAsync(HttpHeaders headers, string? cookiesFr
separator = parser.Separator!;
}

for (int i = 1; i < headerValuesCount; i++)
for (int j = 1; j < headerValuesCount; j++)
{
await WriteAsciiStringAsync(separator, async).ConfigureAwait(false);
await WriteStringAsync(_headerValues[i], async, valueEncoding).ConfigureAwait(false);
await WriteStringAsync(_headerValues[j], async, valueEncoding).ConfigureAwait(false);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private static Memory<byte> HPackEncode(HttpHeaders headers, Encoding? valueEnco
FillAvailableSpaceWithOnes(buffer);
string[] headerValues = Array.Empty<string>();

foreach (KeyValuePair<HeaderDescriptor, object> header in headers.HeaderStore)
foreach (HeaderEntry header in headers.GetEntries())
{
int headerValuesCount = HttpHeaders.GetStoreValuesIntoStringArray(header.Key, header.Value, ref headerValues);
Assert.InRange(headerValuesCount, 0, int.MaxValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1710,6 +1710,24 @@ public void GetEnumerator_UseExplicitInterfaceImplementation_EnumeratorReturnsNo
Assert.False(enumerator.MoveNext(), "Only 2 values expected, but enumerator returns a third one.");
}

[Fact]
public void GetEnumerator_InvalidValueBetweenValidHeaders_EnumeratorReturnsAllValidValuesAndRemovesInvalidValue()
{
MockHeaders headers = new MockHeaders();
headers.TryAddWithoutValidation("foo", "fooValue");
headers.TryAddWithoutValidation("invalid", "invalid\nvalue");
headers.TryAddWithoutValidation("bar", "barValue");

Assert.Equal(3, headers.Count);

IDictionary<string, IEnumerable<string>> dict = headers.ToDictionary(pair => pair.Key, pair => pair.Value);
Assert.Equal("fooValue", Assert.Single(Assert.Contains("foo", dict)));
Assert.Equal("barValue", Assert.Single(Assert.Contains("bar", dict)));

Assert.Equal(2, headers.Count);
Assert.False(headers.NonValidated.Contains("invalid"));
}

[Fact]
public void AddParsedValue_AddSingleValueToNonExistingHeader_HeaderGetsCreatedAndValueAdded()
{
Expand Down Expand Up @@ -2208,6 +2226,150 @@ public void HeaderStringValues_Constructed_ProducesExpectedResults()
}
}

[Theory]
[MemberData(nameof(NumberOfHeadersUpToArrayThreshold_AddNonValidated_EnumerateNonValidated))]
public void Add_WithinArrayThresholdHeaders_EnumerationPreservesOrdering(int numberOfHeaders, bool addNonValidated, bool enumerateNonValidated)
{
var headers = new MockHeaders();

for (int i = 0; i < numberOfHeaders; i++)
{
if (addNonValidated)
{
headers.TryAddWithoutValidation(i.ToString(), i.ToString());
}
else
{
headers.Add(i.ToString(), i.ToString());
}
}

KeyValuePair<string, string>[] entries = enumerateNonValidated
? headers.NonValidated.Select(pair => KeyValuePair.Create(pair.Key, Assert.Single(pair.Value))).ToArray()
: headers.Select(pair => KeyValuePair.Create(pair.Key, Assert.Single(pair.Value))).ToArray();

Assert.Equal(numberOfHeaders, entries.Length);
for (int i = 0; i < numberOfHeaders; i++)
{
Assert.Equal(i.ToString(), entries[i].Key);
Assert.Equal(i.ToString(), entries[i].Value);
}
}

[Fact]
public void Add_Remove_HeaderOrderingIsPreserved()
{
var headers = new MockHeaders();
headers.Add("a", "");
headers.Add("b", "");
headers.Add("c", "");

headers.Remove("b");

Assert.Equal(new[] { "a", "c" }, headers.Select(pair => pair.Key));
}

[Fact]
public void Add_AddToExistingKey_OriginalOrderingIsPreserved()
{
var headers = new MockHeaders();
headers.Add("a", "a1");
headers.Add("b", "b1");
headers.Add("a", "a2");

Assert.Equal(new[] { "a", "b" }, headers.Select(pair => pair.Key));
}

[Theory]
[InlineData(3)]
[InlineData(4)]
[InlineData(5)]
[InlineData(HttpHeaders.ArrayThreshold / 4)]
[InlineData(HttpHeaders.ArrayThreshold / 2)]
[InlineData(HttpHeaders.ArrayThreshold - 1)]
[InlineData(HttpHeaders.ArrayThreshold)]
[InlineData(HttpHeaders.ArrayThreshold + 1)]
[InlineData(HttpHeaders.ArrayThreshold * 2)]
[InlineData(HttpHeaders.ArrayThreshold * 4)]
public void Add_LargeNumberOfHeaders_OperationsStillSupported(int numberOfHeaders)
{
string[] keys = Enumerable.Range(1, numberOfHeaders).Select(i => i.ToString()).ToArray();

var headers = new MockHeaders();
foreach (string key in keys)
{
Assert.False(headers.NonValidated.Contains(key));
headers.TryAddWithoutValidation(key, key);
Assert.True(headers.NonValidated.Contains(key));
}

string[] nonValidatedKeys = headers.NonValidated.Select(pair => pair.Key).ToArray();
Assert.Equal(numberOfHeaders, nonValidatedKeys.Length);

string[] newKeys = headers.Select(pair => pair.Key).ToArray();
Assert.Equal(numberOfHeaders, newKeys.Length);

string[] nonValidatedKeysAfterValidation = headers.NonValidated.Select(pair => pair.Key).ToArray();
Assert.Equal(numberOfHeaders, nonValidatedKeysAfterValidation.Length);

if (numberOfHeaders > HttpHeaders.ArrayThreshold)
{
// Ordering is lost when adding more than ArrayThreshold headers
Array.Sort(nonValidatedKeys, (a, b) => int.Parse(a).CompareTo(int.Parse(b)));
Array.Sort(newKeys, (a, b) => int.Parse(a).CompareTo(int.Parse(b)));
Array.Sort(nonValidatedKeysAfterValidation, (a, b) => int.Parse(a).CompareTo(int.Parse(b)));
}
Assert.Equal(keys, nonValidatedKeys);
Assert.Equal(keys, newKeys);
Assert.Equal(keys, nonValidatedKeysAfterValidation);

headers.Add("3", "secondValue");
Assert.True(headers.TryGetValues("3", out IEnumerable<string> valuesFor3));
Assert.Equal(new[] { "3", "secondValue" }, valuesFor3);

Assert.True(headers.TryAddWithoutValidation("invalid", "invalid\nvalue"));
Assert.True(headers.TryAddWithoutValidation("valid", "validValue"));

Assert.Equal(numberOfHeaders + 2, headers.NonValidated.Count);

// Remove all headers except for "1", "valid", "invalid"
for (int i = 2; i <= numberOfHeaders; i++)
{
Assert.True(headers.Remove(i.ToString()));
}

Assert.False(headers.Remove("3"));

// "1", "invalid", "valid"
Assert.True(headers.NonValidated.Contains("invalid"));
Assert.Equal(3, headers.NonValidated.Count);

Assert.Equal(new[] { "1", "valid" }, headers.Select(pair => pair.Key).OrderBy(i => i));

Assert.Equal(2, headers.NonValidated.Count);

headers.Clear();

Assert.Equal(0, headers.NonValidated.Count);
Assert.Empty(headers);
Assert.False(headers.Contains("3"));

Assert.True(headers.TryAddWithoutValidation("3", "newValue"));
Assert.True(headers.TryGetValues("3", out valuesFor3));
Assert.Equal(new[] { "newValue" }, valuesFor3);
}

public static IEnumerable<object[]> NumberOfHeadersUpToArrayThreshold_AddNonValidated_EnumerateNonValidated()
{
for (int i = 0; i <= HttpHeaders.ArrayThreshold; i++)
{
yield return new object[] { i, false, false };
yield return new object[] { i, false, true };
yield return new object[] { i, true, false };
yield return new object[] { i, true, true };
}
}

public static IEnumerable<object[]> GetInvalidHeaderNames()
{
yield return new object[] { "invalid header" };
Expand Down