Skip to content

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

Merged
merged 2 commits into from
Jul 22, 2025

Conversation

MihaZupan
Copy link
Member

Fixes #116858

@MihaZupan MihaZupan added this to the 10.0.0 milestone Jul 21, 2025
@MihaZupan MihaZupan requested a review from a team July 21, 2025 14:18
@MihaZupan MihaZupan self-assigned this Jul 21, 2025
@Copilot Copilot AI review requested due to automatic review settings July 21, 2025 14:18
Copy link
Contributor

@Copilot Copilot AI left a 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");

Copy link
Contributor

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

@MihaZupan MihaZupan merged commit 822cb27 into dotnet:main Jul 22, 2025
81 of 87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DiagnosticsHandler is missing validation when injecting headers
3 participants