-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[HTTP] Fix new header test #117202
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
[HTTP] Fix new header test #117202
Conversation
Tagging subscribers to this area: @dotnet/ncl |
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.
Pull Request Overview
Fix the new header test by sending the request over a loopback server so that header serialization happens on the wire.
- Wrap the existing test logic in
LoopbackServerFactory.CreateClientAndServerAsync
with separate client/server callbacks. - Introduce a
TaskCompletionSource<bool>
to coordinate whether the server should accept the connection. - Change
CookieContainer.Add(new Uri(uri), …)
to the overloadCookieContainer.Add(uri, …)
.
Comments suppressed due to low confidence (3)
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs:2175
- Consider specifying TaskCreationOptions.RunContinuationsAsynchronously when creating the TaskCompletionSource to avoid potential deadlocks from synchronous continuations.
TaskCompletionSource<bool> acceptConnection = new TaskCompletionSource<bool>();
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs:2205
- Use TrySetResult instead of SetResult to prevent an InvalidOperationException if the TaskCompletionSource is completed more than once.
acceptConnection.SetResult(false);
src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs:2177
- Ensure the TaskCompletionSource is completed in all code paths (including unexpected exceptions) by adding a catch-all or finally block inside the client lambda, otherwise the server callback may hang waiting for the result.
await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
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.
Thanks
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.
LGTM, thanks
Use loopback server since some of the validation happens when serializing to the wire which is after establishing the connection.
Fixes #117198
Compare without whitespace.