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

ProxyUtil.AreInternalsVisibleToDynamicProxy does not take into account target assembly's strong-name status #488

Open
stakx opened this issue Apr 3, 2020 · 9 comments
Labels

Comments

@stakx
Copy link
Member

stakx commented Apr 3, 2020

@GeeWee posted a bug report at devlooped/moq#991 which basically boils down to this:

using Azure.Storage.Blobs.Specialized;  // from NuGet package 'Azure.Storage.Blobs'

using Castle.DynamicProxy;

public class Bork : BlockBlobClient { }

class Program
{
    static void Main()
    {
        var generator = new ProxyGenerator();
        _ = generator.CreateClassProxy<BlockBlobClient>();  // works
        _ = generator.CreateClassProxy<Bork>();  // fails
    }
}

Proxying BlockBlobClient appears to work fine; proxying an empty subclass of it throws the following exception:

System.TypeLoadException: Method 'UploadInternal' on type 'Castle.Proxies.BorkProxy' from assembly 'DynamicProxyGenAssembly2, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' is overriding a method that is not visible from that assembly.
   at System.Reflection.Emit.TypeBuilder.TermCreateClass(RuntimeModule module, Int32 tk, ObjectHandleOnStack type)
   at System.Reflection.Emit.TypeBuilder.CreateTypeNoLock()
   at System.Reflection.Emit.TypeBuilder.CreateTypeInfo()
   at Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.CreateType(TypeBuilder type)
   at Castle.DynamicProxy.Generators.Emitters.AbstractTypeEmitter.BuildType()
   at Castle.DynamicProxy.Generators.ClassProxyGenerator.GenerateType(String name, Type[] interfaces, INamingScope namingScope)
   at Castle.Core.Internal.SynchronizedDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 factory)
   at Castle.DynamicProxy.ProxyGenerator.CreateClassProxy(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, Object[] constructorArguments, IInterceptor[] interceptors)
   at Castle.DynamicProxy.ProxyGenerator.CreateClassProxy[TClass](IInterceptor[] interceptors)
   at Program.Main()

(It may or may not be relevant that PEVerify reports a bunch of invalid signatures in the netstandard2.0 DLL for that package:

[MD]: Error: Token 0x020000da following ELEMENT_TYPE_CLASS (_VALUETYPE) in signature is a ValueType (Class,respectively). [token:0x04000064]
[MD]: Error: Token 0x0200002c following ELEMENT_TYPE_CLASS (_VALUETYPE) in signature is a ValueType (Class,respectively). [token:0x0400006A]
[MD]: Error: Token 0x0200002c following ELEMENT_TYPE_CLASS (_VALUETYPE) in signature is a ValueType (Class,respectively). [token:0x0400006B]
... (a lot more of the same)
@stakx
Copy link
Member Author

stakx commented Apr 3, 2020

I just noticed two more things:

  1. The problem goes away when strong-naming one's own DLL (which contains the test code above).

  2. Azure.Storage.Blobs.dll carries an [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024...5cc7")] attribute. It also has a similar attribute for its own test project. (See their AssemblyInfo.cs.) This might be an error, those attributes should perhaps only be there in a debug build, but not in a release build. I'll ask them about that.

@stakx stakx changed the title Can proxy class Azure.Storage.Blobs.Specialized, but not empty subclass of it Can proxy class Azure.Storage.Blobs.Specialized.BlockBlobClient but not empty subclass of it Apr 3, 2020
@pakrym
Copy link

pakrym commented Apr 3, 2020

Isn't the problem that you are trying to generate a non strong named dynamic assembly DynamicProxyGenAssembly2 but for some reason generate overriding members for things that are not visible to it?

I think there might be a bug in

internal static bool AreInternalsVisibleToDynamicProxy(Assembly asm)
this logic. It should check if strongly typed signature is also the same not only the assembly name.

@stakx
Copy link
Member Author

stakx commented Apr 3, 2020

@pakrym, thanks for the hint. It's indeed possible that there is a bug in DynamicProxy... I'll look into it ASAP.

@stakx
Copy link
Member Author

stakx commented Apr 3, 2020

Ah yes, I think this explains it. What's happening in the above repro code (and in @GeeWee's) is this:

  1. DynamicProxy gets a request to proxy a type T from user assembly A. T inherits from TBase which sits in assembly B.
  2. Because assembly A is not strong-named, DynamicProxy chooses to put the proxy type for T (TProxy) in a dynamic assembly D that also isn't strong-named. This is to ensure that it may reference assembly A when generating class TProxy : T.
  3. Assembly B's [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=...")] attribute doesn't apply to D because the latter isn't strong-named. (B's internals would only be visible to D if the former had an [assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] attribute).
  4. However, due to the faulty logic pointed out above, DynamicProxy starts generating proxy type TProxy in D, and adds overrides for internal virtual methods from TBase. This triggers the error.

The ProxyUtil.AreInternalsVisibleToDynamicProxy method would additionally have to consider for which target ModuleBuilder the query is being made.

@stakx stakx added the bug label Apr 3, 2020
@stakx stakx changed the title Can proxy class Azure.Storage.Blobs.Specialized.BlockBlobClient but not empty subclass of it ProxyUtil.AreInternalsVisibleToDynamicProxy does not take into account target assembly's strong-name status Apr 3, 2020
@smokedlinq
Copy link

Is there a possible workaround for this? If not, what can I do to help move this forward to a resolution?

@qetza
Copy link

qetza commented Apr 6, 2022

Hello, any news on this issue?

I just stubble on this while creating a class inheriting from a BlobContainerClient for the Azure.Storage.Blobs and trying to create a mock based on my class. I can confirm that if a put a strong name on my assembly then mocking is working but this is not an option for us.

@jonorossi
Copy link
Member

Hello, any news on this issue?

There has been no fix made for this defect.

@thackman7
Copy link

It's been a year, any update on this defect. I'm also running into this issue while trying to mock a class inheriting from Azure DataLakeServiceClient. Same exact issue that the devs above are having with BlobContainerClient.

@stakx
Copy link
Member Author

stakx commented Apr 5, 2023

No, no update.

Perhaps a quick explanation why nothing has happened yet (beyond the obvious principal reason that simply noone has submitted a PR for it): compared to other open issues, this has a fairly low priority, because it doesn't affect many common use cases. Also, to get this to work requires an extensive refactoring of pretty much all of DynamicProxy's internal classes... I know because I made an attempt months ago. The necessary changes quickly grew to such proportions that I didn't feel comfortable with a PR at the time. The cost/benefit ratio seemed unfavourable. I deemed it a better idea to first simplify DynamicProxy's internals further, so that this could be solved with fewer code changes.

Don't get your hopes up that this will get resolved anytime in the near future. I suggest you try to find a workaround for the time being (possibly by overriding inherited internal members but keeping them overridable, and delegating to the inherited base methods).

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

No branches or pull requests

6 participants