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

Microsoft.Extensions.Configuration.Binder is not linker-safe #40551

Closed
eerhardt opened this issue Aug 7, 2020 · 3 comments
Closed

Microsoft.Extensions.Configuration.Binder is not linker-safe #40551

eerhardt opened this issue Aug 7, 2020 · 3 comments
Labels
area-Extensions-Configuration blocked Issue/PR is blocked on something - see comments linkable-framework Issues associated with delivering a linker friendly framework
Milestone

Comments

@eerhardt
Copy link
Member

eerhardt commented Aug 7, 2020

Description

The ConfigurationBinder class will recursively create objects and set properties based on an IConfiguration object. This poses a problem to make it linker-safe because the current reflection flow analysis doesn't support recursive properties. See dotnet/linker#1087. We will need this support to make this API linker-safe.

The linker could decide to trim property setters and constructors which would cause the binder to not set properties correctly.

Here are the current warnings from the linker analysis on this assembly:

F:\git\runtime2\src\libraries\Microsoft.Extensions.Configuration.Binder\src\ConfigurationBinder.cs(371,17): Trim analysis warning IL2006: Microsoft.Extensions.Configuration.ConfigurationBinder.CreateInstance(Type): The parameter 'type' of method 'Microsoft.Extensions.Configuration.ConfigurationBinder.CreateInstance(Type)' with dynamically accessed member kinds 'None' is passed into the implicit 'this' parameter of method 'System.Reflection.TypeInfo.get_DeclaredConstructors()' which requires dynamically accessed member kinds 'NonPublicConstructors | PublicConstructors'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'NonPublicConstructors | PublicConstructors'.
F:\git\runtime2\src\libraries\Microsoft.Extensions.Configuration.Binder\src\ConfigurationBinder.cs(380,17): Trim analysis warning IL2006: Microsoft.Extensions.Configuration.ConfigurationBinder.CreateInstance(Type): The parameter 'type' of method 'Microsoft.Extensions.Configuration.ConfigurationBinder.CreateInstance(Type)' with dynamically accessed member kinds 'None' is passed into the parameter #0 of method 'System.Object System.Activator::CreateInstance(System.Type)' which requires dynamically accessed member kinds 'PublicParameterlessConstructor'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'PublicParameterlessConstructor'.
F:\git\runtime2\src\libraries\Microsoft.Extensions.Configuration.Binder\src\ConfigurationBinder.cs(403,13): Trim analysis warning IL2006: Microsoft.Extensions.Configuration.ConfigurationBinder.BindDictionary(Object,Type,IConfiguration,BinderOptions): The parameter 'dictionaryType' of method 'Microsoft.Extensions.Configuration.ConfigurationBinder.BindDictionary(Object,Type,IConfiguration,BinderOptions)' with dynamically accessed member kinds 'None' is passed into the implicit 'this' parameter of method 'System.Reflection.TypeInfo.GetDeclaredProperty(String)' which requires dynamically accessed member kinds 'NonPublicProperties | PublicProperties'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'NonPublicProperties | PublicProperties'.
F:\git\runtime2\src\libraries\Microsoft.Extensions.Configuration.Binder\src\ConfigurationBinder.cs(433,13): Trim analysis warning IL2006: Microsoft.Extensions.Configuration.ConfigurationBinder.BindCollection(Object,Type,IConfiguration,BinderOptions): The parameter 'collectionType' of method 'Microsoft.Extensions.Configuration.ConfigurationBinder.BindCollection(Object,Type,IConfiguration,BinderOptions)' with dynamically accessed member kinds 'None' is passed into the implicit 'this' parameter of method 'System.Reflection.TypeInfo.GetDeclaredMethod(String)' which requires dynamically accessed member kinds 'NonPublicMethods | PublicMethods'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'NonPublicMethods | PublicMethods'.
F:\git\runtime2\src\libraries\Microsoft.Extensions.Configuration.Binder\src\ConfigurationBinder.cs(565,17): Trim analysis warning IL2006: Microsoft.Extensions.Configuration.ConfigurationBinder.GetAllProperties(TypeInfo): The parameter 'type' of method 'Microsoft.Extensions.Configuration.ConfigurationBinder.GetAllProperties(TypeInfo)' with dynamically accessed member kinds 'None' is passed into the implicit 'this' parameter of method 'System.Reflection.TypeInfo.get_DeclaredProperties()' which requires dynamically accessed member kinds 'NonPublicProperties | PublicProperties'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'NonPublicProperties | PublicProperties'.

Regression?

No - this API has never been linker-safe.

Repro

Here's a simple application that doesn't work after being fully trimmed:

using Microsoft.Extensions.Configuration;
using System;
using System.Collections.Generic;

namespace ConsoleApp1
{
    class Program
    {
        static int Main(string[] args)
        {
            var dic = new Dictionary<string, string>
            {
                {"Section:Integer", "-2"},
                {"Section:Boolean", "TRUe"},
                {"Section:Nested:Integer", "11"},
                {"Section:Virtual", "Sup"}
            };
            var configurationBuilder = new ConfigurationBuilder();
            configurationBuilder.AddInMemoryCollection(dic);
            var config = configurationBuilder.Build();

            var options = config.Get<ConfigurationInterfaceOptions>();

            var childOptions = options.Section.Get<DerivedOptions>();

            Assert.True(childOptions.Boolean);
            Assert.Equal(-2, childOptions.Integer);
            Assert.Equal(11, childOptions.Nested.Integer);
            Assert.Equal("Derived:Sup", childOptions.Virtual);

            Assert.Equal("Section", options.Section.Key);
            Assert.Equal("Section", options.Section.Path);
            Assert.Null(options.Section.Value);

            return 100;
        }
    }

    public class ComplexOptions
    {
        public ComplexOptions()
        {
            Nested = new NestedOptions();
            Virtual = "complex";
        }

        public NestedOptions Nested { get; set; }
        public int Integer { get; set; }
        public bool Boolean { get; set; }
        public virtual string Virtual { get; set; }
    }

    public class NestedOptions
    {
        public int Integer { get; set; }
    }

    public class DerivedOptions : ComplexOptions
    {
        public override string Virtual
        {
            get
            {
                return base.Virtual;
            }
            set
            {
                base.Virtual = "Derived:" + value;
            }
        }
    }

    public class ConfigurationInterfaceOptions
    {
        public IConfigurationSection Section { get; set; }
    }

    internal class Assert
    {
        internal static void Equal<T>(T left, T right) where T : IEquatable<T>
        {
            if (!EqualityComparer<T>.Default.Equals(left, right)) throw new Exception();
        }

        internal static void Null(string value)
        {
            if (value is object) throw new Exception();
        }

        internal static void True(bool boolean)
        {
            if (!boolean) throw new Exception();
        }
    }
}

Running that application after trimming throws:

Unhandled exception. System.InvalidOperationException: Cannot create instance of type 'ConsoleApp1.ConfigurationInterfaceOptions' because it is missing a public parameterless constructor.
   at Microsoft.Extensions.Configuration.ConfigurationBinder.CreateInstance(Type type)
   at Microsoft.Extensions.Configuration.ConfigurationBinder.BindInstance(Type type, Object instance, IConfiguration config, BinderOptions options)
   at Microsoft.Extensions.Configuration.ConfigurationBinder.Get(IConfiguration configuration, Type type, Action`1 configureOptions)
   at Microsoft.Extensions.Configuration.ConfigurationBinder.Get[T](IConfiguration configuration, Action`1 configureOptions)
   at Microsoft.Extensions.Configuration.ConfigurationBinder.Get[T](IConfiguration configuration)
   at ConsoleApp1.Program.Main(String[] args) in F:\DotNetTest\HelloWorld\Program.cs:line 19

After forcing the linker to keep that constructor, I start getting:

Unhandled exception. System.ArgumentNullException: Value cannot be null. (Parameter 'configuration')
   at Microsoft.Extensions.Configuration.ConfigurationBinder.Get[T](IConfiguration configuration, Action`1 configureOptions)
   at Microsoft.Extensions.Configuration.ConfigurationBinder.Get[T](IConfiguration configuration)
   at ConsoleApp1.Program.Main(String[] args) in F:\DotNetTest\HelloWorld\Program.cs:line 30
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 7, 2020
@ghost
Copy link

ghost commented Aug 7, 2020

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

@eerhardt eerhardt added the linkable-framework Issues associated with delivering a linker friendly framework label Aug 7, 2020
@eerhardt eerhardt added this to the Future milestone Aug 7, 2020
@eerhardt eerhardt removed the untriaged New issue has not been triaged by the area owner label Aug 24, 2020
@eerhardt eerhardt added the blocked Issue/PR is blocked on something - see comments label Sep 1, 2020
@layomia
Copy link
Contributor

layomia commented Dec 6, 2022

This will be solved by #44493.

@layomia layomia closed this as completed Dec 6, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Configuration blocked Issue/PR is blocked on something - see comments linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

No branches or pull requests

3 participants