-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Validate injected header values in DiagnosticsHandler #117884
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
Validate injected header values in DiagnosticsHandler #117884
Conversation
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
This PR adds validation for header values injected by the DiagnosticsHandler to prevent exceptions from malformed headers. The change ensures that invalid header names or values are properly validated and throw appropriate exceptions early in the request pipeline.
Key changes:
- Enhanced header injection validation in DiagnosticsHandler to use validated header addition
- Exposed internal HeaderDescriptor method for proper header validation
- Added comprehensive test coverage for invalid header scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| DiagnosticsHandler.cs | Modified header injection logic to validate headers using Add() instead of TryAddWithoutValidation() |
| HttpHeaders.cs | Changed GetHeaderDescriptor method visibility from private to internal |
| DiagnosticsTests.cs | Added test cases for invalid header name/value injection scenarios |
Comments suppressed due to low confidence (1)
src/libraries/System.Net.Http/tests/FunctionalTests/DiagnosticsTests.cs:427
- The TestAsync parameter appears to be undefined in this context. This should likely be 'testAsync' (lowercase) or the correct parameter name based on the test method signature.
Assert.DoesNotContain(tags, t => t.Key == "error.type");
|
Tagging subscribers to this area: @dotnet/ncl |
Fixes #116858