Skip to content

Reduce size of HttpHeaderValueCollection #63057

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

Merged
geoffkizer merged 8 commits into from
Dec 23, 2021
Merged

Reduce size of HttpHeaderValueCollection #63057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f2dbe34
fix Connection: close
Dec 21, 2021
ff2bf3b
fix Transfer-Encoding: chunked
Dec 21, 2021
38e9ca3
fix Expect: 100-continue
Dec 21, 2021
e9d0140
cleanup comments
Dec 21, 2021
43753fb
remove dead code and tests
Dec 21, 2021
9164e27
add validator asserts and remove validator tests
Dec 21, 2021
9e8ca5b
cleanup
Dec 21, 2021
a45678e
remove validator and replace validation logic
Dec 21, 2021
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
9 changes: 0 additions & 9 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Text;

namespace System.Net.Http.Headers
Expand All @@ -22,9 +21,6 @@ internal static class HeaderUtilities

internal const string BytesUnit = "bytes";

// Validator
internal static readonly Action<HttpHeaderValueCollection<string>, string> TokenValidator = ValidateToken;

internal static void SetQuality(UnvalidatedObjectCollection<NameValueHeaderValue> parameters, double? value)
{
Debug.Assert(parameters != null);
Expand Down Expand Up @@ -372,11 +368,6 @@ internal static void DumpHeaders(StringBuilder sb, params HttpHeaders?[] headers
sb.Append('}');
}

private static void ValidateToken(HttpHeaderValueCollection<string> collection, string value)
{
CheckValidToken(value, "item");
}

internal static UnvalidatedObjectCollection<NameValueHeaderValue>? Clone(this UnvalidatedObjectCollection<NameValueHeaderValue>? source)
{
if (source == null)
Expand Down
9 changes: 3 additions & 6 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpContentHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ public ICollection<string> Allow
{
if (_allow == null)
{
_allow = new HttpHeaderValueCollection<string>(KnownHeaders.Allow.Descriptor,
this, HeaderUtilities.TokenValidator);
_allow = new HttpHeaderValueCollection<string>(KnownHeaders.Allow.Descriptor, this);
}
return _allow;
}
Copy link
Member

@stephentoub stephentoub Jan 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future cleanup, this can be simplified to just:

public ICollection<string> Allow => _allow ??= new HttpHeaderValueCollection<string>(KnownHeaders.Allow.Descriptor, this);

Same for some other props below.

Expand All @@ -43,8 +42,7 @@ public ICollection<string> ContentEncoding
{
if (_contentEncoding == null)
{
_contentEncoding = new HttpHeaderValueCollection<string>(KnownHeaders.ContentEncoding.Descriptor,
this, HeaderUtilities.TokenValidator);
_contentEncoding = new HttpHeaderValueCollection<string>(KnownHeaders.ContentEncoding.Descriptor, this);
}
return _contentEncoding;
}
Expand All @@ -56,8 +54,7 @@ public ICollection<string> ContentLanguage
{
if (_contentLanguage == null)
{
_contentLanguage = new HttpHeaderValueCollection<string>(KnownHeaders.ContentLanguage.Descriptor,
this, HeaderUtilities.TokenValidator);
_contentLanguage = new HttpHeaderValueCollection<string>(KnownHeaders.ContentLanguage.Descriptor, this);
}
return _contentLanguage;
}
Expand Down
65 changes: 19 additions & 46 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpGeneralHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ public CacheControlHeaderValue? CacheControl
set { _parent.SetOrRemoveParsedValue(KnownHeaders.CacheControl.Descriptor, value); }
}

public HttpHeaderValueCollection<string> Connection
{
get { return ConnectionCore; }
}

public bool? ConnectionClose
{
get
Expand All @@ -46,30 +41,23 @@ public bool? ConnectionClose
if (value == true)
{
_connectionCloseSet = true;
ConnectionCore.SetSpecialValue();
if (!_parent.ContainsParsedValue(KnownHeaders.Connection.Descriptor, HeaderUtilities.ConnectionClose))
{
_parent.AddParsedValue(KnownHeaders.Connection.Descriptor, HeaderUtilities.ConnectionClose);
}
}
else
{
_connectionCloseSet = value != null;
ConnectionCore.RemoveSpecialValue();
// We intentionally ignore the return value. It's OK if "close" wasn't in the store.
_parent.RemoveParsedValue(KnownHeaders.Connection.Descriptor, HeaderUtilities.ConnectionClose);
}
}
}

internal static bool? GetConnectionClose(HttpHeaders parent, HttpGeneralHeaders? headers)
{
// If we've already initialized the connection header value collection
// and it contains the special value, or if we haven't and the headers contain
// the parsed special value, return true. We don't just access ConnectionCore,
// as doing so will unnecessarily initialize the collection even if it's not needed.
if (headers?._connection != null)
{
if (headers._connection.IsSpecialValueSet)
{
return true;
}
}
else if (parent.ContainsParsedValue(KnownHeaders.Connection.Descriptor, HeaderUtilities.ConnectionClose))
if (parent.ContainsParsedValue(KnownHeaders.Connection.Descriptor, HeaderUtilities.ConnectionClose))
{
return true;
}
Expand Down Expand Up @@ -104,32 +92,15 @@ public HttpHeaderValueCollection<string> Trailer
{
if (_trailer == null)
{
_trailer = new HttpHeaderValueCollection<string>(KnownHeaders.Trailer.Descriptor,
_parent, HeaderUtilities.TokenValidator);
_trailer = new HttpHeaderValueCollection<string>(KnownHeaders.Trailer.Descriptor, _parent);
}
return _trailer;
}
}

public HttpHeaderValueCollection<TransferCodingHeaderValue> TransferEncoding
{
get { return TransferEncodingCore; }
}

internal static bool? GetTransferEncodingChunked(HttpHeaders parent, HttpGeneralHeaders? headers)
{
// If we've already initialized the transfer encoding header value collection
// and it contains the special value, or if we haven't and the headers contain
// the parsed special value, return true. We don't just access TransferEncodingCore,
// as doing so will unnecessarily initialize the collection even if it's not needed.
if (headers?._transferEncoding != null)
{
if (headers._transferEncoding.IsSpecialValueSet)
{
return true;
}
}
else if (parent.ContainsParsedValue(KnownHeaders.TransferEncoding.Descriptor, HeaderUtilities.TransferEncodingChunked))
if (parent.ContainsParsedValue(KnownHeaders.TransferEncoding.Descriptor, HeaderUtilities.TransferEncodingChunked))
{
return true;
}
Expand All @@ -154,12 +125,16 @@ public bool? TransferEncodingChunked
if (value == true)
{
_transferEncodingChunkedSet = true;
TransferEncodingCore.SetSpecialValue();
if (!_parent.ContainsParsedValue(KnownHeaders.TransferEncoding.Descriptor, HeaderUtilities.TransferEncodingChunked))
{
_parent.AddParsedValue(KnownHeaders.TransferEncoding.Descriptor, HeaderUtilities.TransferEncodingChunked);
}
}
else
{
_transferEncodingChunkedSet = value != null;
TransferEncodingCore.RemoveSpecialValue();
// We intentionally ignore the return value. It's OK if "chunked" wasn't in the store.
_parent.RemoveParsedValue(KnownHeaders.TransferEncoding.Descriptor, HeaderUtilities.TransferEncodingChunked);
}
}
}
Expand Down Expand Up @@ -200,27 +175,25 @@ public HttpHeaderValueCollection<WarningHeaderValue> Warning
}
}

private HttpHeaderValueCollection<string> ConnectionCore
public HttpHeaderValueCollection<string> Connection
{
get
{
if (_connection == null)
{
_connection = new HttpHeaderValueCollection<string>(KnownHeaders.Connection.Descriptor,
_parent, HeaderUtilities.ConnectionClose, HeaderUtilities.TokenValidator);
_connection = new HttpHeaderValueCollection<string>(KnownHeaders.Connection.Descriptor, _parent);
}
return _connection;
}
}

private HttpHeaderValueCollection<TransferCodingHeaderValue> TransferEncodingCore
public HttpHeaderValueCollection<TransferCodingHeaderValue> TransferEncoding
{
get
{
if (_transferEncoding == null)
{
_transferEncoding = new HttpHeaderValueCollection<TransferCodingHeaderValue>(
KnownHeaders.TransferEncoding.Descriptor, _parent, HeaderUtilities.TransferEncodingChunked);
_transferEncoding = new HttpHeaderValueCollection<TransferCodingHeaderValue>(KnownHeaders.TransferEncoding.Descriptor, _parent);
}
return _transferEncoding;
}
Expand Down
68 changes: 6 additions & 62 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderValueCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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;
using System.Collections.Generic;
using System.Diagnostics;

Expand Down Expand Up @@ -35,8 +34,6 @@ public sealed class HttpHeaderValueCollection<T> : ICollection<T> where T : clas
{
private readonly HeaderDescriptor _descriptor;
private readonly HttpHeaders _store;
private readonly T? _specialValue;
private readonly Action<HttpHeaderValueCollection<T>, T>? _validator;

public int Count
{
Expand All @@ -48,45 +45,10 @@ public bool IsReadOnly
get { return false; }
}

internal bool IsSpecialValueSet
{
get
{
// If this collection instance has a "special value", then check whether that value was already set.
if (_specialValue == null)
{
return false;
}
return _store.ContainsParsedValue(_descriptor, _specialValue);
}
}

internal HttpHeaderValueCollection(HeaderDescriptor descriptor, HttpHeaders store)
: this(descriptor, store, null, null)
{
}

internal HttpHeaderValueCollection(HeaderDescriptor descriptor, HttpHeaders store,
Action<HttpHeaderValueCollection<T>, T> validator)
: this(descriptor, store, null, validator)
{
}

internal HttpHeaderValueCollection(HeaderDescriptor descriptor, HttpHeaders store, T specialValue)
: this(descriptor, store, specialValue, null)
{
}

internal HttpHeaderValueCollection(HeaderDescriptor descriptor, HttpHeaders store, T? specialValue,
Action<HttpHeaderValueCollection<T>, T>? validator)
{
Debug.Assert(descriptor.Name != null);
Debug.Assert(store != null);

_store = store;
_descriptor = descriptor;
_specialValue = specialValue;
_validator = validator;
}

public void Add(T item)
Expand Down Expand Up @@ -204,38 +166,20 @@ public override string ToString()
return _store.GetHeaderString(_descriptor);
}

internal void SetSpecialValue()
{
Debug.Assert(_specialValue != null,
"This method can only be used if the collection has a 'special value' set.");

if (!_store.ContainsParsedValue(_descriptor, _specialValue))
{
_store.AddParsedValue(_descriptor, _specialValue);
}
}

internal void RemoveSpecialValue()
{
Debug.Assert(_specialValue != null,
"This method can only be used if the collection has a 'special value' set.");

// We're not interested in the return value. It's OK if the "special value" wasn't in the store
// before calling RemoveParsedValue().
_store.RemoveParsedValue(_descriptor, _specialValue);
}

private void CheckValue(T item)
{
if (item == null)
{
throw new ArgumentNullException(nameof(item));
}

// If this instance has a custom validator for validating arguments, call it now.
if (_validator != null)
if (_descriptor.Parser == GenericHeaderParser.TokenListParser)
{
_validator(this, item);
// The collection expects valid HTTP tokens, which are typed as string.
// Unlike other parsed values (which are always valid by construction),
// we can't assume the provided string is a valid token. So validate it before we use it.
Debug.Assert(typeof(T) == typeof(string));
HeaderUtilities.CheckValidToken((string)(object)item, nameof(item));
}
}

Expand Down
33 changes: 14 additions & 19 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpRequestHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,26 +51,17 @@ public AuthenticationHeaderValue? Authorization
set { SetOrRemoveParsedValue(KnownHeaders.Authorization.Descriptor, value); }
}

public HttpHeaderValueCollection<NameValueWithParametersHeaderValue> Expect
{
get { return ExpectCore; }
}

public bool? ExpectContinue
{
get
{
// ExpectCore will force the collection into existence, so avoid accessing it if possible.
if (_expectContinueSet || ContainsParsedValue(KnownHeaders.Expect.Descriptor, HeaderUtilities.ExpectContinue))
if (ContainsParsedValue(KnownHeaders.Expect.Descriptor, HeaderUtilities.ExpectContinue))
{
if (ExpectCore.IsSpecialValueSet)
{
return true;
}
if (_expectContinueSet)
{
return false;
}
return true;
}
if (_expectContinueSet)
{
return false;
}

return null;
Expand All @@ -80,12 +71,16 @@ public bool? ExpectContinue
if (value == true)
{
_expectContinueSet = true;
ExpectCore.SetSpecialValue();
if (!ContainsParsedValue(KnownHeaders.Expect.Descriptor, HeaderUtilities.ExpectContinue))
{
AddParsedValue(KnownHeaders.Expect.Descriptor, HeaderUtilities.ExpectContinue);
}
}
else
{
_expectContinueSet = value != null;
ExpectCore.RemoveSpecialValue();
// We intentionally ignore the return value. It's OK if "100-continue" wasn't in the store.
RemoveParsedValue(KnownHeaders.Expect.Descriptor, HeaderUtilities.ExpectContinue);
}
}
}
Expand Down Expand Up @@ -189,8 +184,8 @@ public Uri? Referrer
public HttpHeaderValueCollection<ProductInfoHeaderValue> UserAgent =>
GetSpecializedCollection(UserAgentSlot, static thisRef => new HttpHeaderValueCollection<ProductInfoHeaderValue>(KnownHeaders.UserAgent.Descriptor, thisRef));

private HttpHeaderValueCollection<NameValueWithParametersHeaderValue> ExpectCore =>
_expect ??= new HttpHeaderValueCollection<NameValueWithParametersHeaderValue>(KnownHeaders.Expect.Descriptor, this, HeaderUtilities.ExpectContinue);
public HttpHeaderValueCollection<NameValueWithParametersHeaderValue> Expect =>
_expect ??= new HttpHeaderValueCollection<NameValueWithParametersHeaderValue>(KnownHeaders.Expect.Descriptor, this);

#endregion

Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpResponseHeaders.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ private T GetSpecializedCollection<T>(int slot, Func<HttpResponseHeaders, T> cre
}

public HttpHeaderValueCollection<string> AcceptRanges =>
GetSpecializedCollection(AcceptRangesSlot, static thisRef => new HttpHeaderValueCollection<string>(KnownHeaders.AcceptRanges.Descriptor, thisRef, HeaderUtilities.TokenValidator));
GetSpecializedCollection(AcceptRangesSlot, static thisRef => new HttpHeaderValueCollection<string>(KnownHeaders.AcceptRanges.Descriptor, thisRef));

public TimeSpan? Age
{
Expand Down Expand Up @@ -68,7 +68,7 @@ public RetryConditionHeaderValue? RetryAfter
GetSpecializedCollection(ServerSlot, static thisRef => new HttpHeaderValueCollection<ProductInfoHeaderValue>(KnownHeaders.Server.Descriptor, thisRef));

public HttpHeaderValueCollection<string> Vary =>
GetSpecializedCollection(VarySlot, static thisRef => new HttpHeaderValueCollection<string>(KnownHeaders.Vary.Descriptor, thisRef, HeaderUtilities.TokenValidator));
GetSpecializedCollection(VarySlot, static thisRef => new HttpHeaderValueCollection<string>(KnownHeaders.Vary.Descriptor, thisRef));

public HttpHeaderValueCollection<AuthenticationHeaderValue> WwwAuthenticate =>
GetSpecializedCollection(WwwAuthenticateSlot, static thisRef => new HttpHeaderValueCollection<AuthenticationHeaderValue>(KnownHeaders.WWWAuthenticate.Descriptor, thisRef));
Expand Down
Loading