Outgoing HttpClient LatencyInfo enricher value has incomplete escaping

Open Beginner friendly
#7,757 0 comments 0 reactions 0 assignees View on GitHub

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

area-telemetry untriaged
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:

  1. The section delimiter , is never escaped. A , in any string field
    (server name, tag name, tag value, checkpoint/measure name) shifts every
    downstream section.
  2. List items do not escape /. AppendTags, AppendCheckpoints, and
    AppendMeasures append names and values raw, so a / inside an item corrupts
    the item-level split.
  3. The server name field is not escaped at all (AppendServerName).
Affected code

HttpClientLatencyLogEnricher

  • AppendServerName writes the server name raw.
  • AppendTags / AppendCheckpoints / AppendMeasures append 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from dotnet/extensions

All issues in dotnet/extensions

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.