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

Proxying certain [Serializable] classes produces proxy types that fail PEVerify test #367

Closed
stakx opened this issue Jun 1, 2018 · 1 comment · Fixed by #554
Closed
Assignees
Labels
Milestone

Comments

@stakx
Copy link
Member

stakx commented Jun 1, 2018

Using Castle Core 4.2.1, proxying the following type (or System.Security.Claims.ClaimsIdentity, if you want a more realistic example) produces a proxy class with incorrect IL:

[Serializable]
public class BadSerializable
{
    public BadSerializable() { }
    public BadSerializable(SerializationInfo info, StreamingContext context) { }
    public virtual void GetObjectData(SerializationInfo info, StreamingContext context) { }
}

generator.CreateClassProxy<BadSerializable>(...);

When running PEVerify for the dynamically generated assembly, it complains about duplicate methods:

[MD]: Error: Method has a duplicate, token=0x06000004. [token:0x06000003]
[MD]: Error: Method has a duplicate, token=0x06000003. [token:0x06000004]
2 Error(s) Verifying Castle.Core\src\Castle.Core.Tests\bin\Debug\net461\CastleDynProxy2.dll

The mentioned methods belong to the generated proxy class BadSerializableProxy.

Below you'll find the two methods as ILSpy decompiles them back to C#:

// Castle.Proxies.BadSerializableProxy (method token 06000003):
public override void GetObjectData(SerializationInfo info, StreamingContext context)
{
    BadSerializable_GetObjectData badSerializable_GetObjectData = new BadSerializable_GetObjectData(typeof(SerializableClassTestCase.BadSerializable), this, this.__interceptors, BadSerializableProxy.token_GetObjectData, new object[]
    {
        info,
        context
    });
    badSerializable_GetObjectData.Proceed();
}

// Castle.Proxies.BadSerializableProxy (method token 06000004):
public override void GetObjectData(SerializationInfo serializationInfo, StreamingContext streamingContext)
{
    Type type = Type.GetType("Castle.DynamicProxy.Serialization.ProxyObjectReference, Castle.Core, Version=0.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc", true, false);
    serializationInfo.SetType(type);
    serializationInfo.AddValue("__interceptors", this.__interceptors);
    string[] value = new string[0];
    serializationInfo.AddValue("__interfaces", value);
    serializationInfo.AddValue("__baseType", "Castle.DynamicProxy.Tests.SerializableClassTestCase+BadSerializable, Castle.Core.Tests, Version=0.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc");
    serializationInfo.AddValue("__proxyGenerationOptions", BadSerializableProxy.proxyGenerationOptions);
    serializationInfo.AddValue("__proxyTypeId", "class");
    serializationInfo.AddValue("__delegateToBase", false);
    MemberInfo[] array = FormatterServices.GetSerializableMembers(typeof(SerializableClassTestCase.BadSerializable));
    array = TypeUtil.Sort(array);
    object[] objectData = FormatterServices.GetObjectData(this, array);
    serializationInfo.AddValue("__data", objectData);
}

This error does not happen if either:

  • the class is not decorated with [Serializable]
  • implements the ISerializable interface
  • GetObjectData is not declared virtual
@stakx stakx changed the title Proxying incorrectly implemented [Serializable] class produces type that fails PEVerify test Proxying certain [Serializable] classes produces proxy types that fail PEVerify test Jun 1, 2018
@jonorossi jonorossi added the bug label Jun 1, 2018
@stakx
Copy link
Member Author

stakx commented Jun 3, 2018

I've been looking into this and there appears to be several questionable things going on here:

  • If the type that is to be proxied is marked [Serializable], DynamicProxy will automatically add ISerializable to the list of implemented interfaces (even if the proxied type does not implement it).

  • DynamicProxy will automatically create a GetObjectData method unless the type to be proxied implements ISerializable. That is, if the proxied type doesn't implement the interface but has the method, you potentially get the duplicate method problem.

  • When supplying its own GetObjectData, DynamicProxy uses certain default method modifiers. If there happens to be a GetObjectData method in the base type that is sealed, we get another conflict.

  • I also suspect that in the presence of GetObjectData in the base class, the newly auto-implemented method might not call its base. (But I haven't verified this claim yet.)

Fixing the reported issue likely implies that the whole [Serializable] code needs to be reworked. This is going to take a while so it shouldn't stop the 4.3.0 release at this time.

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