Skip to content

Commit

Permalink
Redact query from HttpTelemetry (#104970)
Browse files Browse the repository at this point in the history 
* Redact query from HttpTelemetry

* Feedback

* Handle redirect as well + feedback
  • Loading branch information
@ManickaP
ManickaP authored Jul 17, 2024
1 parent ceccfe0 commit 98082dc
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 3 deletions.
28 changes: 28 additions & 0 deletions src/libraries/System.Net.Http/src/System/Net/Http/HttpTelemetry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Tracing;
using System.Text;
using System.Threading;

namespace System.Net.Http
Expand Down Expand Up @@ -37,6 +38,14 @@ public static class Keywords
private void RequestStart(string scheme, string host, int port, string pathAndQuery, byte versionMajor, byte versionMinor, HttpVersionPolicy versionPolicy)
{
Interlocked.Increment(ref _startedRequests);
if (!GlobalHttpSettings.DiagnosticsHandler.DisableUriRedaction)
{
int queryIndex = pathAndQuery.IndexOf('?');
if (queryIndex >= 0 && queryIndex < (pathAndQuery.Length - 1))
{
pathAndQuery = $"{pathAndQuery.AsSpan(0, queryIndex + 1)}*";
}
}
WriteEvent(eventId: 1, scheme, host, port, pathAndQuery, versionMajor, versionMinor, versionPolicy);
}

Expand Down Expand Up @@ -175,6 +184,25 @@ public void Redirect(string redirectUri)
WriteEvent(eventId: 16, redirectUri);
}

[NonEvent]
public void Redirect(Uri redirectUri)
{
if (!GlobalHttpSettings.DiagnosticsHandler.DisableUriRedaction)
{
string pathAndQuery = redirectUri.PathAndQuery;
int queryIndex = pathAndQuery.IndexOf('?');
if (queryIndex >= 0 && queryIndex < (pathAndQuery.Length - 1))
{
UriBuilder uriBuilder = new UriBuilder(redirectUri)
{
Query = "*"
};
redirectUri = uriBuilder.Uri;
}
}
Redirect(redirectUri.AbsoluteUri);
}

[NonEvent]
public void Http11ConnectionEstablished(long connectionId, string scheme, string host, int port, IPEndPoint? remoteEndPoint)
{
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/RedirectHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal override async ValueTask<HttpResponseMessage> SendAsync(HttpRequestMess

if (HttpTelemetry.Log.IsEnabled())
{
HttpTelemetry.Log.Redirect(redirectUri.AbsoluteUri);
HttpTelemetry.Log.Redirect(redirectUri);
}
if (NetEventSource.Log.IsEnabled())
{
Expand Down
149 changes: 147 additions & 2 deletions src/libraries/System.Net.Http/tests/FunctionalTests/TelemetryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.Tracing;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -48,6 +49,19 @@ public static IEnumerable<object[]> TestMethods_MemberData()
yield return new object[] { "InvokerSend" };
}

public static IEnumerable<object[]> Redaction_MemberData()
{
string[] uriTails = new string[] { "/test/path?q1=a&q2=b", "/test/path", "?q1=a&q2=b", "" };
foreach (string uriTail in new[] { "/test/path?q1=a&q2=b", "/test/path", "?q1=a&q2=b", "" })
{
foreach (string fragment in new[] { "", "#frag" })
{
yield return new object[] { uriTail + fragment, true };
yield return new object[] { uriTail + fragment, false };
}
}
}

[OuterLoop]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[MemberData(nameof(TestMethods_MemberData))]
Expand Down Expand Up @@ -399,7 +413,74 @@ await server.AcceptConnectionAsync(async connection =>
}, UseVersion.ToString(), testMethod).DisposeAsync();
}

private static void ValidateStartFailedStopEvents(ConcurrentQueue<(EventWrittenEventArgs Event, Guid ActivityId)> events, Version version, bool shouldHaveFailures = false, int count = 1)
[OuterLoop]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[MemberData(nameof(Redaction_MemberData))]
public async Task EventSource_SendingRequest_PathAndQueryRedaction_LogsStartStop(string uriTail, bool disableRedaction)
{
var psi = new ProcessStartInfo();
psi.Environment.Add("DOTNET_SYSTEM_NET_HTTP_DISABLEURIREDACTION", disableRedaction.ToString());
var fragIndex = uriTail.IndexOf('#');
var expectedPathAndQuery = uriTail.Substring(0, fragIndex >= 0 ? fragIndex : uriTail.Length);
if (!disableRedaction)
{
var queryIndex = expectedPathAndQuery.IndexOf('?');
expectedPathAndQuery = expectedPathAndQuery.Substring(0, queryIndex >= 0 ? queryIndex + 1 : expectedPathAndQuery.Length);
expectedPathAndQuery = queryIndex >= 0 ? expectedPathAndQuery + '*' : expectedPathAndQuery;
}
expectedPathAndQuery = expectedPathAndQuery.StartsWith('/') ? expectedPathAndQuery : '/' + expectedPathAndQuery;

await RemoteExecutor.Invoke(static async (useVersionString, uriTail, expectedPathAndQuery) =>
{
Version version = Version.Parse(useVersionString);
using var listener = new TestEventListener("System.Net.Http", EventLevel.Verbose, eventCounterInterval: 0.1d);
listener.AddActivityTracking();

var events = new ConcurrentQueue<(EventWrittenEventArgs Event, Guid ActivityId)>();
Uri expectedUri = null;
await listener.RunWithCallbackAsync(e => events.Enqueue((e, e.ActivityId)), async () =>
{
await GetFactoryForVersion(version).CreateClientAndServerAsync(
async uri =>
{
expectedUri = uri;
using HttpClientHandler handler = CreateHttpClientHandler(version);
using HttpClient client = CreateHttpClient(handler, useVersionString);
client.BaseAddress = uri;
using var invoker = new HttpMessageInvoker(handler);

var request = new HttpRequestMessage(HttpMethod.Get, uri)
{
Version = version
};

await client.GetAsync(uriTail);
},
async server =>
{
await server.AcceptConnectionAsync(async connection =>
{
await connection.ReadRequestDataAsync();
await WaitForEventCountersAsync(events);
await connection.SendResponseAsync();
});
});

await WaitForEventCountersAsync(events);
});
Assert.DoesNotContain(events, e => e.Event.EventId == 0); // errors from the EventSource itself

ValidateStartFailedStopEvents(events, version, pathAndQuery: expectedPathAndQuery);

ValidateConnectionEstablishedClosed(events, version, expectedUri);

ValidateRequestResponseStartStopEvents(events, null, 0, count: 1);

ValidateEventCounters(events, requestCount: 1, shouldHaveFailures: false, versionMajor: version.Major);
}, UseVersion.ToString(), uriTail, expectedPathAndQuery, new RemoteInvokeOptions { StartInfo = psi }).DisposeAsync();
}

private static void ValidateStartFailedStopEvents(ConcurrentQueue<(EventWrittenEventArgs Event, Guid ActivityId)> events, Version version, bool shouldHaveFailures = false, int count = 1, string? pathAndQuery = null)
{
(EventWrittenEventArgs Event, Guid ActivityId)[] starts = events.Where(e => e.Event.EventName == "RequestStart").ToArray();
foreach (EventWrittenEventArgs startEvent in starts.Select(e => e.Event))
Expand All @@ -409,6 +490,10 @@ private static void ValidateStartFailedStopEvents(ConcurrentQueue<(EventWrittenE
Assert.NotEmpty((string)startEvent.Payload[1]); // host
Assert.True(startEvent.Payload[2] is int port && port >= 0 && port <= 65535);
Assert.NotEmpty((string)startEvent.Payload[3]); // pathAndQuery
if (pathAndQuery is not null)
{
Assert.Equal(pathAndQuery, (string)startEvent.Payload[3]); // pathAndQuery
}
byte versionMajor = Assert.IsType<byte>(startEvent.Payload[4]);
Assert.Equal(version.Major, versionMajor);
byte versionMinor = Assert.IsType<byte>(startEvent.Payload[5]);
Expand Down Expand Up @@ -455,7 +540,7 @@ private static void ValidateStartFailedStopEvents(ConcurrentQueue<(EventWrittenE
}
}

// The validation asssumes that the connection id's are in range 0..(connectionCount-1)
// The validation assumes that the connection id's are in range 0..(connectionCount-1)
protected static void ValidateConnectionEstablishedClosed(ConcurrentQueue<(EventWrittenEventArgs Event, Guid ActivityId)> events, Version version, Uri uri, int connectionCount = 1)
{
EventWrittenEventArgs[] connectionsEstablished = events.Select(e => e.Event).Where(e => e.EventName == "ConnectionEstablished").ToArray();
Expand Down Expand Up @@ -824,6 +909,66 @@ await GetFactoryForVersion(version).CreateServerAsync((originalServer, originalU
}, UseVersion.ToString()).DisposeAsync();
}

[OuterLoop]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[MemberData(nameof(Redaction_MemberData))]
public async Task EventSource_Redirect_PathAndQueryRedaction_LogsRedirect(string uriTail, bool disableRedaction)
{
var psi = new ProcessStartInfo();
psi.Environment.Add("DOTNET_SYSTEM_NET_HTTP_DISABLEURIREDACTION", disableRedaction.ToString());
var fragIndex = uriTail.IndexOf('#');
var expectedUriTail = uriTail.Substring(0, fragIndex >= 0 ? fragIndex : uriTail.Length);
if (!disableRedaction)
{
var queryIndex = expectedUriTail.IndexOf('?');
expectedUriTail = expectedUriTail.Substring(0, queryIndex >= 0 ? queryIndex + 1 : expectedUriTail.Length);
expectedUriTail = queryIndex >= 0 ? expectedUriTail + '*' : expectedUriTail;
}
expectedUriTail = fragIndex >= 0 ? expectedUriTail + uriTail.Substring(fragIndex) : expectedUriTail;

await RemoteExecutor.Invoke(static async (useVersionString, uriTail, expectedUriTail) =>
{
Version version = Version.Parse(useVersionString);

using var listener = new TestEventListener("System.Net.Http", EventLevel.Verbose, eventCounterInterval: 0.1d);
listener.AddActivityTracking();
var events = new ConcurrentQueue<(EventWrittenEventArgs Event, Guid ActivityId)>();
Uri expectedUri = null;

await listener.RunWithCallbackAsync(e => events.Enqueue((e, e.ActivityId)), async () =>
{
await GetFactoryForVersion(version).CreateServerAsync((originalServer, originalUri) =>
{
return GetFactoryForVersion(version).CreateServerAsync(async (redirectServer, redirectUri) =>
{
expectedUri = redirectUri;
using HttpClient client = CreateHttpClient(useVersionString);

using HttpRequestMessage request = new(HttpMethod.Get, originalUri) { Version = version };

Task clientTask = client.SendAsync(request);
Task serverTask = originalServer.HandleRequestAsync(HttpStatusCode.Redirect, new[] { new HttpHeaderData("Location", redirectUri.AbsoluteUri + uriTail) });

await Task.WhenAny(clientTask, serverTask);
Assert.False(clientTask.IsCompleted, $"{clientTask.Status}: {clientTask.Exception}");
await serverTask;

serverTask = redirectServer.HandleRequestAsync();
await TestHelper.WhenAllCompletedOrAnyFailed(clientTask, serverTask);
await clientTask;
});
});

await WaitForEventCountersAsync(events);
});

EventWrittenEventArgs redirectEvent = events.Where(e => e.Event.EventName == "Redirect").Single().Event;
Assert.Equal(1, redirectEvent.Payload.Count);
Assert.Equal(expectedUri.ToString() + expectedUriTail, (string)redirectEvent.Payload[0]);
Assert.Equal("redirectUri", redirectEvent.PayloadNames[0]);
}, UseVersion.ToString(), uriTail, expectedUriTail, new RemoteInvokeOptions { StartInfo = psi }).DisposeAsync();
}

public static bool SupportsRemoteExecutorAndAlpn = RemoteExecutor.IsSupported && PlatformDetection.SupportsAlpn;

[OuterLoop]
Expand Down

0 comments on commit 98082dc

Please sign in to comment.