Open
Description
Triage for dotnet/runtime#116858.
Repo filter: All networking issues.
MihuBot version: 1a6438
.
Ping MihaZupan for any issues.
This is a test triage report generated by AI, aimed at helping the triage team quickly identify past issues/PRs that may be related.
Take any conclusions with a large grain of salt.
dotnet/runtime#116858: DiagnosticsHandler is missing validation when injecting headers by MihaZupan
[Tool] Searching for DiagnosticsHandler header validation, TryAddWithoutValidation header, HttpHeaders.CheckContainsNewLineOrNull, propagator header validation, invalid header names DiagnosticsHandler (IncludeOpen=True, IncludeClosed=True, IncludeIssues=True, IncludePullRequests=True, Repository=dotnet/runtime)
[Tool] Found 56 issues, 59 comments, 56 returned results (5165 ms)
Here are the most relevant related issues and discussions for issue #116858, "DiagnosticsHandler is missing validation when injecting headers":
1. PR #116634 (June 2025) - [HTTP] Stricter header value validation
- Summary: This open PR proposes stricter validation of header values, specifically adding NUL (
\0
) to the list of forbidden characters and ensuring compliance with RFC 9110, which prohibits CR, LF, or NUL in header values. The PR also discusses aligning validation across different handlers and includes new tests for negative/positive cases. - Relevance: Directly addresses the risk of unsafe header values and discusses the need for consistent validation, which is the core concern of #116858.
2. PR #116335 (June 2025) - [WinHTTP] Validate header values for Latin1
- Summary: This PR focuses on validating header values for Latin-1 compliance in WinHTTP, referencing the need to filter for CRLF on a per-header basis. MihaZupan comments that new lines may be valid in some scenarios (e.g., line folding), but generally, validation should be left to the header collection.
- Relevance: Discusses the nuances of header validation, including when to allow or reject new lines, which is relevant to the suggestion in #116858 to use
CheckContainsNewLineOrNull
.
3. Issue #49463 (March 2021) - TryAddWithoutValidation works differently for HttpClient.DefaultRequestHeaders and HttpRequestMessage.Headers
- Summary: Highlights inconsistencies in how
TryAddWithoutValidation
handles headers, with some cases still performing validation or formatting. The discussion includes suggestions to avoid unnecessary parsing/validation. - Relevance: Shows historical context for why
TryAddWithoutValidation
may be risky and why explicit validation (as suggested in #116858) could be necessary.
4. Issue #50597 (April 2021) - HttpRequestMessage.Headers.Add should validate the header value
- Summary: Suggests that
Headers.Add
should check for new lines in values to prevent header injection attacks. The issue is about security and the need for validation. - Relevance: Reinforces the security motivation behind the request in #116858.
5. PR #70895 (June 2022) - Implement HTTP/2 WebSockets
- Summary: In the comments, MihaZupan and others discuss the need to validate header values for new lines and control characters, with a suggestion to use
CheckContainsNewLine
. - Relevance: Shows that the team has previously discussed and agreed on the need for such validation in other contexts.
6. Issue #57230 (August 2021) - DiagnosticsHandler: System.FormatException: The format of value ... is invalid
- Summary: Reports a case where invalid header values injected by DiagnosticsHandler cause exceptions. MihaZupan suggests workarounds and notes that a fix is pending.
- Relevance: Demonstrates real-world impact of missing validation in DiagnosticsHandler.
7. PR #55392 (July 2021) - Consume DistributedContextPropagator APIs in DiagnosticsHandler
- Summary: Refactors DiagnosticsHandler to use DistributedContextPropagator for header injection. Discussion includes which headers are allowed and how to handle invalid ones.
- Relevance: Shows the current mechanism for header injection and the need for validation at this layer.
8. Issue #67199 (March 2022) - HttpHeaders should not remove invalid values during validating access
- Summary: Discusses the behavior of
HttpHeaders
when encountering invalid values, including new lines, and the side effects of validation. - Relevance: Provides context on how invalid values are currently handled and the potential for improvement.
9. Issue #79915 (December 2022) - Headers.TryAddWithoutValidation can still have side effects on the value
- Summary: Reports that even "without validation," header values may be trimmed or altered, which can be surprising and potentially unsafe.
- Relevance: Supports the argument that explicit validation is needed even when using "without validation" APIs.
10. Issue #102584 (May 2024) - HttpHeaders TryParse can throw
- Summary: Reports that certain invalid header values can cause exceptions during parsing, even when added with
TryAddWithoutValidation
. - Relevance: Shows that lack of up-front validation can lead to runtime errors.
11. PR #114583 (April 2025) - Support W3C Propagator
- Summary: Implements W3C Propagator support. Comments discuss the need for tests that mix valid and invalid data, and the importance of robust validation.
- Relevance: Shows ongoing work to ensure propagators handle headers safely and validates the need for better validation in DiagnosticsHandler.
Conclusions
- There is broad agreement in the repo that header value validation is important for security and correctness, especially when headers are injected programmatically (as in DiagnosticsHandler).
- Several recent and ongoing PRs (notably #116634) are working to strengthen header validation, including for CR, LF, and NUL characters.
- There is precedent for using
HttpHeaders.CheckContainsNewLineOrNull
or similar checks before injecting headers, and this is being discussed as a best practice. - The risk of using
TryAddWithoutValidation
without additional checks is well-documented, and there are real-world issues caused by this (see #57230, #102584). - There is some nuance around which headers should be validated and how (e.g., line folding, Latin-1 vs ASCII), but the general direction is toward stricter validation.
Your suggestion in #116858 is well-supported by both recent and historical discussions, and aligns with ongoing work in the repo.
If you need more detail on any specific issue or PR, let me know!
Metadata
Metadata
Assignees
Labels
No labels