This repository was archived by the owner on Nov 20, 2018. It is now read-only.
HeaderUtilities enhancements #738
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,7 +98,7 @@ public int? Port | |
| { | ||
| return null; | ||
| } | ||
|
|
||
| return p; | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,18 +11,18 @@ namespace Microsoft.Net.Http.Headers | |
| { | ||
| public class CacheControlHeaderValue | ||
| { | ||
| public static readonly string PublicString = "public"; | ||
| public static readonly string PrivateString = "private"; | ||
| public static readonly string MaxAgeString = "max-age"; | ||
| public static readonly string SharedMaxAgeString = "s-maxage"; | ||
| public static readonly string NoCacheString = "no-cache"; | ||
| public static readonly string NoStoreString = "no-store"; | ||
| public static readonly string MaxStaleString = "max-stale"; | ||
| public static readonly string MinFreshString = "min-fresh"; | ||
| public static readonly string NoTransformString = "no-transform"; | ||
| public static readonly string OnlyIfCachedString = "only-if-cached"; | ||
| public static readonly string MustRevalidateString = "must-revalidate"; | ||
| public static readonly string ProxyRevalidateString = "proxy-revalidate"; | ||
|
There was a problem hiding this comment. Have we learned nothing from #712!? I actually prefer const as you can see if you read the PR comments, but we should be consistent and use static readonly instead. There was a problem hiding this comment. Apparently we were using these values in a switch statement and they need to be const for that. There was a problem hiding this comment. I don't know if David will be OK with this because of the switch statement. Like I said, I'm fine with consts given we don't mess up the values, but I would like consistency. There was a problem hiding this comment. Modified the switch statement. These strings are now public static readonly. There was a problem hiding this comment. 👎 This regressed the code readability and maintainability. These constants aren't worth it. There was a problem hiding this comment. Also avoiding a ToLower() which is required since switch on strings are case-sensitive. I'm seeing a one percent performance increase in ResponseCaching with it. Also it's code within a private method, so I'm not too concerned about its readability. |
||
|
|
||
| // The Cache-Control header is special: It is a header supporting a list of values, but we represent the list | ||
| // as _one_ instance of CacheControlHeaderValue. I.e we set 'SupportsMultipleValues' to 'true' since it is | ||
|
|
@@ -394,63 +394,120 @@ private static bool TrySetCacheControlValues( | |
| CacheControlHeaderValue cc, | ||
| List<NameValueHeaderValue> nameValueList) | ||
| { | ||
| for (var i = 0; i < nameValueList.Count; i++) | ||
| { | ||
| var nameValue = nameValueList[i]; | ||
| var name = nameValue.Name; | ||
| var success = true; | ||
|
|
||
| switch (name.Length) | ||
| { | ||
| case 6: | ||
| if (string.Equals(PublicString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTokenOnlyValue(nameValue, ref cc._public); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| case 7: | ||
| if (string.Equals(MaxAgeString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTimeSpan(nameValue, ref cc._maxAge); | ||
| } | ||
| else if(string.Equals(PrivateString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetOptionalTokenList(nameValue, ref cc._private, ref cc._privateHeaders); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| case 8: | ||
| if (string.Equals(NoCacheString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetOptionalTokenList(nameValue, ref cc._noCache, ref cc._noCacheHeaders); | ||
| } | ||
| else if (string.Equals(NoStoreString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTokenOnlyValue(nameValue, ref cc._noStore); | ||
| } | ||
| else if (string.Equals(SharedMaxAgeString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTimeSpan(nameValue, ref cc._sharedMaxAge); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| case 9: | ||
| if (string.Equals(MaxStaleString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = ((nameValue.Value == null) || TrySetTimeSpan(nameValue, ref cc._maxStaleLimit)); | ||
| if (success) | ||
| { | ||
| cc._maxStale = true; | ||
| } | ||
| } | ||
| else if (string.Equals(MinFreshString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTimeSpan(nameValue, ref cc._minFresh); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| case 12: | ||
| if (string.Equals(NoTransformString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTokenOnlyValue(nameValue, ref cc._noTransform); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| case 14: | ||
| if (string.Equals(OnlyIfCachedString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTokenOnlyValue(nameValue, ref cc._onlyIfCached); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| case 15: | ||
| if (string.Equals(MustRevalidateString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTokenOnlyValue(nameValue, ref cc._mustRevalidate); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| case 16: | ||
| if (string.Equals(ProxyRevalidateString, name, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| success = TrySetTokenOnlyValue(nameValue, ref cc._proxyRevalidate); | ||
| } | ||
| else | ||
| { | ||
| goto default; | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do SetContentLength while we're at it, but I don't know if we have any optimized code for this yet. Kestrel has one but it's for hex.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified the
FormatInt64implementation.As a reference, formatting
long num = 1234567890;to"1234567890"for 30000000 iterations:Old: 2027ms using
num.ToString(CultureInfo.InvariantCulture)New: 1023ms using
FormatInt64(num)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @KrzysztofCwalina