Skip to content

Remove workaround in HeaderPropagationMiddleware #20533

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

Merged
merged 1 commit into from
Apr 9, 2020

Conversation

halter73
Copy link
Member

@halter73 halter73 commented Apr 4, 2020

Fixed by #14146 which starts each request on a fresh ExecutionContext

Fixed by #14146 which starts each request on a fresh ExecutionContext
@ghost ghost added the area-middleware label Apr 4, 2020
@halter73 halter73 requested a review from davidfowl April 4, 2020 01:02
@@ -185,43 +185,5 @@ public async Task MultipleEntries_AddsFirstToProduceValue()
Assert.Contains("in", CapturedHeaders.Keys);
Assert.Equal("Test", CapturedHeaders["in"]);
}

[Fact]
public async Task HeaderInRequest_WithBleedAsyncLocal_HasCorrectValue()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed these tests because it's the responsibility of the server, not the middleware, to ensure it has a fresh ExecutionContext for each request. The HeaderPropagationMiddleware does not have any functional tests, and @benaadams already added functional tests to verify IAsyncLocals in general work as expected with Kestrel.

That said, I did manually verify that #14146 fixes things so that the HeaderPropagationMiddleware sees a fresh HeaderPropagationValues each request after removing async from Invoke. And verified this was not the case without the changes from #14146.

@halter73 halter73 merged commit 5607ea1 into master Apr 9, 2020
@halter73 halter73 deleted the halter73/remove-header-propagation-workaround branch April 9, 2020 23:43
@amcasey amcasey added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants