Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates ReflectionBasedDestructurer to destructure IQueryable values to prevent enumeration. #425

Merged
merged 2 commits into from
Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ Add the `SqlExceptionDestructurer` during setup:

[![Serilog.Exceptions.EntityFrameworkCore NuGet Package](https://img.shields.io/nuget/v/Serilog.Exceptions.EntityFrameworkCore.svg)](https://www.nuget.org/packages/Serilog.Exceptions.EntityFrameworkCore/) [![Serilog.Exceptions.EntityFrameworkCore package in serilog-exceptions feed in Azure Artifacts](https://feeds.dev.azure.com/serilog-exceptions/_apis/public/Packaging/Feeds/8479813c-da6b-4677-b40d-78df8725dc9c/Packages/ee2cd6f8-4c93-4774-9398-23c49ba41928/Badge)](https://dev.azure.com/serilog-exceptions/Serilog.Exceptions/_packaging?_a=package&feed=8479813c-da6b-4677-b40d-78df8725dc9c&package=ee2cd6f8-4c93-4774-9398-23c49ba41928&preferRelease=true) [![Serilog.Exceptions.EntityFrameworkCore NuGet Package Downloads](https://img.shields.io/nuget/dt/Serilog.Exceptions.EntityFrameworkCore)](https://www.nuget.org/packages/Serilog.Exceptions.EntityFrameworkCore)

> **WARNING**: If you are using EntityFrameworkCore with Serilog.Exceptions you must add this, otherwise in certain cases your entire database will be logged! This is because the exceptions in Entity Framework Core have properties that link to the entire database schema in them (See [#100](https://github.com/RehanSaeed/Serilog.Exceptions/issues/100), [aspnet/EntityFrameworkCore#15214](https://github.com/aspnet/EntityFrameworkCore/issues/15214))
> **WARNING**: In older versions of Serilog.Exceptions, if you are using EntityFrameworkCore with Serilog.Exceptions you must add this, otherwise in certain cases your entire database will be logged! This is because the exceptions in Entity Framework Core have properties that link to the entire database schema in them (See [#100](https://github.com/RehanSaeed/Serilog.Exceptions/issues/100), [aspnet/EntityFrameworkCore#15214](https://github.com/aspnet/EntityFrameworkCore/issues/15214)). Newer versions of Serilog.Exceptions avoids this issue by preventing the destructure of properties that implement IQueryable preventing their execution.

Add the [Serilog.Exceptions.EntityFrameworkCore](https://www.nuget.org/packages/Serilog.Exceptions.EntityFrameworkCore/) NuGet package to your project when using EntityFrameworkCore in your project

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class ReflectionBasedDestructurer : IExceptionDestructurer
private const string IdLabel = "$id";
private const string RefLabel = "$ref";
private const string CyclicReferenceMessage = "Cyclic reference";
private const string IQueryableDestructureSkippedMessage = "IQueryable skipped";
private readonly int destructuringDepth;
private readonly ReflectionInfoExtractor reflectionInfoExtractor = new();

Expand Down Expand Up @@ -204,7 +205,11 @@ private void AppendProperties(
return this.DestructureValueDictionary(dictionary, level, destructuredObjects, ref nextCyclicRefId);
}

if (value is IEnumerable enumerable)
if (value is IQueryable queryable)
{
return IQueryableDestructureSkippedMessage;
}
else if (value is IEnumerable enumerable)
{
return this.DestructureValueEnumerable(enumerable, level, destructuredObjects, ref nextCyclicRefId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ internal class TestContext : DbContext

public DbSet<User>? Users { get; set; }

public string CustomData { get; set; } = UserIdIDoNotWantToSee;

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseInMemoryDatabase(databaseName: "TestDebUpdateException");

protected override void OnModelCreating(ModelBuilder modelBuilder)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ namespace Serilog.Exceptions.Test.Destructurers
using System.Collections.Generic;
using System.Globalization;
using FluentAssertions;
using Microsoft.EntityFrameworkCore;
using Moq;
using Newtonsoft.Json.Linq;
using Serilog.Exceptions.Core;
Expand Down Expand Up @@ -196,6 +197,34 @@ public void WhenExceptionContainsDictionaryWithNonScalarValue_ShouldNotThrow()
.Which.Name.Should().Be("System.Collections.Generic.List`1[System.Int32]");
}

[Fact]
public void WhenExceptionContainsDbContext_ShouldSkipIQueryableProperties()
{
// Arrange
using var context = new ExceptionDbContext();
var exception = new CustomDbContextException("hello world", context);

// Act
var result = LogAndDestructureException(exception, new DestructuringOptionsBuilder());

// Assert
var exceptionDetails = ExtractExceptionDetails(result).Should().BeOfType<JObject>().Which;
var nameProperty = exceptionDetails
.Properties().Should().ContainSingle(x => x.Name == nameof(CustomDbContextException.Name)).Which
.Should().BeOfType<JProperty>().Which;

nameProperty.Value.Should().BeOfType<JValue>().Which.Value.Should().Be("hello world");

var contextProperty = exceptionDetails
.Properties().Should().ContainSingle(x => x.Name == nameof(CustomDbContextException.Context)).Which;

var customerProperty = contextProperty.Value.Should().BeOfType<JObject>().Which
.Properties().Should().ContainSingle(x => x.Name == nameof(ExceptionDbContext.Customer)).Which;

customerProperty.Value.Should().BeOfType<JValue>().Which.Value.Should().BeOfType<string>().Which
.Should().Be("IQueryable skipped");
}

public class DictNonScalarKeyException : Exception
{
public DictNonScalarKeyException() => this.Reference = new Dictionary<IEnumerable<int>, object>();
Expand All @@ -210,6 +239,35 @@ public DictNonScalarKeyException(string message, Exception innerException)

public Dictionary<IEnumerable<int>, object> Reference { get; }
}

#pragma warning disable CS3001 // Argument type is not CLS-compliant
#pragma warning disable CS3003 // Type is not CLS-compliant
public class CustomDbContextException : Exception
{
public CustomDbContextException(string name, DbContext context)
{
this.Name = name;
this.Context = context;
}

public string Name { get; set; }

public DbContext Context { get; }
}

private class ExceptionDbContext : DbContext
{
public DbSet<CustomerEntity> Customer => this.Set<CustomerEntity>();

protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder.UseInMemoryDatabase(databaseName: "TestDebUpdateException");

public class CustomerEntity
{
public string? Name { get; set; }

public int Id { get; set; }
}
}
}
}
#pragma warning restore CA2208 // Instantiate argument exceptions correctly