From 4132f4b5bac6fe5ac040b8ee707ce330dd12cc9b Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Sat, 18 Mar 2023 17:42:32 +0100 Subject: [PATCH 1/2] Reduce the size of some Http header values --- .../Http/Headers/CacheControlHeaderValue.cs | 325 ++++++++---------- .../Http/Headers/ContentRangeHeaderValue.cs | 84 ++--- .../Http/Headers/RangeConditionHeaderValue.cs | 55 +-- .../Net/Http/Headers/RangeItemHeaderValue.cs | 64 ++-- .../Http/Headers/RetryConditionHeaderValue.cs | 65 +--- .../Headers/StringWithQualityHeaderValue.cs | 78 ++--- .../Net/Http/Headers/WarningHeaderValue.cs | 85 ++--- 7 files changed, 267 insertions(+), 489 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs index 918c2bd22dcee..4671aec19393a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs @@ -5,7 +5,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.IO; using System.Text; namespace System.Net.Http.Headers @@ -27,103 +26,128 @@ public class CacheControlHeaderValue : ICloneable private static readonly GenericHeaderParser s_nameValueListParser = GenericHeaderParser.MultipleValueNameValueParser; - private bool _noCache; + [Flags] + private enum Flags : int + { + None = 0, + MaxAgeHasValue = 1 << 0, + SharedMaxAgeHasValue = 1 << 1, + MaxStaleLimitHasValue = 1 << 2, + MinFreshHasValue = 1 << 3, + NoCache = 1 << 4, + NoStore = 1 << 5, + MaxStale = 1 << 6, + NoTransform = 1 << 7, + OnlyIfCached = 1 << 8, + Public = 1 << 9, + Private = 1 << 10, + MustRevalidate = 1 << 11, + ProxyRevalidate = 1 << 12, + } + + private Flags _flags; private TokenObjectCollection? _noCacheHeaders; - private bool _noStore; - private TimeSpan? _maxAge; - private TimeSpan? _sharedMaxAge; - private bool _maxStale; - private TimeSpan? _maxStaleLimit; - private TimeSpan? _minFresh; - private bool _noTransform; - private bool _onlyIfCached; - private bool _publicField; - private bool _privateField; + private TimeSpan _maxAge; + private TimeSpan _sharedMaxAge; + private TimeSpan _maxStaleLimit; + private TimeSpan _minFresh; private TokenObjectCollection? _privateHeaders; - private bool _mustRevalidate; - private bool _proxyRevalidate; private UnvalidatedObjectCollection? _extensions; + private void SetTimeSpan(ref TimeSpan fieldRef, Flags flag, TimeSpan? value) + { + if (value is null) + { + fieldRef = default; + _flags &= ~flag; + } + else + { + fieldRef = value.Value; + _flags |= flag; + } + } + public bool NoCache { - get { return _noCache; } - set { _noCache = value; } + get => (_flags & Flags.NoCache) != 0; + set => _flags = value ? (_flags | Flags.NoCache) : (_flags & ~Flags.NoCache); } public ICollection NoCacheHeaders => _noCacheHeaders ??= new TokenObjectCollection(); public bool NoStore { - get { return _noStore; } - set { _noStore = value; } + get => (_flags & Flags.NoStore) != 0; + set => _flags = value ? (_flags | Flags.NoStore) : (_flags & ~Flags.NoStore); } public TimeSpan? MaxAge { - get { return _maxAge; } - set { _maxAge = value; } + get => (_flags & Flags.MaxAgeHasValue) == 0 ? null : _maxAge; + set => SetTimeSpan(ref _maxAge, Flags.MaxAgeHasValue, value); } public TimeSpan? SharedMaxAge { - get { return _sharedMaxAge; } - set { _sharedMaxAge = value; } + get => (_flags & Flags.SharedMaxAgeHasValue) == 0 ? null : _sharedMaxAge; + set => SetTimeSpan(ref _sharedMaxAge, Flags.SharedMaxAgeHasValue, value); } public bool MaxStale { - get { return _maxStale; } - set { _maxStale = value; } + get => (_flags & Flags.MaxStale) != 0; + set => _flags = value ? (_flags | Flags.MaxStale) : (_flags & ~Flags.MaxStale); } public TimeSpan? MaxStaleLimit { - get { return _maxStaleLimit; } - set { _maxStaleLimit = value; } + get => (_flags & Flags.MaxStaleLimitHasValue) == 0 ? null : _maxStaleLimit; + set => SetTimeSpan(ref _maxStaleLimit, Flags.MaxStaleLimitHasValue, value); } public TimeSpan? MinFresh { - get { return _minFresh; } - set { _minFresh = value; } + get => (_flags & Flags.MinFreshHasValue) == 0 ? null : _minFresh; + set => SetTimeSpan(ref _minFresh, Flags.MinFreshHasValue, value); } public bool NoTransform { - get { return _noTransform; } - set { _noTransform = value; } + get => (_flags & Flags.NoTransform) != 0; + set => _flags = value ? (_flags | Flags.NoTransform) : (_flags & ~Flags.NoTransform); } public bool OnlyIfCached { - get { return _onlyIfCached; } - set { _onlyIfCached = value; } + get => (_flags & Flags.OnlyIfCached) != 0; + set => _flags = value ? (_flags | Flags.OnlyIfCached) : (_flags & ~Flags.OnlyIfCached); } public bool Public { - get { return _publicField; } - set { _publicField = value; } + get => (_flags & Flags.Public) != 0; + set => _flags = value ? (_flags | Flags.Public) : (_flags & ~Flags.Public); } public bool Private { - get { return _privateField; } - set { _privateField = value; } + get => (_flags & Flags.Private) != 0; + set => _flags = value ? (_flags | Flags.Private) : (_flags & ~Flags.Private); } public ICollection PrivateHeaders => _privateHeaders ??= new TokenObjectCollection(); public bool MustRevalidate { - get { return _mustRevalidate; } - set { _mustRevalidate = value; } + get => (_flags & Flags.MustRevalidate) != 0; + set => _flags = value ? (_flags | Flags.MustRevalidate) : (_flags & ~Flags.MustRevalidate); } public bool ProxyRevalidate { - get { return _proxyRevalidate; } - set { _proxyRevalidate = value; } + get => (_flags & Flags.ProxyRevalidate) != 0; + set => _flags = value ? (_flags | Flags.ProxyRevalidate) : (_flags & ~Flags.ProxyRevalidate); } public ICollection Extensions => _extensions ??= new UnvalidatedObjectCollection(); @@ -136,23 +160,15 @@ private CacheControlHeaderValue(CacheControlHeaderValue source) { Debug.Assert(source != null); - _noCache = source._noCache; - _noStore = source._noStore; + _flags = source._flags; _maxAge = source._maxAge; _sharedMaxAge = source._sharedMaxAge; - _maxStale = source._maxStale; _maxStaleLimit = source._maxStaleLimit; _minFresh = source._minFresh; - _noTransform = source._noTransform; - _onlyIfCached = source._onlyIfCached; - _publicField = source._publicField; - _privateField = source._privateField; - _mustRevalidate = source._mustRevalidate; - _proxyRevalidate = source._proxyRevalidate; if (source._noCacheHeaders != null) { - foreach (var noCacheHeader in source._noCacheHeaders) + foreach (string noCacheHeader in source._noCacheHeaders) { NoCacheHeaders.Add(noCacheHeader); } @@ -160,7 +176,7 @@ private CacheControlHeaderValue(CacheControlHeaderValue source) if (source._privateHeaders != null) { - foreach (var privateHeader in source._privateHeaders) + foreach (string privateHeader in source._privateHeaders) { PrivateHeaders.Add(privateHeader); } @@ -173,14 +189,14 @@ public override string ToString() { StringBuilder sb = StringBuilderCache.Acquire(); - AppendValueIfRequired(sb, _noStore, noStoreString); - AppendValueIfRequired(sb, _noTransform, noTransformString); - AppendValueIfRequired(sb, _onlyIfCached, onlyIfCachedString); - AppendValueIfRequired(sb, _publicField, publicString); - AppendValueIfRequired(sb, _mustRevalidate, mustRevalidateString); - AppendValueIfRequired(sb, _proxyRevalidate, proxyRevalidateString); + AppendValueIfRequired(sb, NoStore, noStoreString); + AppendValueIfRequired(sb, NoTransform, noTransformString); + AppendValueIfRequired(sb, OnlyIfCached, onlyIfCachedString); + AppendValueIfRequired(sb, Public, publicString); + AppendValueIfRequired(sb, MustRevalidate, mustRevalidateString); + AppendValueIfRequired(sb, ProxyRevalidate, proxyRevalidateString); - if (_noCache) + if (NoCache) { AppendValueWithSeparatorIfRequired(sb, noCacheString); if ((_noCacheHeaders != null) && (_noCacheHeaders.Count > 0)) @@ -191,11 +207,11 @@ public override string ToString() } } - if (_maxAge.HasValue) + if ((_flags & Flags.MaxAgeHasValue) != 0) { AppendValueWithSeparatorIfRequired(sb, maxAgeString); sb.Append('='); - int maxAge = (int)_maxAge.GetValueOrDefault().TotalSeconds; + int maxAge = (int)_maxAge.TotalSeconds; if (maxAge >= 0) { sb.Append(maxAge); @@ -208,11 +224,11 @@ public override string ToString() } } - if (_sharedMaxAge.HasValue) + if ((_flags & Flags.SharedMaxAgeHasValue) != 0) { AppendValueWithSeparatorIfRequired(sb, sharedMaxAgeString); sb.Append('='); - int sharedMaxAge = (int)_sharedMaxAge.GetValueOrDefault().TotalSeconds; + int sharedMaxAge = (int)_sharedMaxAge.TotalSeconds; if (sharedMaxAge >= 0) { sb.Append(sharedMaxAge); @@ -225,13 +241,13 @@ public override string ToString() } } - if (_maxStale) + if (MaxStale) { AppendValueWithSeparatorIfRequired(sb, maxStaleString); - if (_maxStaleLimit.HasValue) + if ((_flags & Flags.MaxStaleLimitHasValue) != 0) { sb.Append('='); - int maxStaleLimit = (int)_maxStaleLimit.GetValueOrDefault().TotalSeconds; + int maxStaleLimit = (int)_maxStaleLimit.TotalSeconds; if (maxStaleLimit >= 0) { sb.Append(maxStaleLimit); @@ -245,11 +261,11 @@ public override string ToString() } } - if (_minFresh.HasValue) + if ((_flags & Flags.MinFreshHasValue) != 0) { AppendValueWithSeparatorIfRequired(sb, minFreshString); sb.Append('='); - int minFresh = (int)_minFresh.GetValueOrDefault().TotalSeconds; + int minFresh = (int)_minFresh.TotalSeconds; if (minFresh >= 0) { sb.Append(minFresh); @@ -262,7 +278,7 @@ public override string ToString() } } - if (_privateField) + if (Private) { AppendValueWithSeparatorIfRequired(sb, privateString); if ((_privateHeaders != null) && (_privateHeaders.Count > 0)) @@ -278,87 +294,49 @@ public override string ToString() return StringBuilderCache.GetStringAndRelease(sb); } - public override bool Equals([NotNullWhen(true)] object? obj) - { - CacheControlHeaderValue? other = obj as CacheControlHeaderValue; - - if (other == null) - { - return false; - } - - if ((_noCache != other._noCache) || (_noStore != other._noStore) || (_maxAge != other._maxAge) || - (_sharedMaxAge != other._sharedMaxAge) || (_maxStale != other._maxStale) || - (_maxStaleLimit != other._maxStaleLimit) || (_minFresh != other._minFresh) || - (_noTransform != other._noTransform) || (_onlyIfCached != other._onlyIfCached) || - (_publicField != other._publicField) || (_privateField != other._privateField) || - (_mustRevalidate != other._mustRevalidate) || (_proxyRevalidate != other._proxyRevalidate)) - { - return false; - } - - if (!HeaderUtilities.AreEqualCollections(_noCacheHeaders, other._noCacheHeaders, - StringComparer.OrdinalIgnoreCase)) - { - return false; - } - - if (!HeaderUtilities.AreEqualCollections(_privateHeaders, other._privateHeaders, - StringComparer.OrdinalIgnoreCase)) - { - return false; - } - - if (!HeaderUtilities.AreEqualCollections(_extensions, other._extensions)) - { - return false; - } - - return true; - } + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is CacheControlHeaderValue other && + _flags == other._flags && + _maxAge == other._maxAge && + _sharedMaxAge == other._sharedMaxAge && + _minFresh == other._minFresh && + HeaderUtilities.AreEqualCollections(_noCacheHeaders, other._noCacheHeaders, StringComparer.OrdinalIgnoreCase) && + HeaderUtilities.AreEqualCollections(_privateHeaders, other._privateHeaders, StringComparer.OrdinalIgnoreCase) && + HeaderUtilities.AreEqualCollections(_extensions, other._extensions); public override int GetHashCode() { - // Use a different bit for bool fields: bool.GetHashCode() will return 0 (false) or 1 (true). So we would - // end up having the same hash code for e.g. two instances where one has only noCache set and the other - // only noStore. - int result = _noCache.GetHashCode() ^ (_noStore.GetHashCode() << 1) ^ (_maxStale.GetHashCode() << 2) ^ - (_noTransform.GetHashCode() << 3) ^ (_onlyIfCached.GetHashCode() << 4) ^ - (_publicField.GetHashCode() << 5) ^ (_privateField.GetHashCode() << 6) ^ - (_mustRevalidate.GetHashCode() << 7) ^ (_proxyRevalidate.GetHashCode() << 8); - - // XOR the hashcode of timespan values with different numbers to make sure two instances with the same - // timespan set on different fields result in different hashcodes. - result = result ^ (_maxAge.HasValue ? _maxAge.Value.GetHashCode() ^ 1 : 0) ^ - (_sharedMaxAge.HasValue ? _sharedMaxAge.Value.GetHashCode() ^ 2 : 0) ^ - (_maxStaleLimit.HasValue ? _maxStaleLimit.Value.GetHashCode() ^ 4 : 0) ^ - (_minFresh.HasValue ? _minFresh.Value.GetHashCode() ^ 8 : 0); + HashCode hashcode = default; + + hashcode.Add(_flags); + hashcode.Add(_maxAge); + hashcode.Add(_sharedMaxAge); + hashcode.Add(_maxStaleLimit); + hashcode.Add(_minFresh); if ((_noCacheHeaders != null) && (_noCacheHeaders.Count > 0)) { - foreach (var noCacheHeader in _noCacheHeaders) + int unorderedHash = 0; + foreach (string noCacheHeader in _noCacheHeaders) { - result ^= StringComparer.OrdinalIgnoreCase.GetHashCode(noCacheHeader); + unorderedHash ^= StringComparer.OrdinalIgnoreCase.GetHashCode(noCacheHeader); } + hashcode.Add(unorderedHash); } if ((_privateHeaders != null) && (_privateHeaders.Count > 0)) { - foreach (var privateHeader in _privateHeaders) + int unorderedHash = 0; + foreach (string privateHeader in _privateHeaders) { - result ^= StringComparer.OrdinalIgnoreCase.GetHashCode(privateHeader); + unorderedHash ^= StringComparer.OrdinalIgnoreCase.GetHashCode(privateHeader); } + hashcode.Add(unorderedHash); } - if ((_extensions != null) && (_extensions.Count > 0)) - { - foreach (var extension in _extensions) - { - result ^= extension.GetHashCode(); - } - } + hashcode.Add(NameValueHeaderValue.GetHashCode(_extensions)); - return result; + return hashcode.ToHashCode(); } public static CacheControlHeaderValue Parse(string? input) @@ -395,11 +373,10 @@ internal static int GetCacheControlLength(string? input, int startIndex, CacheCo // Cache-Control header consists of a list of name/value pairs, where the value is optional. So use an // instance of NameValueHeaderParser to parse the string. int current = startIndex; - object? nameValue; List nameValueList = new List(); while (current < input.Length) { - if (!s_nameValueListParser.TryParseValue(input, null, ref current, out nameValue)) + if (!s_nameValueListParser.TryParseValue(input, null, ref current, out object? nameValue)) { return 0; } @@ -433,74 +410,87 @@ internal static int GetCacheControlLength(string? input, int startIndex, CacheCo return input.Length - startIndex; } - private static bool TrySetCacheControlValues(CacheControlHeaderValue cc, - List nameValueList) + private static bool TrySetCacheControlValues(CacheControlHeaderValue cc, List nameValueList) { foreach (NameValueHeaderValue nameValue in nameValueList) { - bool success = true; string name = nameValue.Name.ToLowerInvariant(); + string? value = nameValue.Value; + + Flags flagsToSet = Flags.None; + bool success = value is null; switch (name) { case noCacheString: - success = TrySetOptionalTokenList(nameValue, ref cc._noCache, ref cc._noCacheHeaders); + flagsToSet = Flags.NoCache; + success = TrySetOptionalTokenList(nameValue, ref cc._noCacheHeaders); break; case noStoreString: - success = TrySetTokenOnlyValue(nameValue, ref cc._noStore); + flagsToSet = Flags.NoStore; break; case maxAgeString: - success = TrySetTimeSpan(nameValue, ref cc._maxAge); + flagsToSet = Flags.MaxAgeHasValue; + success = TrySetTimeSpan(value, ref cc._maxAge); break; case maxStaleString: - success = ((nameValue.Value == null) || TrySetTimeSpan(nameValue, ref cc._maxStaleLimit)); - if (success) + flagsToSet = Flags.MaxStale; + if (TrySetTimeSpan(value, ref cc._maxStaleLimit)) { - cc._maxStale = true; + success = true; + flagsToSet = Flags.MaxStale | Flags.MaxStaleLimitHasValue; } break; case minFreshString: - success = TrySetTimeSpan(nameValue, ref cc._minFresh); + flagsToSet = Flags.MinFreshHasValue; + success = TrySetTimeSpan(value, ref cc._minFresh); break; case noTransformString: - success = TrySetTokenOnlyValue(nameValue, ref cc._noTransform); + flagsToSet = Flags.NoTransform; break; case onlyIfCachedString: - success = TrySetTokenOnlyValue(nameValue, ref cc._onlyIfCached); + flagsToSet = Flags.OnlyIfCached; break; case publicString: - success = TrySetTokenOnlyValue(nameValue, ref cc._publicField); + flagsToSet = Flags.Public; break; case privateString: - success = TrySetOptionalTokenList(nameValue, ref cc._privateField, ref cc._privateHeaders); + flagsToSet = Flags.Private; + success = TrySetOptionalTokenList(nameValue, ref cc._privateHeaders); break; case mustRevalidateString: - success = TrySetTokenOnlyValue(nameValue, ref cc._mustRevalidate); + flagsToSet = Flags.MustRevalidate; break; case proxyRevalidateString: - success = TrySetTokenOnlyValue(nameValue, ref cc._proxyRevalidate); + flagsToSet = Flags.ProxyRevalidate; break; case sharedMaxAgeString: - success = TrySetTimeSpan(nameValue, ref cc._sharedMaxAge); + flagsToSet = Flags.SharedMaxAgeHasValue; + success = TrySetTimeSpan(value, ref cc._sharedMaxAge); break; default: - cc.Extensions.Add(nameValue); // success is always true + success = true; + cc.Extensions.Add(nameValue); break; } - if (!success) + if (success) + { + cc._flags |= flagsToSet; + } + else { return false; } @@ -509,25 +499,12 @@ private static bool TrySetCacheControlValues(CacheControlHeaderValue cc, return true; } - private static bool TrySetTokenOnlyValue(NameValueHeaderValue nameValue, ref bool boolField) - { - if (nameValue.Value != null) - { - return false; - } - - boolField = true; - return true; - } - - private static bool TrySetOptionalTokenList(NameValueHeaderValue nameValue, ref bool boolField, - ref TokenObjectCollection? destination) + private static bool TrySetOptionalTokenList(NameValueHeaderValue nameValue, ref TokenObjectCollection? destination) { Debug.Assert(nameValue != null); if (nameValue.Value == null) { - boolField = true; return true; } @@ -571,30 +548,20 @@ private static bool TrySetOptionalTokenList(NameValueHeaderValue nameValue, ref // After parsing a valid token list, we expect to have at least one value if ((destination != null) && (destination.Count > originalValueCount)) { - boolField = true; return true; } return false; } - private static bool TrySetTimeSpan(NameValueHeaderValue nameValue, ref TimeSpan? timeSpan) + private static bool TrySetTimeSpan(string? value, ref TimeSpan timeSpan) { - Debug.Assert(nameValue != null); - - if (nameValue.Value == null) - { - return false; - } - - int seconds; - if (!HeaderUtilities.TryParseInt32(nameValue.Value, out seconds)) + if (value is null || !HeaderUtilities.TryParseInt32(value, out int seconds)) { return false; } timeSpan = new TimeSpan(0, 0, seconds); - return true; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentRangeHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentRangeHeaderValue.cs index 80ba175085767..aa08f5e507e03 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentRangeHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentRangeHeaderValue.cs @@ -3,8 +3,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Globalization; -using System.IO; using System.Text; namespace System.Net.Http.Headers @@ -12,9 +10,9 @@ namespace System.Net.Http.Headers public class ContentRangeHeaderValue : ICloneable { private string _unit = null!; - private long? _from; - private long? _to; - private long? _length; + private long _from; + private long _to; + private long _length; public string Unit { @@ -26,30 +24,15 @@ public string Unit } } - public long? From - { - get { return _from; } - } + public long? From => HasRange ? _from : null; - public long? To - { - get { return _to; } - } + public long? To => HasRange ? _to : null; - public long? Length - { - get { return _length; } - } + public long? Length => HasLength ? _length : null; - public bool HasLength // e.g. "Content-Range: bytes 12-34/*" - { - get { return _length != null; } - } + public bool HasLength => _length >= 0; // e.g. "Content-Range: bytes 12-34/*" - public bool HasRange // e.g. "Content-Range: bytes */1234" - { - get { return _from != null; } - } + public bool HasRange => _from >= 0; // e.g. "Content-Range: bytes */1234" public ContentRangeHeaderValue(long from, long to, long length) { @@ -75,6 +58,7 @@ public ContentRangeHeaderValue(long length) _length = length; _unit = HeaderUtilities.BytesUnit; + _from = -1; } public ContentRangeHeaderValue(long from, long to) @@ -88,10 +72,13 @@ public ContentRangeHeaderValue(long from, long to) _from = from; _to = to; _unit = HeaderUtilities.BytesUnit; + _length = -1; } private ContentRangeHeaderValue() { + _from = -1; + _length = -1; } private ContentRangeHeaderValue(ContentRangeHeaderValue source) @@ -104,35 +91,19 @@ private ContentRangeHeaderValue(ContentRangeHeaderValue source) _unit = source._unit; } - public override bool Equals([NotNullWhen(true)] object? obj) - { - ContentRangeHeaderValue? other = obj as ContentRangeHeaderValue; - - if (other == null) - { - return false; - } - - return ((_from == other._from) && (_to == other._to) && (_length == other._length) && - string.Equals(_unit, other._unit, StringComparison.OrdinalIgnoreCase)); - } - - public override int GetHashCode() - { - int result = StringComparer.OrdinalIgnoreCase.GetHashCode(_unit); - - if (HasRange) - { - result = result ^ _from.GetHashCode() ^ _to.GetHashCode(); - } - - if (HasLength) - { - result ^= _length.GetHashCode(); - } + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is ContentRangeHeaderValue other && + _from == other._from && + _to == other._to && + _length == other._length && + string.Equals(_unit, other._unit, StringComparison.OrdinalIgnoreCase); - return result; - } + public override int GetHashCode() => + HashCode.Combine( + StringComparer.OrdinalIgnoreCase.GetHashCode(_unit), + _from, + _to, + _length); public override string ToString() { @@ -142,10 +113,9 @@ public override string ToString() if (HasRange) { - sb.AppendSpanFormattable(_from!.Value); + sb.AppendSpanFormattable(_from); sb.Append('-'); - Debug.Assert(_to.HasValue); - sb.AppendSpanFormattable(_to.Value); + sb.AppendSpanFormattable(_to); } else { @@ -155,7 +125,7 @@ public override string ToString() sb.Append('/'); if (HasLength) { - sb.AppendSpanFormattable(_length!.Value); + sb.AppendSpanFormattable(_length); } else { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs index 78255fb825daf..e6804b251cf6e 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs @@ -8,18 +8,12 @@ namespace System.Net.Http.Headers { public class RangeConditionHeaderValue : ICloneable { - private readonly DateTimeOffset? _date; + private readonly DateTimeOffset _date; private readonly EntityTagHeaderValue? _entityTag; - public DateTimeOffset? Date - { - get { return _date; } - } + public DateTimeOffset? Date => _entityTag is null ? _date : null; - public EntityTagHeaderValue? EntityTag - { - get { return _entityTag; } - } + public EntityTagHeaderValue? EntityTag => _entityTag; public RangeConditionHeaderValue(DateTimeOffset date) { @@ -46,45 +40,14 @@ private RangeConditionHeaderValue(RangeConditionHeaderValue source) _date = source._date; } - public override string ToString() - { - if (_entityTag == null) - { - Debug.Assert(_date != null); - return _date.GetValueOrDefault().ToString("r"); - } - - return _entityTag.ToString(); - } + public override string ToString() => _entityTag?.ToString() ?? _date.ToString("r"); - public override bool Equals([NotNullWhen(true)] object? obj) - { - RangeConditionHeaderValue? other = obj as RangeConditionHeaderValue; + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is RangeConditionHeaderValue other && + (_entityTag is null ? other._entityTag is null : _entityTag.Equals(other._entityTag)) && + _date == other._date; - if (other == null) - { - return false; - } - - if (_entityTag == null) - { - Debug.Assert(_date != null); - return (other._date != null) && (_date.Value == other._date.Value); - } - - return _entityTag.Equals(other._entityTag); - } - - public override int GetHashCode() - { - if (_entityTag == null) - { - Debug.Assert(_date != null); - return _date.Value.GetHashCode(); - } - - return _entityTag.GetHashCode(); - } + public override int GetHashCode() => _entityTag?.GetHashCode() ?? _date.GetHashCode(); public static RangeConditionHeaderValue Parse(string input) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs index 2fbb4e677b06c..f74a819631515 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs @@ -10,18 +10,13 @@ namespace System.Net.Http.Headers { public class RangeItemHeaderValue : ICloneable { - private readonly long? _from; - private readonly long? _to; + // Set to -1 if not set. + private readonly long _from; + private readonly long _to; - public long? From - { - get { return _from; } - } + public long? From => _from >= 0 ? _from : null; - public long? To - { - get { return _to; } - } + public long? To => _to >= 0 ? _to : null; public RangeItemHeaderValue(long? from, long? to) { @@ -42,8 +37,8 @@ public RangeItemHeaderValue(long? from, long? to) ArgumentOutOfRangeException.ThrowIfGreaterThan(from.GetValueOrDefault(), to.GetValueOrDefault(), nameof(from)); } - _from = from; - _to = to; + _from = from ?? -1; + _to = to ?? -1; } internal RangeItemHeaderValue(RangeItemHeaderValue source) @@ -58,43 +53,26 @@ public override string ToString() { Span stackBuffer = stackalloc char[128]; - if (!_from.HasValue) + if (_from < 0) { - Debug.Assert(_to != null); - return string.Create(CultureInfo.InvariantCulture, stackBuffer, $"-{_to.Value}"); + return string.Create(CultureInfo.InvariantCulture, stackBuffer, $"-{_to}"); } - if (!_to.HasValue) + if (_to < 0) { - return string.Create(CultureInfo.InvariantCulture, stackBuffer, $"{_from.Value}-"); ; + return string.Create(CultureInfo.InvariantCulture, stackBuffer, $"{_from}-"); ; } - return string.Create(CultureInfo.InvariantCulture, stackBuffer, $"{_from.Value}-{_to.Value}"); + return string.Create(CultureInfo.InvariantCulture, stackBuffer, $"{_from}-{_to}"); } - public override bool Equals([NotNullWhen(true)] object? obj) - { - RangeItemHeaderValue? other = obj as RangeItemHeaderValue; + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is RangeItemHeaderValue other && + _from == other._from && + _to == other._to; - if (other == null) - { - return false; - } - return ((_from == other._from) && (_to == other._to)); - } - - public override int GetHashCode() - { - if (!_from.HasValue) - { - return _to.GetHashCode(); - } - else if (!_to.HasValue) - { - return _from.GetHashCode(); - } - return _from.GetHashCode() ^ _to.GetHashCode(); - } + public override int GetHashCode() => + HashCode.Combine(_from, _to); // Returns the length of a range list. E.g. "1-2, 3-4, 5-6" adds 3 ranges to 'rangeCollection'. Note that empty // list segments are allowed, e.g. ",1-2, , 3-4,,". @@ -118,10 +96,9 @@ internal static int GetRangeItemListLength(string? input, int startIndex, return 0; } - RangeItemHeaderValue? range; while (true) { - int rangeLength = GetRangeItemLength(input, current, out range); + int rangeLength = GetRangeItemLength(input, current, out RangeItemHeaderValue? range); if (rangeLength == 0) { @@ -227,8 +204,7 @@ internal static int GetRangeItemLength(string? input, int startIndex, out RangeI return 0; } - parsedValue = new RangeItemHeaderValue((fromLength == 0 ? (long?)null : (long?)from), - (toLength == 0 ? (long?)null : (long?)to)); + parsedValue = new RangeItemHeaderValue((fromLength == 0 ? null : from), (toLength == 0 ? null : to)); return current - startIndex; } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RetryConditionHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RetryConditionHeaderValue.cs index f1175f8414d26..db32ceea9a5a8 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RetryConditionHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RetryConditionHeaderValue.cs @@ -9,22 +9,20 @@ namespace System.Net.Http.Headers { public class RetryConditionHeaderValue : ICloneable { - private readonly DateTimeOffset? _date; - private readonly TimeSpan? _delta; + private const long DeltaNotSetTicksSentinel = long.MaxValue; - public DateTimeOffset? Date - { - get { return _date; } - } + // Only one of date and delta may be set. + private readonly DateTimeOffset _date; + private readonly TimeSpan _delta; - public TimeSpan? Delta - { - get { return _delta; } - } + public DateTimeOffset? Date => _delta.Ticks == DeltaNotSetTicksSentinel ? _date : null; + + public TimeSpan? Delta => _delta.Ticks == DeltaNotSetTicksSentinel ? null : _delta; public RetryConditionHeaderValue(DateTimeOffset date) { _date = date; + _delta = new TimeSpan(DeltaNotSetTicksSentinel); } public RetryConditionHeaderValue(TimeSpan delta) @@ -43,45 +41,18 @@ private RetryConditionHeaderValue(RetryConditionHeaderValue source) _date = source._date; } - public override string ToString() - { - if (_delta.HasValue) - { - return ((int)_delta.Value.TotalSeconds).ToString(NumberFormatInfo.InvariantInfo); - } - - Debug.Assert(_date != null); - return _date.GetValueOrDefault().ToString("r"); - } - - public override bool Equals([NotNullWhen(true)] object? obj) - { - RetryConditionHeaderValue? other = obj as RetryConditionHeaderValue; - - if (other == null) - { - return false; - } - - if (_delta.HasValue) - { - return (other._delta != null) && (_delta.Value == other._delta.Value); - } - - Debug.Assert(_date != null); - return (other._date != null) && (_date.Value == other._date.Value); - } + public override string ToString() => + _delta.Ticks != DeltaNotSetTicksSentinel + ? ((int)_delta.TotalSeconds).ToString(NumberFormatInfo.InvariantInfo) + : _date.ToString("r"); - public override int GetHashCode() - { - if (_delta == null) - { - Debug.Assert(_date != null); - return _date.Value.GetHashCode(); - } + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is RetryConditionHeaderValue other && + _delta == other._delta && + _date == other._date; - return _delta.Value.GetHashCode(); - } + public override int GetHashCode() => + HashCode.Combine(_delta, _date); public static RetryConditionHeaderValue Parse(string input) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/StringWithQualityHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/StringWithQualityHeaderValue.cs index 00ce20b43a7c0..d7da89b5cb0bb 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/StringWithQualityHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/StringWithQualityHeaderValue.cs @@ -9,24 +9,21 @@ namespace System.Net.Http.Headers { public class StringWithQualityHeaderValue : ICloneable { + private const double NotSetSentinel = double.PositiveInfinity; + private readonly string _value; - private readonly double? _quality; + private readonly double _quality; - public string Value - { - get { return _value; } - } + public string Value => _value; - public double? Quality - { - get { return _quality; } - } + public double? Quality => _quality == NotSetSentinel ? null : _quality; public StringWithQualityHeaderValue(string value) { HeaderUtilities.CheckValidToken(value, nameof(value)); _value = value; + _quality = NotSetSentinel; } public StringWithQualityHeaderValue(string value, double quality) @@ -48,54 +45,21 @@ private StringWithQualityHeaderValue(StringWithQualityHeaderValue source) _quality = source._quality; } - public override string ToString() - { - if (_quality.HasValue) - { - return string.Create(CultureInfo.InvariantCulture, stackalloc char[128], $"{_value}; q={_quality.Value:0.0##}"); - } - - return _value; - } - - public override bool Equals([NotNullWhen(true)] object? obj) - { - StringWithQualityHeaderValue? other = obj as StringWithQualityHeaderValue; - - if (other == null) - { - return false; - } - - if (!string.Equals(_value, other._value, StringComparison.OrdinalIgnoreCase)) - { - return false; - } - - if (_quality.HasValue) - { - // Note that we don't consider double.Epsilon here. We really consider two values equal if they're - // actually equal. This makes sure that we also get the same hashcode for two values considered equal - // by Equals(). - return other._quality.HasValue && (_quality.Value == other._quality.Value); - } - - // If we don't have a quality value, then 'other' must also have no quality assigned in order to be - // considered equal. - return !other._quality.HasValue; - } - - public override int GetHashCode() - { - int result = StringComparer.OrdinalIgnoreCase.GetHashCode(_value); - - if (_quality.HasValue) - { - result ^= _quality.Value.GetHashCode(); - } - - return result; - } + public override string ToString() => + _quality == NotSetSentinel + ? _value + : string.Create(CultureInfo.InvariantCulture, stackalloc char[128], $"{_value}; q={_quality:0.0##}"); + + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is StringWithQualityHeaderValue other && + string.Equals(_value, other._value, StringComparison.OrdinalIgnoreCase) && + // Note that we don't consider double.Epsilon here. We really consider two values equal if they're + // actually equal. This makes sure that we also get the same hashcode for two values considered equal + // by Equals(). + _quality == other._quality; + + public override int GetHashCode() => + HashCode.Combine(StringComparer.OrdinalIgnoreCase.GetHashCode(_value), _quality); public static StringWithQualityHeaderValue Parse(string input) { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/WarningHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/WarningHeaderValue.cs index 6599ffd82fa5a..586c0b68fc549 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/WarningHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/WarningHeaderValue.cs @@ -4,7 +4,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; -using System.IO; using System.Text; namespace System.Net.Http.Headers @@ -14,27 +13,16 @@ public class WarningHeaderValue : ICloneable private readonly int _code; private readonly string _agent; private readonly string _text; - private readonly DateTimeOffset? _date; + private readonly DateTimeOffset _date; + private readonly bool _dateHasValue; - public int Code - { - get { return _code; } - } + public int Code => _code; - public string Agent - { - get { return _agent; } - } + public string Agent => _agent; - public string Text - { - get { return _text; } - } + public string Text => _text; - public DateTimeOffset? Date - { - get { return _date; } - } + public DateTimeOffset? Date => _dateHasValue ? _date : null; public WarningHeaderValue(int code, string agent, string text) { @@ -57,6 +45,7 @@ public WarningHeaderValue(int code, string agent, string text, DateTimeOffset da _agent = agent; _text = text; _date = date; + _dateHasValue = true; } private WarningHeaderValue(WarningHeaderValue source) @@ -67,6 +56,7 @@ private WarningHeaderValue(WarningHeaderValue source) _agent = source._agent; _text = source._text; _date = source._date; + _dateHasValue = source._dateHasValue; } public override string ToString() @@ -81,56 +71,33 @@ public override string ToString() sb.Append(' '); sb.Append(_text); - if (_date.HasValue) + if (_dateHasValue) { sb.Append(" \""); - sb.AppendSpanFormattable(_date.Value, "r"); + sb.AppendSpanFormattable(_date, "r"); sb.Append('\"'); } return sb.ToString(); } - public override bool Equals([NotNullWhen(true)] object? obj) - { - WarningHeaderValue? other = obj as WarningHeaderValue; - - if (other == null) - { - return false; - } - + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is WarningHeaderValue other && + _code == other._code && // 'agent' is a host/token, i.e. use case-insensitive comparison. Use case-sensitive comparison for 'text' // since it is a quoted string. - if ((_code != other._code) || (!string.Equals(_agent, other._agent, StringComparison.OrdinalIgnoreCase)) || - (!string.Equals(_text, other._text, StringComparison.Ordinal))) - { - return false; - } - - // We have a date set. Verify 'other' has also a date that matches our value. - if (_date.HasValue) - { - return other._date.HasValue && (_date.Value == other._date.Value); - } - - // We don't have a date. If 'other' has a date, we're not equal. - return !other._date.HasValue; - } - - public override int GetHashCode() - { - int result = _code.GetHashCode() ^ - StringComparer.OrdinalIgnoreCase.GetHashCode(_agent) ^ - _text.GetHashCode(); - - if (_date.HasValue) - { - result ^= _date.Value.GetHashCode(); - } - - return result; - } + string.Equals(_agent, other._agent, StringComparison.OrdinalIgnoreCase) && + string.Equals(_text, other._text, StringComparison.Ordinal) && + _dateHasValue == other._dateHasValue && + _date == other._date; + + public override int GetHashCode() => + HashCode.Combine( + _code, + StringComparer.OrdinalIgnoreCase.GetHashCode(_agent), + _text, + _dateHasValue, + _date); public static WarningHeaderValue Parse(string input) { @@ -324,7 +291,7 @@ private static void CheckAgent(string agent) // is a valid host. if (HttpRuleParser.GetHostLength(agent, 0, true) != agent.Length) { - throw new FormatException(SR.Format(System.Globalization.CultureInfo.InvariantCulture, SR.net_http_headers_invalid_value, agent)); + throw new FormatException(SR.Format(SR.net_http_headers_invalid_value, agent)); } } } From 082782966c47af4f6c32e2546efea6cdc281c797 Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Mon, 20 Mar 2023 06:55:18 +0100 Subject: [PATCH 2/2] PR feedback --- .../Http/Headers/CacheControlHeaderValue.cs | 95 +++++++++---------- .../Http/Headers/RangeConditionHeaderValue.cs | 1 + .../Headers/CacheControlHeaderValueTest.cs | 8 +- 3 files changed, 55 insertions(+), 49 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs index 4671aec19393a..3067cc68689b4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs @@ -5,7 +5,9 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; +using System.Runtime.CompilerServices; using System.Text; +using System.Threading; namespace System.Net.Http.Headers { @@ -56,22 +58,30 @@ private enum Flags : int private void SetTimeSpan(ref TimeSpan fieldRef, Flags flag, TimeSpan? value) { - if (value is null) + fieldRef = value.GetValueOrDefault(); + SetFlag(flag, value.HasValue); + } + + private void SetFlag(Flags flag, bool value) + { + Debug.Assert(sizeof(Flags) == sizeof(int)); + + // This type is not thread-safe, but we do a minimal amount of synchronization to ensure + // that concurrent modifications of different properties don't interfere with each other. + if (value) { - fieldRef = default; - _flags &= ~flag; + Interlocked.Or(ref Unsafe.As(ref _flags), (int)flag); } else { - fieldRef = value.Value; - _flags |= flag; + Interlocked.And(ref Unsafe.As(ref _flags), (int)~flag); } } public bool NoCache { get => (_flags & Flags.NoCache) != 0; - set => _flags = value ? (_flags | Flags.NoCache) : (_flags & ~Flags.NoCache); + set => SetFlag(Flags.NoCache, value); } public ICollection NoCacheHeaders => _noCacheHeaders ??= new TokenObjectCollection(); @@ -79,7 +89,7 @@ public bool NoCache public bool NoStore { get => (_flags & Flags.NoStore) != 0; - set => _flags = value ? (_flags | Flags.NoStore) : (_flags & ~Flags.NoStore); + set => SetFlag(Flags.NoStore, value); } public TimeSpan? MaxAge @@ -97,7 +107,7 @@ public TimeSpan? SharedMaxAge public bool MaxStale { get => (_flags & Flags.MaxStale) != 0; - set => _flags = value ? (_flags | Flags.MaxStale) : (_flags & ~Flags.MaxStale); + set => SetFlag(Flags.MaxStale, value); } public TimeSpan? MaxStaleLimit @@ -115,25 +125,25 @@ public TimeSpan? MinFresh public bool NoTransform { get => (_flags & Flags.NoTransform) != 0; - set => _flags = value ? (_flags | Flags.NoTransform) : (_flags & ~Flags.NoTransform); + set => SetFlag(Flags.NoTransform, value); } public bool OnlyIfCached { get => (_flags & Flags.OnlyIfCached) != 0; - set => _flags = value ? (_flags | Flags.OnlyIfCached) : (_flags & ~Flags.OnlyIfCached); + set => SetFlag(Flags.OnlyIfCached, value); } public bool Public { get => (_flags & Flags.Public) != 0; - set => _flags = value ? (_flags | Flags.Public) : (_flags & ~Flags.Public); + set => SetFlag(Flags.Public, value); } public bool Private { get => (_flags & Flags.Private) != 0; - set => _flags = value ? (_flags | Flags.Private) : (_flags & ~Flags.Private); + set => SetFlag(Flags.Private, value); } public ICollection PrivateHeaders => _privateHeaders ??= new TokenObjectCollection(); @@ -141,13 +151,13 @@ public bool Private public bool MustRevalidate { get => (_flags & Flags.MustRevalidate) != 0; - set => _flags = value ? (_flags | Flags.MustRevalidate) : (_flags & ~Flags.MustRevalidate); + set => SetFlag(Flags.MustRevalidate, value); } public bool ProxyRevalidate { get => (_flags & Flags.ProxyRevalidate) != 0; - set => _flags = value ? (_flags | Flags.ProxyRevalidate) : (_flags & ~Flags.ProxyRevalidate); + set => SetFlag(Flags.ProxyRevalidate, value); } public ICollection Extensions => _extensions ??= new UnvalidatedObjectCollection(); @@ -299,45 +309,22 @@ obj is CacheControlHeaderValue other && _flags == other._flags && _maxAge == other._maxAge && _sharedMaxAge == other._sharedMaxAge && + _maxStaleLimit == other._maxStaleLimit && _minFresh == other._minFresh && HeaderUtilities.AreEqualCollections(_noCacheHeaders, other._noCacheHeaders, StringComparer.OrdinalIgnoreCase) && HeaderUtilities.AreEqualCollections(_privateHeaders, other._privateHeaders, StringComparer.OrdinalIgnoreCase) && HeaderUtilities.AreEqualCollections(_extensions, other._extensions); - public override int GetHashCode() - { - HashCode hashcode = default; - - hashcode.Add(_flags); - hashcode.Add(_maxAge); - hashcode.Add(_sharedMaxAge); - hashcode.Add(_maxStaleLimit); - hashcode.Add(_minFresh); - - if ((_noCacheHeaders != null) && (_noCacheHeaders.Count > 0)) - { - int unorderedHash = 0; - foreach (string noCacheHeader in _noCacheHeaders) - { - unorderedHash ^= StringComparer.OrdinalIgnoreCase.GetHashCode(noCacheHeader); - } - hashcode.Add(unorderedHash); - } - - if ((_privateHeaders != null) && (_privateHeaders.Count > 0)) - { - int unorderedHash = 0; - foreach (string privateHeader in _privateHeaders) - { - unorderedHash ^= StringComparer.OrdinalIgnoreCase.GetHashCode(privateHeader); - } - hashcode.Add(unorderedHash); - } - - hashcode.Add(NameValueHeaderValue.GetHashCode(_extensions)); - - return hashcode.ToHashCode(); - } + public override int GetHashCode() => + HashCode.Combine( + _flags, + _maxAge, + _sharedMaxAge, + _maxStaleLimit, + _minFresh, + (_noCacheHeaders is null ? 0 : _noCacheHeaders.GetHashCode(StringComparer.OrdinalIgnoreCase)), + (_privateHeaders is null ? 0 : _privateHeaders.GetHashCode(StringComparer.OrdinalIgnoreCase)), + NameValueHeaderValue.GetHashCode(_extensions)); public static CacheControlHeaderValue Parse(string? input) { @@ -608,6 +595,18 @@ object ICloneable.Clone() private sealed class TokenObjectCollection : ObjectCollection { public override void Validate(string item) => HeaderUtilities.CheckValidToken(item, nameof(item)); + + public int GetHashCode(StringComparer comparer) + { + int hashcode = 0; + + foreach (string value in this) + { + hashcode ^= comparer.GetHashCode(value); + } + + return hashcode; + } } } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs index e6804b251cf6e..2fd19f857cbb0 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs @@ -8,6 +8,7 @@ namespace System.Net.Http.Headers { public class RangeConditionHeaderValue : ICloneable { + // Exactly one of date and entityTag will be set. private readonly DateTimeOffset _date; private readonly EntityTagHeaderValue? _entityTag; diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/CacheControlHeaderValueTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/CacheControlHeaderValueTest.cs index 05f9120f8073c..e93c46f4816e2 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/CacheControlHeaderValueTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/CacheControlHeaderValueTest.cs @@ -354,8 +354,14 @@ public void Equals_CompareValuesWithTimeSpanFieldsSet_MatchExpectation() value2.MaxStale = true; CompareValues(value1, value2, true); - value2.MaxStaleLimit = new TimeSpan(1, 2, 3); + value1.MaxStaleLimit = new TimeSpan(1, 2, 3); CompareValues(value1, value2, false); + + value2.MaxStaleLimit = new TimeSpan(2, 3, 4); + CompareValues(value1, value2, false); + + value1.MaxStaleLimit = new TimeSpan(2, 3, 4); + CompareValues(value1, value2, true); } [Fact]