Outgoing HttpClient LatencyInfo enricher value has incomplete escaping
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- csharp
- Domain
- observability
Research direction
Start in src/Libraries/Microsoft.Extensions.Http.Diagnostics/Latency/Internal/HttpClientLatencyLogEnricher.cs, reading AppendServerName, AppendTags, AppendCheckpoints, and AppendMeasures. Compare their escaping behavior with the incoming enricher scheme referenced in #7729. Done means every string field escapes both commas and slashes so the eight-section and item-level layout remains parseable.
Written by the indexing model from the issue text.
Description
Description
The outgoing HttpClient latency enricher (AddHttpClientLatencyTelemetry)
serializes the LatencyInfo tag value as a positional string but escapes nothing,
so legal input can shift the positional layout and corrupt the value a parser
reads.
The value format is (8 sections):
version , serverName , tagNames/ , tagValues/ , checkpointNames/ , checkpointMs/ , measureNames/ , measureValues/
- Sections are joined by
,. - List items are joined by
/.
Gaps in HttpClientLatencyLogEnricher:
- The section delimiter
,is never escaped. A,in any string field
(server name, tag name, tag value, checkpoint/measure name) shifts every
downstream section. - List items do not escape
/.AppendTags,AppendCheckpoints, and
AppendMeasuresappend names and values raw, so a/inside an item corrupts
the item-level split. - The server name field is not escaped at all (
AppendServerName).
Affected code
AppendServerNamewrites the server name raw.AppendTags/AppendCheckpoints/AppendMeasuresappend names and values raw, escaping neither,nor/.
Impact
Low severity. Most fields are registered/controlled names (checkpoints, measures,
tags). The one caller-controlled field is the server application name, taken from
a response header, which can legally contain , and /. Newlines are not
reachable (rejected by the server), so this is a data-integrity / parse-corruption
issue for consumers of LatencyInfo, not a log-forging issue.
Proposed fix
Escape , and / in every string field, including the server name, matching the
scheme the incoming enricher uses (/ and , replaced with _).
Notes
Discovered during API review of #7729 (AddHttpLatencyTelemetry). The incoming
enricher had the same class of defect and was fixed in that change; this issue
tracks the remaining gap in the outgoing HttpClient enricher, which currently
escapes nothing.
- Dominant language
- C#
- Stars
- 3.2k
- Forks
- 895
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 18
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from dotnet/extensions
-
bug untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
dotnet/extensions#7740 ·
-
untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
dotnet/extensions#7714 ·
-
area-ai enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
dotnet/extensions#7626 · 3 comments ·
-
bug untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
dotnet/extensions#7618 · 1 comment ·
-
bug needs-area-label untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
dotnet/extensions#7415 · 1 comment ·
All issues in dotnet/extensions
Similar issues
-
type/automation type/tech-debt
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
t/bug
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
ci-failure-cause test-failure
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
-
area:auth FE mvp P3
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
klasolsson81/jobbliggaren#1788 ·