ConfuserEx icon indicating copy to clipboard operation
ConfuserEx copied to clipboard

For renamer, space(\u0020) is not an vaild name when used as first char in fullname of type or name of nested type

Open yyjdelete opened this issue 5 years ago • 6 comments

For the below code, will get TypeLoadException after rename Z0 to Z0 or Z3 to Z3, Z0 works well, and Z2 will get Z2 in runtime.

Seems dotnet runtime just trim the lead space in fullname of type and name of nested type when read it from string, and the same logic as dnlib does. Maybe renamer should consider this and follow the same logic to avoid crash.

See the result of Type.GetType(" System.Collections.Generic.Dictionary2+ Enumerator"), it will get the same result as Type.GetType("System.Collections.Generic.Dictionary2+Enumerator")

    class Program
    {
        [Z2(Z = typeof(Z0), Z1 = Z3.A, Z3 = Z3.A)]
        public class Z0
        {
        }
        public enum Z3
        {
            A = 1
        }

        [Z2(Z = typeof(Z2))]
        public class Z2 : Attribute
        {
            public Type Z { get; set; }
            public object Z1 { get; set; }
            public Z3 Z3 { get; set; }
        }
        static void Main(string[] args)
        {
            Console.WriteLine(typeof(Z0).GetCustomAttribute<Z2>().Z);
            Console.WriteLine(typeof(Z2).GetCustomAttribute<Z2>().Z);
        }
    }

yyjdelete avatar Aug 14 '20 09:08 yyjdelete

Could you please prepare a unit-test for such a case (see my example? Ideally, minimal reproducible sample.

KvanTTT avatar Aug 14 '20 12:08 KvanTTT

I do the test again and again manually with ConfuserEX or edit assembly with dnSpy to verify this, since Renamer use random names every time, and not always hit the issue. Maybe I can try to find an fixed magic seed to do that.

The fullname of output type needed to be one of <space>Namespace.Type*(space at begin of namespace) or <space>Type*(with empty namespace and space at begin of typename) or *Type+<space>NestedType*(space at begin of nested type name) to hit the issue.

yyjdelete avatar Aug 14 '20 13:08 yyjdelete

Well that is a very specific issue. The space as first character is legal for msil types, but types with names like this do not work anymore when using the Type.GetType(string) method to load the specific type.

In general I strongly recommend not using the unicode renaming mode on types that are meant to be used by reflection. It may work with it, but in general that is plain luck.

The Letters renaming mode is the first one that is safe for reflection. The more complex modes may add characters that are "special" into name name of the members. Such as +-signs or square brackets. Those will mess with the detection of nested types and generic types.

This is also the reason why all types that are referenced by WPF are limited to this renaming mode. WPF relies heavily on reflection and there are all sorts of problems when using more complex renaming modes.

mkaring avatar Aug 16 '20 16:08 mkaring

@mkaring Yes, but seems this will also broken async stackTrace for netcoreapp >= 3.0. Because the stackTrace now use typeinfo inside [AsyncStateMachineAttribute(stateMachineType: typeof(<M>d__0))]see code here internal in [StackTrace.ToString()] (https://github.com/dotnet/runtime/blob/71283319d0bf0f8c9132e117419fc2294342b510/src/libraries/System.Private.CoreLib/src/System/Diagnostics/StackTrace.cs#L395) to hide all async internal method from output, and which is failed to load after rename.

yyjdelete avatar Aug 17 '20 05:08 yyjdelete

Does this only break the display of the stack trace or does it cause a crash?

mkaring avatar Aug 21 '20 07:08 mkaring

Any call to Exception.ToString() or Environment.StackTrace in async path will throw another TypeLoadException, and may crash if is not catched again without ToString()

        static async Task XXX()
        {
            try
            {
                throw new Exception();
            }
            catch (Exception ex)
            {
                try
                {
                    Console.WriteLine(ex.ToString());
                }
                catch (TypeLoadException ex1)
                {
                    Console.WriteLine("ex.ToString():" + ex1.GetType() + "->" + ex1.Message);
                }
                try
                {
                    Console.WriteLine(Environment.StackTrace);
                }
                catch (TypeLoadException ex1)
                {
                    Console.WriteLine("Environment.StackTrace:" + ex1.GetType() + "->" + ex1.Message);
                }
            }

yyjdelete avatar Aug 23 '20 13:08 yyjdelete