Skip to content

invocation.MethodInvocationTarget throws ArgumentNullException for default interface method #684

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

Open
stakx opened this issue Aug 30, 2024 · 3 comments
Assignees
Labels
Milestone

Comments

@stakx
Copy link
Member

stakx commented Aug 30, 2024

Repro code

var generator = new ProxyGenerator();
var foo = generator.CreateInterfaceProxyWithoutTarget<IFoo>(new InspectInvocation());
foo.Bar();

public interface IFoo
{
    void Bar() { }
}

public class InspectInvocation : IInterceptor
{
    public void Intercept(IInvocation invocation)
    {
        Debug.Assert(invocation.InvocationTarget == null || invocation.MethodInvocationTarget != null);  // will throw
    }
}

Expected outcome

Unless I misunderstand the meaning of IInvocation.InvocationTarget and IInvocation.MethodInvocationTarget, I would expect the latter to point to a method of the former's type, if the former (IInvocation.InvocationTarget) is non-null.

Actual outcome

MethodInvocationTarget will throw an ArgumentNullException due to the invocation's targetType not being set.

Affected version(s)

Castle.Core starting at tag v5.2.0

/cc @dtchepak @zvirja @alexandrnikitin – this will likely matter for NSubstitute (in https://github.com/nsubstitute/NSubstitute/blob/4d258a28aba054ea18785d36b4bdd83da023aefb/src/NSubstitute/Proxies/CastleDynamicProxy/CastleInvocationMapper.cs#L11-L16) once we've published version 5.2.0 and you upgrade to that version.

@stakx stakx added the bug label Aug 30, 2024
@stakx stakx added this to the vNext milestone Aug 30, 2024
@stakx
Copy link
Member Author

stakx commented Aug 30, 2024

If I understand things correctly, then generally speaking the "target" is whatever one can .Proceed() to:

  • For composition-based proxies, a target object can be supplied explicitly.
  • For inheritance-based proxies, the target can be either missing (i. e. null) or the intercepted method's implementation inherited from the proxied type.

If the above assumptions are correct, then for an IInvocation representing an intercepted default interface method...:

  • invocation.TargetType should return the interface type where the default implementation resides.
  • invocation.InvocationTarget should return the same as invocation.Proxy.
  • invocation.MethodInvocationTarget should return the MethodInfo of the method with the default implementation... so possibly the same as invocation.Method?

@jonorossi, sorry to bother you again, but are my assumptions and conclusions correct, or am I missing some finer distinctions?

@304NotModified
Copy link

304NotModified commented Mar 10, 2025

FYI I tested this with NSubstitute 5.3.0 and Castle.Core 5.1.1 and 5.2.1

Tested with the following code:

    [Fact]
    public void TestDefaultInterfaceImpl1()
    {
        // Arrange
        var for1 = Substitute.For<IMyInterface>();

        // Act
        var message = for1.GetText();

        // Assert
        message.Should().Be("abc");
    }

    [Fact]
    public void TestDefaultInterfaceImpl2()
    {
        // Arrange
        var for1 = Substitute.For<IMyInterface>();
        for1.GetText().Returns("abcd");

        // Act
        var message = for1.GetText();

        // Assert
        message.Should().Be("abcd");
    }

    public interface IMyInterface
    {
        string GetText()
        {
            return "abc";
        }
    }

Castle.Core 5.1.1:

  • Test TestDefaultInterfaceImpl1 assert fails: Expected message to be "abc" with a length of 3, but "" has a length of 0, differs near "" (index 0).
  • Test TestDefaultInterfaceImpl2 succeeds

Castle.Core 5.2.1

Both tests fail with:

System.ArgumentNullException
Value cannot be null. (Parameter 'type')
at Castle.DynamicProxy.Internal.InvocationHelper.GetMethodOnType(Type type, MethodInfo proxiedMethod)
at Castle.DynamicProxy.Internal.InheritanceInvocation.get_MethodInvocationTarget()
at NSubstitute.Proxies.CastleDynamicProxy.CastleInvocationMapper.Map(IInvocation castleInvocation)
at NSubstitute.Proxies.CastleDynamicProxy.CastleForwardingInterceptor.Intercept(IInvocation invocation)
at Castle.DynamicProxy.AbstractInvocation.Proceed()
at NSubstitute.Proxies.CastleDynamicProxy.ProxyIdInterceptor.Intercept(IInvocation invocation)
at Castle.DynamicProxy.AbstractInvocation.Proceed()
at Castle.Proxies.ObjectProxy.GetText()

Remarks

If we could do something in NSubstitute to fix this, please let me know.

If the above assumptions are correct, then for an IInvocation representing an intercepted default interface method...:

  • invocation.TargetType should return the interface type where the default implementation resides.
  • invocation.InvocationTarget should return the same as invocation.Proxy.
  • invocation.MethodInvocationTarget should return the MethodInfo of the method with the default implementation... so possibly the same as invocation.Method?

I'm not sure as I don't know the code base that well, but it sounds correct to me

@stakx
Copy link
Member Author

stakx commented Mar 10, 2025

@304NotModified, thanks for adding some more test cases to this issue, plus thinking about the meaning of those invocation properties. I'll see what I can do... but it might take a while (hopefully not another 6 months though...).

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

2 participants