Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Native AOT: don't load SentryNative on unsupported platforms ([#4347](https://github.com/getsentry/sentry-dotnet/pull/4347))
- Fixed issue introduced in release 5.12.0 that might prevent other middleware or user code from reading request bodies ([#4373](https://github.com/getsentry/sentry-dotnet/pull/4373))
- SentryTunnelMiddleware overwrites the X-Forwarded-For header ([#4375](https://github.com/getsentry/sentry-dotnet/pull/4375))

### Dependencies

Expand Down
6 changes: 5 additions & 1 deletion src/Sentry.AspNetCore/SentryTunnelMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,14 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next)
Method = HttpMethod.Post,
Content = new StreamContent(memoryStream),
};
var existingForwardedFor = context.Request.Headers["X-Forwarded-For"];
var clientIp = context.Connection?.RemoteIpAddress?.ToString();
if (clientIp != null)
{
sentryRequest.Headers.Add("X-Forwarded-For", context.Connection?.RemoteIpAddress?.ToString());
var forwardedFor = string.IsNullOrEmpty(existingForwardedFor)
? clientIp
: $"{existingForwardedFor}, {clientIp}";
sentryRequest.Headers.Add("X-Forwarded-For", forwardedFor);
}
var responseMessage = await client.SendAsync(sentryRequest).ConfigureAwait(false);
// We send the response back to the client, whatever it was
Expand Down
43 changes: 42 additions & 1 deletion test/Sentry.AspNetCore.Tests/Tunnel/IntegrationsTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.TestHost;
using Microsoft.Extensions.DependencyInjection;

Expand All @@ -9,6 +11,7 @@ public class IntegrationsTests : IDisposable
private readonly TestServer _server;
private HttpClient _httpClient;
private MockHttpMessageHandler _httpMessageHandler;
private const string FakeRequestIp = "192.168.200.200";

public IntegrationsTests()
{
Expand All @@ -22,7 +25,16 @@ public IntegrationsTests()
factory.CreateClient(Arg.Any<string>()).Returns(_httpClient);
s.AddSingleton(factory);
})
.Configure(app => { app.UseSentryTunneling(); });
.Configure(app =>
{
app.Use((context, next) =>
{
// The context doesn't get sent by TestServer automatically... so we fake a remote request here
context.Connection.RemoteIpAddress = IPAddress.Parse(FakeRequestIp);
return next();
});
app.UseSentryTunneling();
});
_server = new TestServer(builder);
}

Expand Down Expand Up @@ -86,6 +98,35 @@ public async Task TunnelMiddleware_CanForwardEnvelopeToWhiteListedHost()
Assert.Equal(1, _httpMessageHandler.NumberOfCalls);
}

[Fact]
public async Task TunnelMiddleware_XForwardedFor_RetainsOriginIp()
{
// Arrange: Create a request with X-Forwarded-For header
var requestMessage = new HttpRequestMessage(new HttpMethod("POST"), "/tunnel")
{
Content = new StringContent(
@"{""sent_at"":""2021-01-01T00:00:00.000Z"",""sdk"":{""name"":""sentry.javascript.browser"",""version"":""6.8.0""},""dsn"":""https://[email protected]/1""}
{""type"":""session""}
{""sid"":""fda00e933162466c849962eaea0cfaff""}")
};
const string originalForwardedFor = "192.168.1.100, 10.0.0.1";
requestMessage.Headers.Add("X-Forwarded-For", originalForwardedFor);

// Act
await _server.CreateClient().SendAsync(requestMessage);

// Assert
Assert.Equal(1, _httpMessageHandler.NumberOfCalls);

var forwardedRequest = _httpMessageHandler.LastRequest;
Assert.NotNull(forwardedRequest);

Assert.True(forwardedRequest.Headers.Contains("X-Forwarded-For"));
var forwardedForHeader = forwardedRequest.Headers.GetValues("X-Forwarded-For").FirstOrDefault();
Assert.NotNull(forwardedForHeader);
forwardedForHeader.Should().Be($"{originalForwardedFor}, {FakeRequestIp}");
}

public void Dispose()
{
_httpClient.Dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ public class MockHttpMessageHandler : DelegatingHandler

public string Input { get; private set; }
public int NumberOfCalls { get; private set; }
public HttpRequestMessage LastRequest { get; private set; }

public MockHttpMessageHandler(string response, HttpStatusCode statusCode)
{
Expand All @@ -18,9 +19,10 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
CancellationToken cancellationToken)
{
NumberOfCalls++;
LastRequest = request;
if (request.Content != null) // Could be a GET-request without a body
{
Input = await request.Content.ReadAsStringAsync();
Input = await request.Content.ReadAsStringAsync(cancellationToken);
}
return new HttpResponseMessage
{
Expand Down
Loading