Skip to content

AdaptiveCapacityDictionary assert with 11 elements #34307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Tratcher opened this issue Jul 13, 2021 · 0 comments · Fixed by #34313
Closed

AdaptiveCapacityDictionary assert with 11 elements #34307

Tratcher opened this issue Jul 13, 2021 · 0 comments · Fixed by #34313
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug.
Milestone

Comments

@Tratcher
Copy link
Member

The client sent 11 cookies, the threshold for ACD is 10.

While this is only an assert, I expect being in this invalid state will cause some weird side-effects, potentially discarding cookies when doing incorrect resizes.

Process terminated. Assertion failed.
at Microsoft.AspNetCore.Internal.AdaptiveCapacityDictionary`2.set_Item(TKey key, TValue value) in C:\Github\aspnetcore\src\Shared\Dictionary\AdaptiveCapacityDictionary.cs:line 141
at Microsoft.Net.Http.Headers.CookieHeaderParserShared.TryParseValues(StringValues values, IDictionary`2 store, Boolean enableCookieNameEncoding, Boolean supportsMultipleValues) in C:\Github\aspnetcore\src\Http\Shared\CookieHeaderParserShared.cs:line 37
at Microsoft.AspNetCore.Http.RequestCookieCollection.ParseInternal(StringValues values, Boolean enableCookieNameEncoding) in C:\Github\aspnetcore\src\Http\Http\src\Internal\RequestCookieCollection.cs:line 76
at Microsoft.AspNetCore.Http.RequestCookieCollection.Parse(StringValues values) in C:\Github\aspnetcore\src\Http\Http\src\Internal\RequestCookieCollection.cs:line 65
at Microsoft.AspNetCore.Http.Features.RequestCookiesFeature.get_Cookies() in C:\Github\aspnetcore\src\Http\Http\src\Features\RequestCookiesFeature.cs:line 74
at Microsoft.AspNetCore.Http.DefaultHttpRequest.get_Cookies() in C:\Github\aspnetcore\src\Http\Http\src\Internal\DefaultHttpRequest.cs:line 145
at Microsoft.AspNetCore.Authentication.Cookies.ChunkingCookieManager.GetRequestCookie(HttpContext context, String key) in C:\Github\aspnetcore\src\Shared\ChunkingCookieManager\ChunkingCookieManager.cs:line 101
at Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler.ReadCookieTicket() in C:\Github\aspnetcore\src\Security\Authentication\Cookies\src\CookieAuthenticationHandler.cs:line 143
at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[TStateMachine](TStateMachine& stateMachine) in System.Private.CoreLib.dll:token 0x6004ea3+0x28
at Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler.ReadCookieTicket() in Microsoft.AspNetCore.Authentication.Cookies.dll:token 0x6000034+0x1f
at Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationHandler.EnsureCookieTicket() in C:\Github\aspnetcore\src\Security\Authentication\Cookies\src\CookieAuthenticationHandler.cs:line 78

Stack walk:

_dictionaryStorage and _arrayStorage are not both supposed to have a value

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private int FindIndex(TKey key)
{
Debug.Assert(_dictionaryStorage == null);
Debug.Assert(_arrayStorage != null);

Indexer.Set:

store[name] = Uri.UnescapeDataString(parsedValue.Value.Value);

Collection created:
var collection = new RequestCookieCollection(values.Count);
var store = collection.Store!;
if (CookieHeaderParserShared.TryParseValues(values, store, enableCookieNameEncoding, supportsMultipleValues: true))

(related: using StringValues.Count as the initial capacity is actually a bad idea, cookies are supposed to be sent in a single header so there should only be 0 or 1 entries in the StringValues. (except for a Kestrel HTTP/2 #26461 bug which I think is necessary to hit this issue).

Bug: The capacity constructor initializes both fields.

_dictionaryStorage = new Dictionary<TKey, TValue>(capacity);
_arrayStorage = Array.Empty<KeyValuePair<TKey, TValue>>();

@Tratcher Tratcher added bug This issue describes a behavior which is not expected - a bug. area-runtime labels Jul 13, 2021
@Tratcher Tratcher added this to the 6.0-preview7 milestone Jul 13, 2021
@Tratcher Tratcher self-assigned this Jul 13, 2021
Tratcher added a commit to Tratcher/aspnetcore that referenced this issue Jul 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions bug This issue describes a behavior which is not expected - a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants