diff --git a/CHANGELOG.md b/CHANGELOG.md index 550ebac042..00d50ad648 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixes +- Sentry now decompresses Request bodies in ASP.NET Core when RequestDecompression middleware is enabled ([#4315](https://github.com/getsentry/sentry-dotnet/pull/4315)) - Custom ISentryEventProcessors are now run for native iOS events ([#4318](https://github.com/getsentry/sentry-dotnet/pull/4318)) - Crontab validation when capturing checkins ([#4314](https://github.com/getsentry/sentry-dotnet/pull/4314)) - Native AOT: link to static `lzma` on Linux/MUSL ([#4326](https://github.com/getsentry/sentry-dotnet/pull/4326)) diff --git a/src/Sentry.AspNetCore/RequestDecompression/ATTRIBUTION.txt b/src/Sentry.AspNetCore/RequestDecompression/ATTRIBUTION.txt new file mode 100644 index 0000000000..ff233854f6 --- /dev/null +++ b/src/Sentry.AspNetCore/RequestDecompression/ATTRIBUTION.txt @@ -0,0 +1,28 @@ +The code in this subdirectory has been adapted from +https://github.com/dotnet/aspnetcore + +The original license is as follows: + +The MIT License (MIT) + +Copyright (c) .NET Foundation and Contributors + +All rights reserved. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/src/Sentry.AspNetCore/RequestDecompression/RequestDecompressionMiddleware.cs b/src/Sentry.AspNetCore/RequestDecompression/RequestDecompressionMiddleware.cs new file mode 100644 index 0000000000..ad6576b3f2 --- /dev/null +++ b/src/Sentry.AspNetCore/RequestDecompression/RequestDecompressionMiddleware.cs @@ -0,0 +1,107 @@ +// Adapted from: https://github.com/dotnet/aspnetcore/blob/c18e93a9a2e2949e1a9c880da16abf0837aa978f/src/Middleware/RequestDecompression/src/RequestDecompressionMiddleware.cs + +// // Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Http.Metadata; +using Microsoft.AspNetCore.RequestDecompression; +using Microsoft.Extensions.Logging; + +namespace Sentry.AspNetCore.RequestDecompression; + +/// +/// Enables HTTP request decompression. +/// +internal sealed partial class RequestDecompressionMiddleware +{ + private readonly RequestDelegate _next; + private readonly ILogger _logger; + private readonly IRequestDecompressionProvider _provider; + private readonly IHub _hub; + + /// + /// Initialize the request decompression middleware. + /// + /// The delegate representing the remaining middleware in the request pipeline. + /// The logger. + /// The . + /// The Sentry Hub + public RequestDecompressionMiddleware( + RequestDelegate next, + ILogger logger, + IRequestDecompressionProvider provider, + IHub hub) + { + ArgumentNullException.ThrowIfNull(next); + ArgumentNullException.ThrowIfNull(logger); + ArgumentNullException.ThrowIfNull(provider); + ArgumentNullException.ThrowIfNull(hub); + + _next = next; + _logger = logger; + _provider = provider; + _hub = hub; + } + + /// + /// Invoke the middleware. + /// + /// The . + /// A task that represents the execution of this middleware. + public Task Invoke(HttpContext context) + { + Stream? decompressionStream = null; + try + { + decompressionStream = _provider.GetDecompressionStream(context); + } + catch (Exception e) + { + HandleException(e); + } + return decompressionStream is null + ? _next(context) + : InvokeCore(context, decompressionStream); + } + + private async Task InvokeCore(HttpContext context, Stream decompressionStream) + { + var request = context.Request.Body; + try + { + try + { + var sizeLimit = + context.GetEndpoint()?.Metadata?.GetMetadata()?.MaxRequestBodySize + ?? context.Features.Get()?.MaxRequestBodySize; + + context.Request.Body = new SizeLimitedStream(decompressionStream, sizeLimit, static (long sizeLimit) => throw new BadHttpRequestException( + $"The decompressed request body is larger than the request body size limit {sizeLimit}.", + StatusCodes.Status413PayloadTooLarge)); + } + catch (Exception e) + { + HandleException(e); + } + + await _next(context).ConfigureAwait(false); + } + finally + { + context.Request.Body = request; + await decompressionStream.DisposeAsync().ConfigureAwait(false); + } + } + + private void HandleException(Exception e) + { + const string description = + "An exception was captured and then re-thrown, when attempting to decompress the request body." + + "The web server likely returned a 5xx error code as a result of this exception."; + e.SetSentryMechanism(nameof(RequestDecompressionMiddleware), description, handled: false); + _hub.CaptureException(e); + ExceptionDispatchInfo.Capture(e).Throw(); + } +} diff --git a/src/Sentry.AspNetCore/RequestDecompression/SizeLimitedStream.cs b/src/Sentry.AspNetCore/RequestDecompression/SizeLimitedStream.cs new file mode 100644 index 0000000000..9264bbc555 --- /dev/null +++ b/src/Sentry.AspNetCore/RequestDecompression/SizeLimitedStream.cs @@ -0,0 +1,113 @@ +// Copied from: https://github.com/dotnet/aspnetcore/blob/c18e93a9a2e2949e1a9c880da16abf0837aa978f/src/Shared/SizeLimitedStream.cs +// The only changes are the namespace and the addition of this comment + +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Sentry.AspNetCore.RequestDecompression; + +#nullable enable + +internal sealed class SizeLimitedStream : Stream +{ + private readonly Stream _innerStream; + private readonly long? _sizeLimit; + private readonly Action? _handleSizeLimit; + private long _totalBytesRead; + + public SizeLimitedStream(Stream innerStream, long? sizeLimit, Action? handleSizeLimit = null) + { + ArgumentNullException.ThrowIfNull(innerStream); + + _innerStream = innerStream; + _sizeLimit = sizeLimit; + _handleSizeLimit = handleSizeLimit; + } + + public override bool CanRead => _innerStream.CanRead; + + public override bool CanSeek => _innerStream.CanSeek; + + public override bool CanWrite => _innerStream.CanWrite; + + public override long Length => _innerStream.Length; + + public override long Position + { + get + { + return _innerStream.Position; + } + set + { + _innerStream.Position = value; + } + } + + public override void Flush() + { + _innerStream.Flush(); + } + + public override int Read(byte[] buffer, int offset, int count) + { + var bytesRead = _innerStream.Read(buffer, offset, count); + + _totalBytesRead += bytesRead; + if (_totalBytesRead > _sizeLimit) + { + if (_handleSizeLimit != null) + { + _handleSizeLimit(_sizeLimit.Value); + } + else + { + throw new InvalidOperationException("The maximum number of bytes have been read."); + } + } + + return bytesRead; + } + + public override long Seek(long offset, SeekOrigin origin) + { + return _innerStream.Seek(offset, origin); + } + + public override void SetLength(long value) + { + _innerStream.SetLength(value); + } + + public override void Write(byte[] buffer, int offset, int count) + { + _innerStream.Write(buffer, offset, count); + } + + public override Task ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) + { + return ReadAsync(buffer.AsMemory(offset, count), cancellationToken).AsTask(); + } + + public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) + { +#pragma warning disable CA2007 + var bytesRead = await _innerStream.ReadAsync(buffer, cancellationToken); +#pragma warning restore CA2007 + + _totalBytesRead += bytesRead; + if (_totalBytesRead > _sizeLimit) + { + if (_handleSizeLimit != null) + { + _handleSizeLimit(_sizeLimit.Value); + } + else + { + throw new InvalidOperationException("The maximum number of bytes have been read."); + } + } + + return bytesRead; + } +} diff --git a/src/Sentry.AspNetCore/SentryStartupFilter.cs b/src/Sentry.AspNetCore/SentryStartupFilter.cs index 34fa0b94e0..998e4bf560 100644 --- a/src/Sentry.AspNetCore/SentryStartupFilter.cs +++ b/src/Sentry.AspNetCore/SentryStartupFilter.cs @@ -1,5 +1,10 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.RequestDecompression; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Sentry.AspNetCore.RequestDecompression; +using Sentry.Extensibility; namespace Sentry.AspNetCore; @@ -11,10 +16,19 @@ public class SentryStartupFilter : IStartupFilter /// /// Adds Sentry to the pipeline. /// - public Action Configure(Action next) => e => + public Action Configure(Action next) => app => { - e.UseSentry(); + // If we are capturing request bodies and the user has configured request body decompression, we need to + // ensure that the RequestDecompression middleware gets called before Sentry's middleware. + var options = app.ApplicationServices.GetService>(); + if (options?.Value is { } o && o.MaxRequestBodySize != RequestSize.None + && app.ApplicationServices.GetService() is not null) + { + app.UseMiddleware(); + } - next(e); + app.UseSentry(); + + next(app); }; } diff --git a/src/Sentry/Extensibility/BaseRequestPayloadExtractor.cs b/src/Sentry/Extensibility/BaseRequestPayloadExtractor.cs index 2b10c67d1b..75ac864886 100644 --- a/src/Sentry/Extensibility/BaseRequestPayloadExtractor.cs +++ b/src/Sentry/Extensibility/BaseRequestPayloadExtractor.cs @@ -18,19 +18,22 @@ public abstract class BaseRequestPayloadExtractor : IRequestPayloadExtractor return null; } - if (request.Body == null - || !request.Body.CanSeek - || !request.Body.CanRead - || !IsSupported(request)) + if (request.Body is not { CanRead: true } || !IsSupported(request)) { return null; } + if (!request.Body.CanSeek) + { + // When RequestDecompression is enabled, the RequestDecompressionMiddleware will store a SizeLimitedStream + // in the request body after decompression. Seek operations throw an exception, but we can still read the stream + return DoExtractPayLoad(request); + } + var originalPosition = request.Body.Position; try { request.Body.Position = 0; - return DoExtractPayLoad(request); } finally diff --git a/test/Sentry.AspNetCore.TestUtils/FakeSentryServer.cs b/test/Sentry.AspNetCore.TestUtils/FakeSentryServer.cs index 495ec0659c..ce52a82f53 100644 --- a/test/Sentry.AspNetCore.TestUtils/FakeSentryServer.cs +++ b/test/Sentry.AspNetCore.TestUtils/FakeSentryServer.cs @@ -5,7 +5,7 @@ namespace Sentry.AspNetCore.TestUtils; -internal static class FakeSentryServer +public static class FakeSentryServer { public static TestServer CreateServer(IReadOnlyCollection handlers) { diff --git a/test/Sentry.AspNetCore.Tests/RequestDecompressionMiddleware/RequestDecompressionMiddlewareTests.cs b/test/Sentry.AspNetCore.Tests/RequestDecompressionMiddleware/RequestDecompressionMiddlewareTests.cs new file mode 100644 index 0000000000..d3639ca96c --- /dev/null +++ b/test/Sentry.AspNetCore.Tests/RequestDecompressionMiddleware/RequestDecompressionMiddlewareTests.cs @@ -0,0 +1,172 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.RequestDecompression; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Sentry.AspNetCore.TestUtils; + +namespace Sentry.AspNetCore.Tests.RequestDecompressionMiddleware; + +public class RequestDecompressionMiddlewareTests : IDisposable +{ + private class Fixture : IDisposable + { + private TestServer _server; + private HttpClient _client; + private IRequestDecompressionProvider _provider; + public Exception LastException { get; private set; } + + private IWebHostBuilder GetBuilder() + { + var exceptionProcessor = Substitute.For(); + exceptionProcessor.Process(Arg.Do(e => LastException = e), + Arg.Any()); + + var sentry = FakeSentryServer.CreateServer(); + var sentryHttpClient = sentry.CreateClient(); + return new WebHostBuilder() + .ConfigureServices(services => + { + services.AddRouting(); + if (_provider is not null) + { + services.AddSingleton(_provider); + } + else + { + services.AddRequestDecompression(); + } + }) + .UseSentry(o => + { + o.Dsn = ValidDsn; + o.MaxRequestBodySize = RequestSize.Always; + o.SentryHttpClientFactory = new DelegateHttpClientFactory(_ => sentryHttpClient); + o.AddExceptionProcessor(exceptionProcessor); + }) + .Configure(app => + { + app.UseRouting(); + app.UseEndpoints(endpoints => + { + endpoints.MapPost("/echo", async context => + { + using var reader = new StreamReader(context.Request.Body, Encoding.UTF8); + var body = await reader.ReadToEndAsync(); + await context.Response.WriteAsync(body); + }); + }); + }); + } + + public void UseFakeDecompressionProvider() + { + _provider = new FlakyDecompressionProvider(); + } + + private class FlakyDecompressionProvider : IRequestDecompressionProvider + { + public Stream GetDecompressionStream(HttpContext context) + { + // Simulate a decompression error + throw new InvalidDataException("Flaky decompression error"); + } + } + + public HttpClient GetSut() + { + _server = new TestServer(GetBuilder()); + _client = _server.CreateClient(); + return _client; + } + + public void Dispose() + { + _client.Dispose(); + _server.Dispose(); + } + } + + private readonly Fixture _fixture = new(); + + public void Dispose() + { + _fixture.Dispose(); + } + + [Fact] + public async Task AddRequestDecompression_PlainBodyContent_IsUnaltered() + { + var client = _fixture.GetSut(); + + var json = "{\"Foo\":\"Bar\"}"; + var content = new StringContent(json, Encoding.UTF8, "application/json"); + + var response = await client.PostAsync("/echo", content); + var responseBody = await response.Content.ReadAsStringAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(json, responseBody); + } + + [Fact] + public async Task AddRequestDecompression_CompressedBodyContent_IsDecompressed() + { + var client = _fixture.GetSut(); + + var json = "{\"Foo\":\"Bar\"}"; + var gzipped = CompressGzip(json); + var content = new ByteArrayContent(gzipped); + content.Headers.Add("Content-Encoding", "gzip"); + content.Headers.ContentType = new MediaTypeHeaderValue("application/json"); + + var response = await client.PostAsync("/echo", content); + var responseBody = await response.Content.ReadAsStringAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(json, responseBody); + } + + [Fact] + public async Task DecompressionError_SentryCapturesException() + { + // Arrange + _fixture.UseFakeDecompressionProvider(); + var client = _fixture.GetSut(); + + var json = "{\"Foo\":\"Bar\"}"; + var gzipped = CompressGzip(json); + var content = new ByteArrayContent(gzipped); + content.Headers.Add("Content-Encoding", "gzip"); + content.Headers.ContentType = new MediaTypeHeaderValue("application/json"); + + // Act + try + { + _ = await client.PostAsync("/echo", content); + } + catch + { + // We're expecting an exception here... what we're interested in is what happens on the server + } + + // Assert + using (new AssertionScope()) + { + _fixture.LastException.Should().NotBeNull(); + _fixture.LastException?.Message.Should().Be("Flaky decompression error"); + } + } + + private static byte[] CompressGzip(string str) + { + var bytes = Encoding.UTF8.GetBytes(str); + using var output = new MemoryStream(); + using (var gzip = new GZipStream(output, CompressionLevel.Optimal, leaveOpen: true)) + { + gzip.Write(bytes, 0, bytes.Length); + } + return output.ToArray(); + } +}