Reduce the size of some Http header values (#83640)
authorMiha Zupan <mihazupan.zupan1@gmail.com>
Mon, 20 Mar 2023 19:45:43 +0000 (20:45 +0100)
committerGitHub <noreply@github.com>
Mon, 20 Mar 2023 19:45:43 +0000 (12:45 -0700)
* Reduce the size of some Http header values

* PR feedback

src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentRangeHeaderValue.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeConditionHeaderValue.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeItemHeaderValue.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/RetryConditionHeaderValue.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/StringWithQualityHeaderValue.cs
src/libraries/System.Net.Http/src/System/Net/Http/Headers/WarningHeaderValue.cs
src/libraries/System.Net.Http/tests/UnitTests/Headers/CacheControlHeaderValueTest.cs

index 918c2bd22dcee183b78c3778b0d485d84cb0316d..3067cc68689b46f8e93ac017606ec1ea83b2e19c 100644 (file)
@@ -5,8 +5,9 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
-using System.IO;
+using System.Runtime.CompilerServices;
 using System.Text;
+using System.Threading;
 
 namespace System.Net.Http.Headers
 {
@@ -27,103 +28,136 @@ namespace System.Net.Http.Headers
 
         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<NameValueHeaderValue>? _extensions;
 
+        private void SetTimeSpan(ref TimeSpan fieldRef, Flags flag, TimeSpan? value)
+        {
+            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)
+            {
+                Interlocked.Or(ref Unsafe.As<Flags, int>(ref _flags), (int)flag);
+            }
+            else
+            {
+                Interlocked.And(ref Unsafe.As<Flags, int>(ref _flags), (int)~flag);
+            }
+        }
+
         public bool NoCache
         {
-            get { return _noCache; }
-            set { _noCache = value; }
+            get => (_flags & Flags.NoCache) != 0;
+            set => SetFlag(Flags.NoCache, value);
         }
 
         public ICollection<string> NoCacheHeaders => _noCacheHeaders ??= new TokenObjectCollection();
 
         public bool NoStore
         {
-            get { return _noStore; }
-            set { _noStore = value; }
+            get => (_flags & Flags.NoStore) != 0;
+            set => SetFlag(Flags.NoStore, value);
         }
 
         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 => SetFlag(Flags.MaxStale, value);
         }
 
         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 => SetFlag(Flags.NoTransform, value);
         }
 
         public bool OnlyIfCached
         {
-            get { return _onlyIfCached; }
-            set { _onlyIfCached = value; }
+            get => (_flags & Flags.OnlyIfCached) != 0;
+            set => SetFlag(Flags.OnlyIfCached, value);
         }
 
         public bool Public
         {
-            get { return _publicField; }
-            set { _publicField = value; }
+            get => (_flags & Flags.Public) != 0;
+            set => SetFlag(Flags.Public, value);
         }
 
         public bool Private
         {
-            get { return _privateField; }
-            set { _privateField = value; }
+            get => (_flags & Flags.Private) != 0;
+            set => SetFlag(Flags.Private, value);
         }
 
         public ICollection<string> PrivateHeaders => _privateHeaders ??= new TokenObjectCollection();
 
         public bool MustRevalidate
         {
-            get { return _mustRevalidate; }
-            set { _mustRevalidate = value; }
+            get => (_flags & Flags.MustRevalidate) != 0;
+            set => SetFlag(Flags.MustRevalidate, value);
         }
 
         public bool ProxyRevalidate
         {
-            get { return _proxyRevalidate; }
-            set { _proxyRevalidate = value; }
+            get => (_flags & Flags.ProxyRevalidate) != 0;
+            set => SetFlag(Flags.ProxyRevalidate, value);
         }
 
         public ICollection<NameValueHeaderValue> Extensions => _extensions ??= new UnvalidatedObjectCollection<NameValueHeaderValue>();
@@ -136,23 +170,15 @@ namespace System.Net.Http.Headers
         {
             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 +186,7 @@ namespace System.Net.Http.Headers
 
             if (source._privateHeaders != null)
             {
-                foreach (var privateHeader in source._privateHeaders)
+                foreach (string privateHeader in source._privateHeaders)
                 {
                     PrivateHeaders.Add(privateHeader);
                 }
@@ -173,14 +199,14 @@ namespace System.Net.Http.Headers
         {
             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 +217,11 @@ namespace System.Net.Http.Headers
                 }
             }
 
-            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 +234,11 @@ namespace System.Net.Http.Headers
                 }
             }
 
-            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 +251,13 @@ namespace System.Net.Http.Headers
                 }
             }
 
-            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 +271,11 @@ namespace System.Net.Http.Headers
                 }
             }
 
-            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 +288,7 @@ namespace System.Net.Http.Headers
                 }
             }
 
-            if (_privateField)
+            if (Private)
             {
                 AppendValueWithSeparatorIfRequired(sb, privateString);
                 if ((_privateHeaders != null) && (_privateHeaders.Count > 0))
@@ -278,88 +304,27 @@ namespace System.Net.Http.Headers
             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 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);
-
-            if ((_noCacheHeaders != null) && (_noCacheHeaders.Count > 0))
-            {
-                foreach (var noCacheHeader in _noCacheHeaders)
-                {
-                    result ^= StringComparer.OrdinalIgnoreCase.GetHashCode(noCacheHeader);
-                }
-            }
-
-            if ((_privateHeaders != null) && (_privateHeaders.Count > 0))
-            {
-                foreach (var privateHeader in _privateHeaders)
-                {
-                    result ^= StringComparer.OrdinalIgnoreCase.GetHashCode(privateHeader);
-                }
-            }
-
-            if ((_extensions != null) && (_extensions.Count > 0))
-            {
-                foreach (var extension in _extensions)
-                {
-                    result ^= extension.GetHashCode();
-                }
-            }
-
-            return result;
-        }
+        public override bool Equals([NotNullWhen(true)] object? obj) =>
+            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.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)
         {
@@ -395,11 +360,10 @@ namespace System.Net.Http.Headers
             // 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<NameValueHeaderValue> nameValueList = new List<NameValueHeaderValue>();
             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 +397,87 @@ namespace System.Net.Http.Headers
             return input.Length - startIndex;
         }
 
-        private static bool TrySetCacheControlValues(CacheControlHeaderValue cc,
-            List<NameValueHeaderValue> nameValueList)
+        private static bool TrySetCacheControlValues(CacheControlHeaderValue cc, List<NameValueHeaderValue> 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 +486,12 @@ namespace System.Net.Http.Headers
             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 +535,20 @@ namespace System.Net.Http.Headers
             // 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;
         }
 
@@ -641,6 +595,18 @@ namespace System.Net.Http.Headers
         private sealed class TokenObjectCollection : ObjectCollection<string>
         {
             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;
+            }
         }
     }
 }
index 80ba175085767775c3d1e0195aa64164aded5d52..aa08f5e507e03516c2728bb332f11177870387c4 100644 (file)
@@ -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 @@ namespace System.Net.Http.Headers
             }
         }
 
-        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 @@ namespace System.Net.Http.Headers
 
             _length = length;
             _unit = HeaderUtilities.BytesUnit;
+            _from = -1;
         }
 
         public ContentRangeHeaderValue(long from, long to)
@@ -88,10 +72,13 @@ namespace System.Net.Http.Headers
             _from = from;
             _to = to;
             _unit = HeaderUtilities.BytesUnit;
+            _length = -1;
         }
 
         private ContentRangeHeaderValue()
         {
+            _from = -1;
+            _length = -1;
         }
 
         private ContentRangeHeaderValue(ContentRangeHeaderValue source)
@@ -104,35 +91,19 @@ namespace System.Net.Http.Headers
             _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 @@ namespace System.Net.Http.Headers
 
             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 @@ namespace System.Net.Http.Headers
             sb.Append('/');
             if (HasLength)
             {
-                sb.AppendSpanFormattable(_length!.Value);
+                sb.AppendSpanFormattable(_length);
             }
             else
             {
index 78255fb825dafdb4c73b2ad0b69d590118178dbb..2fd19f857cbb06731fe18aaef96e4808c251b99b 100644 (file)
@@ -8,18 +8,13 @@ namespace System.Net.Http.Headers
 {
     public class RangeConditionHeaderValue : ICloneable
     {
-        private readonly DateTimeOffset? _date;
+        // Exactly one of date and entityTag will be set.
+        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 +41,14 @@ namespace System.Net.Http.Headers
             _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)
         {
index 2fbb4e677b06c332fcd31e9d95e3ef6ef01746f0..f74a819631515d39a85b566cb011e369fae096e5 100644 (file)
@@ -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 @@ namespace System.Net.Http.Headers
                 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 @@ namespace System.Net.Http.Headers
         {
             Span<char> 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 @@ namespace System.Net.Http.Headers
                 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 @@ namespace System.Net.Http.Headers
                 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;
         }
 
index f1175f8414d26a3e1e88e89424858ad022c53e4f..db32ceea9a5a8aa6de2c15e5aeb89ea88e313a93 100644 (file)
@@ -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 @@ namespace System.Net.Http.Headers
             _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)
         {
index 00ce20b43a7c0b50e4997e974ff331a542668558..d7da89b5cb0bb8705fa0c1b5c10be3a91700e7d8 100644 (file)
@@ -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 @@ namespace System.Net.Http.Headers
             _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)
         {
index 6599ffd82fa5a3a0337760713d2a5ddc17216186..586c0b68fc549b7a28937656b8e30e91e0aca14b 100644 (file)
@@ -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 @@ namespace System.Net.Http.Headers
         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 @@ namespace System.Net.Http.Headers
             _agent = agent;
             _text = text;
             _date = date;
+            _dateHasValue = true;
         }
 
         private WarningHeaderValue(WarningHeaderValue source)
@@ -67,6 +56,7 @@ namespace System.Net.Http.Headers
             _agent = source._agent;
             _text = source._text;
             _date = source._date;
+            _dateHasValue = source._dateHasValue;
         }
 
         public override string ToString()
@@ -81,56 +71,33 @@ namespace System.Net.Http.Headers
             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 @@ namespace System.Net.Http.Headers
             // 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));
             }
         }
     }
index 05f9120f8073ca731bb43524721655a65709ef6c..e93c46f4816e2a42d2256afedb28e152ee3075f1 100644 (file)
@@ -354,8 +354,14 @@ namespace System.Net.Http.Tests
             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]