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

Encrypted attribute convention throws exception in Select projection in EF Core 9.0.0-rc.2.24474.1 #34934

Closed
russcam opened this issue Oct 18, 2024 · 4 comments

Comments

@russcam
Copy link

russcam commented Oct 18, 2024

Here's a minimal repro: https://github.com/russcam/efcore9-bug

Background

The EF Core configuration uses a convention to encrypt properties in the EF model that are attributed with an EncryptedAttribute.
This is done in an IModelFinalizingConvention by setting a value converter for those attributed properties

public class EncryptedConvention : IModelFinalizingConvention
{
    private readonly EncryptionConverter _encryptionConverter;

    public EncryptedConvention(IEncryptor encryptor) =>
        _encryptionConverter = new EncryptionConverter(encryptor);

    public void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
        IConventionContext<IConventionModelBuilder> context)
    {
        foreach (var entityType in modelBuilder.Metadata.GetEntityTypes())
        {
            foreach (var property in entityType.GetDeclaredProperties())
            {
                var encryptedAttribute = property.PropertyInfo?.GetCustomAttribute<EncryptedAttribute>();
                if (encryptedAttribute != null)
                    property.SetValueConverter(_encryptionConverter);
            }
        }
    }
}

Problem

When projecting an encrypted property in a .Select(...) projection such as

var users = await db.Users
        .AsNoTracking()
        .Select(u => new 
        {
            Username = u.Username,
            Encrypted = u.Encrypted,
        })
        .ToListAsync();

the following exception is thrown in EF Core 9.0.0-rc.2.24474.1

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at lambda_method43(Closure, QueryContext, DbDataReader, ResultContext, SingleQueryResultCoordinator)
   at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
   at Program.<Main>$(String[] args) in C:\Users\russc\source\EFCore9Bug\EFCore9Bug\Program.cs:line 22
   at Program.<Main>(String[] args)

Observations

  • Removing the encrypted property from the select projection no longer throws the exception
  • Calls that return the encrypted property without using projection do not throw an exception (example in the minimal repro is returning the alice entity).
  • This works as expected with EF Core 8.0.10

Include provider and version information

EF Core version: 9.0.0-rc.2.24474.1
Database provider: Sqlite in the minimal repro, but also observe it with Postgres provider. It looks to be a core issue and not provider specific.
Target framework: net9.0
Operating system: Windows 11

@roji
Copy link
Member

roji commented Oct 18, 2024

Duplicate of #34760

@roji roji marked this as a duplicate of #34760 Oct 18, 2024
@roji
Copy link
Member

roji commented Oct 18, 2024

@russcam thanks for the repro, I could indeed see the regression between 8.0 and 9.0. However, this has nothing to do with MemberMemberBinding - the cause is having a value converter which captures (closure). This has already been fixed for GA in #34760 - if you try the latest EF daily build, the problem goes away (I tried with 9.0.0-rtm.24514.19).

Regardless, thanks again for reporting (regardless of it ending up being a duplicate) - this is exactly the kind of report we need to make sure we're not regressing, etc.

For reference, here's the narrowed-down standalone repro I produced from your original repro above:

Narrowed-down repro
await using var context = new AppDbContext();
await context.Database.EnsureDeletedAsync();
await context.Database.EnsureCreatedAsync();

var users = await context.Users
    .Select(u => u.Encrypted)
    .ToListAsync();

public class AppDbContext : DbContext
{
    public DbSet<User> Users { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer("Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<User>(b =>
        {
            b.Property(u => u.Encrypted).HasConversion(x => Foo(x), x => Foo(x));
            b.HasData(new User { UserId = 1, Encrypted = "some value" });
        });
    }

    // Adding static to the below makes the error go away
    public string Foo(string value) => value;
}

public class User
{
    public int UserId { get; set; }
    public string Encrypted { get; set; } = default!;
}

@russcam
Copy link
Author

russcam commented Oct 25, 2024

Thanks for taking the time to review @roji!

@roji
Copy link
Member

roji commented Oct 25, 2024

My pleasure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants