Skip to content
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

Minor race condition in HttpHeaders parsing #103238

Closed
MihaZupan opened this issue Jun 10, 2024 · 1 comment · Fixed by #103263
Closed

Minor race condition in HttpHeaders parsing #103238

MihaZupan opened this issue Jun 10, 2024 · 1 comment · Fixed by #103263

Comments

@MihaZupan
Copy link
Member

This #103008 (comment) from @ManickaP made me double-check the thread-safety of our reading/parsing logic on HttpHeaders.
After #68115, concurrent reads on the header collection are thread-safe. All read operations will see the same values, and won't corrupt the underlying storage.

A minor exception to that is that while they will all see equivalent values, they may not be exactly the same objects. Since some header values are mutable, this could lead to observable differences later.

Simple repro:

while (true)
{
    var headers = new ByteArrayContent([]).Headers;
    headers.TryAddWithoutValidation("Content-Type", "application/json; charset=utf-8");

    var task = Task.Run(() => headers.ContentType);
    MediaTypeHeaderValue contentType = headers.ContentType;
    await task;

    contentType.MediaType = "foo/bar";
    
    if (headers.ContentType.MediaType != "foo/bar")
    {
        Console.WriteLine("Race condition");
        break;
    }
}

The fix would be to avoid overwriting the underlying storage with different HeaderStoreItemInfo instances. The simplest way would be via an Interlocked.CompareExchange here:

storeValueRef = info = new HeaderStoreItemInfo() { RawValue = value };

and here
if (EntriesAreLiveView)
{
entries[i].Value = info;
}
else
{
Debug.Assert(Contains(entry.Key));
((Dictionary<HeaderDescriptor, object>)_headerStore!)[entry.Key] = info;
}

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 10, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@MihaZupan MihaZupan added this to the Future milestone Jun 10, 2024
@MihaZupan MihaZupan added bug and removed untriaged New issue has not been triaged by the area owner labels Jun 10, 2024
@MihaZupan MihaZupan modified the milestones: Future, 9.0.0 Jun 11, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant