[Proposal]: Interceptors

[RikkiGibson]

Interceptors

Feature specification has been moved to https://github.com/dotnet/roslyn/blob/main/docs/features/interceptors.md. As this is not currently implemented as a language feature, we will not be tracking specific status pieces on this issue.

Design meetings

• https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-02-27.md
• https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-07-12.md#interceptors
• https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-07-24.md#interceptors

[CyrusNajmabadi]

Can you spec out the paths, esp vis a vis unix vs windows paths? Specifically, this seems problematic:

File paths used in [InterceptsLocation] must exactly match the paths on the syntax nodes by ordinal comparison

Unless the compiler is already normalizing these paths across platform.

[CyrusNajmabadi]

public static void InterceptorMethod(this C c, int param)

I worry we're doing this at the wrong time. This only seems to be for methods, when we might want it for properties, etc. should this wait for 'extensions' to be done first?

[CyrusNajmabadi]

Constructors are a bit of a sore point for the Regex scenario. It would be nice to be able to intercept new Regex("..."), but it would be strange for an object-creation expression to return an instance of a subtype, for example, even if the expression's static type is unchanged.

I don't see this as any morally wrong versus calling one method and another being called. :)

[CyrusNajmabadi]

takes a qualified method name. For example, for the method bool System.Text.RegularExpressions.Regex.IsMatch(string), we would use the System.Text.RegularExpressions.Regex.IsMatch.

Would prefer that this break up the type-name and member-name into separate pieces.

[HaloFour]

There's a lot to digest here but I'm trying to reconcile how this proposal compares to the AOP scenarios that I often face. At first glance this seems really limited, especially if only one interception can happen at a time. It also seems that the interception cannot "wrap" the target, at least not unless the target is itself accessible from the interceptor. There's no access to the underlying joinpoint, which would make aspects like logging, tracing, transactions, bulkheading, etc. possible. This seems to be very specialized around the specialization of framework code.

I have other concerns about how this is very much spooky action at a distance, as in looking at a PR you can no longer tell if two identical method invocations back to back actually call the same method. There's also the fragility that if the generator doesn't run the interception becomes broken.

[yaakov-h]

To me this feels extremely limited and niche, and at the same time introduces a lot of moving parts that need to be kept in sync.

There also doesn't seem to be a way to opt in or out of it for different invocations, the only construct I can think of would be comments like the /* lang=regex */ syntax highlighting comments, which could be hard to read and understand with sequential invocations like builder objects.

The obvious question I have though is how would this work with default parameters? Would the interceptor need to know the default value and mirror it, or would that be passed through even though the caller did not specify it?

[alrz]

If I read this correctly I think this is basically replace/original on the call-site. There's https://github.com/dotnet/csharplang/issues/3992 to help with that but this doesn't seem to make it easier?

It seems possible to implement a version of replace/original using interceptors

You can already do this (sort of):

[AddLogging]
public partial void Method();
private void Method_Impl() { .. }

I think it comes down to a not-so-hacky way to do either of those, interceptors is definitely an option but still kind of hacky IMO.

The first case is like an overload sensitive to constant arguments. That's pure/constexpr which can also interact with generators:

 // could evaluate to 3 at compile-time, provided all arguments are constant and `Add` is pure.
var result = Add(1, 2);

// could evaluate the argument at compile-time provided it's a constant, using source generators.
var result = Evaluate(expressoin: "1+2"); 
var result = IsMatch(str, pattern: "\d+"); 

// generate a specialized IsMatch for each constant pattern
partial bool IsMatch(string, const string pattern)

The second case is kind of like a horizontal new:

extension for C {
  public new void Method() { .. } // preferred over the original C.Method
}

I've filed https://github.com/dotnet/roslyn/issues/20631 for this a while back - right now it can be silently ignored.

[orthoxerox]

The biggest drawback is probably spooky action at a distance. I understand that by marking my method interceptable I acquiesce to having my logic replaced, but now the user of my library Foo can reference another library Bar that silently generates interceptors for my method that do whatever they want.

[TheUnlocked]

We still think it's important that this works by adding source code to the compilation, and not by exposing source generator APIs that modify the object model of the compilation in any way. It is a goal that you can build a project with source generators, write all the generator outputs to disk, delete the generators, and then build again and have the same behavior.

I don't necessarily agree that the first sentence is the best way to achieve the goal in the second sentence. If there was an API to make direct edits, I would imagine that the compiler could still generate some kind of serialized patch containing those edits which it could save to disk and read later. While that would be pretty fragile in the event of source changes, it wouldn't be any more fragile than referencing a character offset, and it might avoid needing to re-parse/bind everything in the (probably more typical) case when you aren't saving/reading the modifications to/from disk.

[HaloFour]

The more I think about this proposal the less I think it addresses more general-purpose AOP scenarios. I use AOP heavily in my Java services. We routinely use aspects for cross cutting concerns. Those concerns include tracing, logging, metrics, bulk heading, circuit breaking, concurrent call rates, retries (with exponential backoff), memoization, transactions and declarative security. I've also written more specific aspects to facilitate instrumentation of third-party libraries. This interceptor solution may cover some of those cases, but even then it seems it would be a non-ideal way to accomplish it. It doesn't handle other commonly requested scenarios at all, such as IPNC, since the interception happens at the call-site. There's nothing wrong with call-site AOP, but it's only one flavor and can only handle certain use cases. Worse, this proposal explicitly specifies that multiple interceptors aren't legal, but that's an incredibly common case in my code.

I honestly don't see what use cases this proposal is intended to satisfy, except the extraordinarily narrow case of allowing ASP.NET to replace user-written code with their own.

It's also been suggested that this is an appropriate route to implement optimization, where runtime-provided source generators will rewrite some percentage of the user-written code with more runtime-optimized (but certainly less readable) versions. That feels 100% backwards to me, and still very narrow. Not to mention, such "optimizations" can only ever benefit people using newer versions of C# with support for these intercepting generators. Every other language is left out.

Coming from languages where post-processing and byte-code rewriting are a normal thing, this is extremely bizarre to suggest as a language enhancement.

[teo-tsirpanis]

How would [InterceptsLocation] accepting strings as file names interact with dotnet/roslyn#57239 when/if it is implemented? Wouldn't it remove the guarantee that file names are unique?

[RikkiGibson]

Thank you for the feedback so far.

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1446903296

Can you spec out the paths, esp vis a vis unix vs windows paths?

Note that the compiler doesn't require paths to have any particular form. They just have to be representable in a .NET string.

You can really think of this as: imagine the method we are calling has a CallerFilePath parameter. What string would we pass there? The string we put in the InterceptsLocationAttribute needs to exactly match that string in order to intercept that call. If the implicit CallerFilePath string argument differs based on which environment we are compiling in, then the argument to InterceptsLocation needs to change in the exact same way.

This is very problematic for any manually-written InterceptsLocationAttribute--it's automatically non-portable, unless very specific pathmap arguments are used--for example, have no idea if the right thing will just start happening if you pathmap \ to /. Maybe, maybe not. It starts to feel like a huge hack to make it work at all.

I worry we're doing this at the wrong time. This only seems to be for methods, when we might want it for properties, etc. should this wait for 'extensions' to be done first?

Great question. It does feel like if we wanted to do this, it should be possible to do it with pretty much any non-type member, unless we had a specific reason for leaving them out. This tends to raise the question about how to denote the "location" of a usage of any kind of member, in a way that is not ambiguous with the usage of any other member. For example, use of implicit conversion operators might be tough to intercept based on location.

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1446941089 I wasn't sure what you meant by underlying joinpoint.

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1448998072 I wasn't sure what IPNC means.

That feels 100% backwards to me, and still very narrow. Not to mention, such "optimizations" can only ever benefit people using newer versions of C# with support for these intercepting generators. Every other language is left out.

I definitely want us to consider if the use case of the platform specializing calls to platform methods within a project is better served by an post-compilation IL rewriting step. That will require revisiting any reasons we've been reluctant to introduce such a step in the past.

It's clear that disallowing composition of multiple interceptors is really limiting here. As we look a little more broadly at this space I would definitely like for us to consider what it would mean to allow composition. The sequencing definitely seems easiest to define when you also control the original definition of the thing whose calls will get replaced.

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1447340973

There also doesn't seem to be a way to opt in or out of it for different invocations

I think this would require the "syntactic marker at interception site".

The obvious question I have though is how would this work with default parameters? Would the interceptor need to know the default value and mirror it, or would that be passed through even though the caller did not specify it?

The interceptor would not need to know or mirror the default value, but it would need to have a parameter corresponding to that optional parameter--just as we disallow implicitly converting the method group of void M(string x, string y = "a") { } to Action<string>, for example. In fact, it might be bad for interceptors to use default values--since we don't want to bind the original call site any differently, we're likely to want to spec this to not even insert any default values which are specified by the interceptor.

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1454902160

Wouldn't it remove the guarantee that file names are unique?

Although it's hard to have duplicate file names in a real world project, there's no guarantee of unique paths in the compiler itself. (This was a significant complicating factor for file-local types.) Most likely, if a single tuple of (filePath, line, column) does not refer to exactly one source location in the compilation, it needs to be an error to try and intercept the location. This could be problematic for intercepting calls in generated code.

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1448046520

The biggest drawback is probably spooky action at a distance. I understand that by marking my method interceptable I acquiesce to having my logic replaced, but now the user of my library Foo can reference another library Bar that silently generates interceptors for my method that do whatever they want.

Yeah, we would potentially want something like "Key tokens" listed under "Alternatives" to reduce the risk of this.

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1447830935

The second case is kind of like a horizontal new

I think this is a great point. I would like to try looking at the problem from a different angle, where we consider if some controlled facility could be introduced to make certain extensions better matches than instance methods in some cases. Could this give us some of the benefits of replace/original while still being able to work across projects--that is, where the replacement isn't in the same project as the original.

I don't yet know in which cases this would be or what drawbacks this would have. There could be some fairly spooky problems where the replacement method wouldn't actually get used in places where we want it to.

What I am seeing is that some scenarios like ASP.NET minimal API or query providers want to dispatch based on location essentially because some information about lambda expressions is lost after compilation or requires reflection or complex traversal to access at runtime. But other places want to dispatch based on:

• the value of a string argument, like Regex,
• a System.Type or the 'typeof' a type parameter, like serialization and dependency injection
• nothing in particular! Vectorization for example would probably be simpler to handle with a replace/original approach, since it is using tests which apply to the entire runtime environment to decide whether/how to specialize.

Basically, what if a specialized implementation of a method could be added to a project, which dispatches however the implementer wants, simply by using the flow-control constructs of the language, rather than sticking declarative references to source locations into attributes on the signature.

Consider the following extremely strawman sample, which hopefully gets the basic idea across.

IRouteBuilder builder = ...;
// optionally: "conversion" to an explicit-extension type? where the framework exposes some path for doing this.
SpecializedRouteBuilder builder1 = builder.WithOpportunisticSpecialization();
builder1.MapGet("/products", (request, response) => ...);
builder1.MapGet("/items", (request, response) => ...);
builder1.MapGet("/config", (request, response) => ...);

// WebApp.generated.cs
// make it an optimization problem?
explicit extension SpecializedRouteBuilder for IRouteBuilder
{
    // add an extension which deliberately wins over the one actually present in the type?
    public shadowing IRouteBuilder MapGet(string route, Delegate handler,
        [CallerFilePath] string filePath = null,
        [CallerLineNumber] int lineNumber = -1,
        [CallerColumnNumber] int columnNumber = -1)
    {
        switch (filePath, lineNumber, columnNumber)
        {
            // assume Native AOT can inline and constant-propagate to ensure
            // this dispatch doesn't actually need to happen at runtime in most cases.
            case ("file.cs", 1, 1): Method1(route, handler); return this;
            case ("file.cs", 2, 2): Method2(route, handler); return this;
            case ("file.cs", 3, 3): Method3(route, handler); return this;
            case ("file.cs", 4, 4): Method4(route, handler); return this;
            case ("file.cs", 5, 5): Method5(route, handler); return this;
            case ("file.cs", 6, 6): Method6(route, handler); return this;
            default: base.MapGet(route, handler);
        }
    }
}

// Regex.generated.cs
explicit extension MyRegexFactory for Regex
{
    // add an extension which deliberately wins over the one actually present in the type?
    public static shadowing Regex IsMatch(string pattern)
    {
        switch (pattern)
        {
            case ("a+"): return APlusMatcher();
            case ("ab+"): return ABPlusMatcher();
            default: return base.IsMatch(pattern);
        }
    }
}

[vladd]

@RikkiGibson

I wasn't sure what IPNC means.

I think it’s a common desire to decorate a property in some way and get the boilerplate for implementing INotifyPropertyChange automagically applied. This scenario is not supported by current source generators.

[alrz]

There could be some fairly spooky problems where the replacement method wouldn't actually get used in places where we want it to.

If I understood the extension proposal correctly, an implicit extension always applies? a la https://github.com/dotnet/csharplang/issues/4029

What I am seeing is that some scenarios like ASP.NET minimal API or query providers want to dispatch based on location

I don't think merely the "location" is the defining factor here (just like the IsMatch case), I would expect any c.InterceptableMethod(1) dispatches to the same generated code if the argument is the same.

Of course, this isn't workable for typeof, lambdas or generics (basically anything that doesn't have a const representation).

For generics, it could be a case for specialization:

extension for Serializer {
    // `new` makes sure that this is specialization/shadowing so the signature must match.
    public static new string Serialize</*const*/ MyType>() { .. } 
}

Which can be enabled for constants as well:

extension for Regex {
    public static new bool IsMatch(string str, const string pattern = "abc") { .. }
}

These would result in a separate method and eliminate the need for a switch.

None of that would work for lambdas but I think it should remain an implementation detail, that is, to use some auxiliary APIs to help with generating the switch and getting the "concrete" value (typeof, lambda body, etc) for the generator to use directly.

[HaloFour]

@RikkiGibson

I definitely want us to consider if the use case of the platform specializing calls to platform methods within a project is better served by an post-compilation IL rewriting step. That will require revisiting any reasons we've been reluctant to introduce such a step in the past.

It was mentioned on Discord that this is because IL-rewriting is challenging and the tools aren't up to snuff, and I don't doubt it. Regardless of these use cases I think there's a good opportunity for the .NET teams to come together and provide such tooling, that can emit appropriate IL and debugging symbols. For the most part, writing transformers and aspects In the Java world is a pretty simple experience. It's extremely rare to emit raw byte code, you typically write normal Java classes to describe the advice and use either annotations or a fluent DSL to describe where that advice will be applied. Tooling in IDEs like IntelliJ will show you where the advice will be applied. Transformation can then be performed as a post-compilation step, prior to a native/AOT build. It's pretty slick and easy to use once you grok the concepts of AOP and interception. I would imagine an official solution for .NET, with tooling support, could very easily rival it, too.

[RikkiGibson]

https://github.com/dotnet/csharplang/issues/7009#issuecomment-1461603069

If I understood the extension proposal correctly, an implicit extension always applies?

I think if the receiver of the member access is the extension type, then members on the extension type are a better match than members on the underlying type. But if the receiver is the underlying type of an implicit extension, then the implicit extension members are only considered after we try and fail to find an applicable member on the underlying type. e.g.

var mt = new MyType();
mt.M(); // MyType.M();
mt.M1(); // Ex.M();

Ex ex = mt;
ex.M(); // Ex.M();

public class MyType
{
    public void M() { }
}

implicit extension Ex for MyType
{
    public void M() { }
    public void M1() { }
}

[alrz]

The biggest drawback is probably spooky action at a distance. I understand that by marking my method interceptable I acquiesce to having my logic replaced, but now the user of my library Foo can reference another library Bar that silently generates interceptors for my method that do whatever they want.

I think to account for that it could be required to add specializations/interceptors for a particular member only in a single extension declaration in addition to a marker on the original member.

class C {
  public replaceable void Method() { .. } // permits replacement by a generator
}
extension for C {
  public replace void Method() { .. } // but if this already exists, no one else can replace it
}

That's assuming an external replacement could be unambiguous to begin with.

[jnm2]

Please please please call this ComeFromAttribute. The opportunity is too perfect! I'm only half kidding.

[RikkiGibson]

I think to account for that it could be required to add specializations/interceptors for a particular member only in a single extension declaration in addition to a marker on the original member.

This is quite a bit like what is described in #7061, except that we were considering only letting extensions get shadowed/replaced by extensions. Instead of a replaceable or similar marker, the signal to say "let this get replaced" is just to declare it as an extension.

[aetos382]

Is it considered that the interceptor receives a reference to the original method through the Action or Func?

class Original
{
    public void Caller()
    {
        Console.WriteLine(this.Interceptable1(1)); // prints '6'
    }

    [Interceptable]
    public int Interceptable1(int x)
    {
        return x + 1;
    }
}

class Interceptor
{
    [InterceptsLocation(...)]
    public int Intercepter1(int x, Func<int, int> original)
    {
        return original(x + 1) * 2;
    }
}

[gfraiteur]

This feature seems to address a very specific problem with a very specific solution. While this may be effective in the cases at hand, it may also be problematic in the long run, as the C# compiler is expected to last many more decades.

Instead of creating a highly specialized solution at the compiler level, it may be worth considering a more general and flexible approach based on aspect-oriented programming (AOP). AOP has been used successfully in various domains for over 20 years and offers many benefits for modularizing cross-cutting concerns.

At PostSharp Technologies, we have been developing Metalama, an AOP framework based on Roslyn. Metalama still does not support call-site aspects (i.e., interceptors), but we have plans to implement them in the future. We believe that Metalama could meet the requirements of this feature proposal and provide a more elegant and robust solution.

We would be happy to collaborate with you on this matter and share our expertise and insights.

[ufcpp]

Does this work safely even if the code-cleanup automatically replaces spaces with tabs or vice versa?

[CyrusNajmabadi]

@ufcpp yes :-)

[Bonuspunkt]

what did i miss?

using System;
using System.Runtime.CompilerServices;

var c = new C();
c.InterceptableMethod(1); // prints `interceptor 1`
c.InterceptableMethod(1); // prints `other interceptor 1`
c.InterceptableMethod(1); // prints `interceptable 1`

class C
{
    public static Action<int> Interceptable { get;set; } = (param) => Console.WriteLine($"interceptable {param}");

    public Action<int> InterceptableMethod { get; set; } = Interceptable;
}

static class D
{
    [ModuleInitializer]
    public static void Init() {

        Func<Action<int>> gen = () => {
            var originalMethod = C.Interceptable;

            var actions = new Action<int>[] {
                param => Console.WriteLine($"interceptor {param}"),
                param => Console.WriteLine($"other interceptor {param}"),
                originalMethod
            };
            var index = 0;

            return (param) => {
                actions[index](param);
                if (index < actions.Length - 1) index++;
            };
        };

        C.Interceptable = gen();
    }
}

[LazyAnnoyingStupidIdiot]

This feature seems to address a very specific problem with a very specific solution. While this may be effective in the cases at hand, it may also be problematic in the long run, as the C# compiler is expected to last many more decades.

Instead of creating a highly specialized solution at the compiler level, it may be worth considering a more general and flexible approach based on aspect-oriented programming (AOP). AOP has been used successfully in various domains for over 20 years and offers many benefits for modularizing cross-cutting concerns.

At PostSharp Technologies, we have been developing Metalama, an AOP framework based on Roslyn. Metalama still does not support call-site aspects (i.e., interceptors), but we have plans to implement them in the future. We believe that Metalama could meet the requirements of this feature proposal and provide a more elegant and robust solution.

We would be happy to collaborate with you on this matter and share our expertise and insights.

And is PostSharp Technology going to put the AOP features into C# as a core feature set? Or should your link have went to the pricing page?

https://www.postsharp.net/metalama/pricing

Until that happens, it is not a solution worth mentioning here on the Core DotNet issue trackers. What's next? We should be paying some third party vendors for basic language features like generics / expression tree / etc?

Or is it dotnet team's position to have gaps in the language feature set so third party vendors can profit off it?

Be sure to let us all know either way.

This is a specific solution to a specific problem many of us here face, and having this implemented will solve many of our specific problems.

[HaloFour]

@LazyAnnoyingStupidIdiot

And is PostSharp Technology going to put the AOP features into C# as a core feature set? Or should your link have went to the pricing page?

I believe the point here is that a proper AOP solution will cover not only this use case but many, many others that interceptors cannot handle at all. Obviously any solution that ends up built into the language/compiler will not require developers to license/purchase third-party components.

[mgravell]

One potentially simplifying alternative here might be to disallow type parameters on interceptor methods, and force them to specialize based on the constructed signature of the interceptable method.

I'm not sure that's possible; to modify the example immediately preceding this statement:

class C<T>
{
    private T someField;
    public static void Use(...)
    {
        var x1 = something./*L1*/Interceptable("a", someField);
        var x2 = something./*L2*/Interceptable("a", someField);
    }
}

An interceptor cannot magic away the fact that there's an unresolved <T> in the constructed signature, unless the InterceptsLocation additionally allows types to be specified, and would also require JIT interception and per-type JIT (even for ref-type T). There's also the issue that if C<T> is public in library B, the generator here needs to run against B since it is B's calls that are being intercepted, but the final types aren't known until some consuming library/application A defines what C<ConcreteFoo> it actually wants.

[mgravell]

One additional thought - "unpronounceable types"; this would be an enticing API for libraries like Dapper to embrace AOT, but a typical Dapper usage today might look like:

connection.Execute("some sql", new { Foo = 12, Bar = "abc" });

it is not possible to convey this in an interceptor because the type with the parameters is unpronounceable; even if we took object or T as the arg, we can't cast it to get the values out, for similar reasons. There are some ugly workarounds, but... nothing good.

Obviously one alternative would be for us to push an analyzer that replaces the above with value-tuple syntax, i.e.

connection.Execute("some sql", (Foo: 12, Bar: "abc"));

which would allow us to write an interceptor:

[InterceptsLocation(...)]
static int GeneratedExecute(this IDbConnection connection, string sql, (int Foo, string Bar) args)
{       // don't over-analyze this code - just for illustration
        using var cmd = conn.CreateCommand();
        cmd.CommandType = CommandType.Text;
        cmd.CommandText = "some sql"; // not quite always exactly the same as original input

        var p = cmd.CreateParameter();
        p.ParameterName = "Foo";
        p.DbType = DbType.Int32;
        p.Value = args.Foo;
        p.Direction = ParameterDirection.Input;
        cmd.Parameters.Add(p);

        p = cmd.CreateParameter();
        p.ParameterName = "Bar";
        p.DbType = DbType.String;
        p.Value = args.Bar ?? (object)DBNull.Value;
        p.Direction = ParameterDirection.Input;
        cmd.Parameters.Add(p);

        return cmd.ExecuteNonQuery();
}

However, this then gets into a super confusing situation, because if someone uses value-tuple syntax in a scenario that can't use generators for some reason, then nothing works - because value-tuples cannot convey the necessary name information inwards (only outwards).

I wonder, then, if there's some missing feature related to interceptors that makes unpronounceable types: pronounceable. I'm not suggesting such types should be pronounceable between modules/assemblies, but... here me out:

public static int Execute(this IDbConnection conn, string sql, { int Foo, string Bar } args)
{
    // everything else as above
}

what if "named value-tuple type declaration syntax", but using { } instead of ( ) worked for anonymous types? I appreciate this is a bit tangential, but IMO it interweaves the two.

Oh, one other subtle difference to consider between named value-tuples and anon-types (for why we can't just use value-tuples):

var good = new { One = 1 }; // presumably maps to imaginary structural type syntax `{ int One }`
var bad = (One: 1); // CS8124 Tuple must contain at least two elements.

[mgravell]

Just for completeness, here's one of the evil hacks, just to be honest and say "yes, we can get around this":

    connection.Execute("some sql", new { Foo = 12, Bar = "abc" });


    [InterceptsLocation(...)]
    static int GeneratedExecute(this IDbConnection conn, string sql, object args)
    {
        return TypeHack(conn, sql, args,
            static () => new { Foo = default(int), Bar = default(string) },
            static obj => (obj.Foo, obj.Bar));
        static int TypeHack<T>(IDbConnection conn, string sql, object rawArgs,
            Func<T> unused, Func<T, (int Foo, string Bar)> argsFactory)
        {
            var args = argsFactory((T)rawArgs);
            using var cmd = conn.CreateCommand();
            cmd.CommandType = CommandType.Text;
            cmd.CommandText = "some sql";

            var p = cmd.CreateParameter();
            p.ParameterName = "Foo";
            p.DbType = DbType.Int32;
            p.Value = args.Foo;
            p.Direction = ParameterDirection.Input;
            cmd.Parameters.Add(p);

            p = cmd.CreateParameter();
            p.ParameterName = "Bar";
            p.DbType = DbType.String;
            p.Value = args.Bar ?? (object)DBNull.Value;
            p.Direction = ParameterDirection.Input;
            cmd.Parameters.Add(p);

            return cmd.ExecuteNonQuery();
        }
    }

[mgravell]

I've been playing with this locally, and the more I do: the more I'm concerned by the security implications of arbitrary interception. Especially when aspnetcore is looking at generators/interceptors, meaning any global flag would probably be on in a good number of cases.

It seems to me that there are two categories of interception:

• things that are designed to expect interception - in which case the [Interceptable] makes perfect sense
• ad-hoc

Right now, the "ad-hoc" is the implemented case, but it seems like the window for abuse (by injecting malicious generators into existing libraries as a supply-chain attack) can be kept minimal by making only advertised [Interceptable] locations interceptable by default. In reality, I expect most common interceptor cases to be fairly predictable at the library level over which methods need to be intercepted, with the same team writing the library and the interceptor (so: able to add attributes).

If there is a good use-case for ad-hoc interception, IMO that should be a separate flag with a suitable name citing "SomethingDangerousAdhocInterceptorsSomething" (naming is hard). Today a malicious generator could emot a module initializer or static constructor in a private class to do some things, but it can't change the flow of existing code, log per-call arguments, etc.