Consider optimising exception handling of IAuthorizeDataPortal, IInterceptDataPortal and IDataPortalExceptionInspector

[TheCakeMonster]

The code we have added to the server-side data portal to enhance authorization, revalidation and exception sanitization uses exception throwing for control-of-flow in what might be considered expected circumstances. Exceptions are thrown and then recaught before being passed back to the client.

Throwing an exception is roughly 2 orders of magnitude slower than creating it and then passing it as a return value. As a result, using this exception handling technique ultimately limits the scalability of the data portal in any scenarios where those exceptions are being generated on a regular basis, such as if someone is attempting to brute force attack the server-side data portal. This could theoretically make DDoS attacks more likely to succeed.

Ultimately, DDoS attacks cannot be prevented in code - this is an something that has to be considered an infrastructure concern - but we can at least raise the bar as to the level of invalid request injection that could be handled by a finite amount of resources.

I suggest we change the interfaces to support return values rather than require that exceptions are thrown and then recaught. This is especially noticeable for IDataPortalExceptionInspector, as there is no way to get anything out of the single method it defines without throwing an exception.

Current interface definition:

public interface IDataPortalExceptionInspector
{
    void InspectException(Type objectType, object businessObject, object criteria, string methodName, Exception ex);
}

Suggested replacement:

public interface IDataPortalExceptionInspector
{
    Exception InspectException(Type objectType, object businessObject, object criteria, string methodName, Exception ex);
}

Alternative replacement:

public interface IDataPortalExceptionInspector
{
    void InspectException(Type objectType, object businessObject, object criteria, string methodName, out Exception ex);
}

Downsides For Consideration/Discussion
Altering the code in the way I describe is a breaking change to these existing, public interfaces. Anyone who has previously built their own implementations to make use of these extensibility points will have to change their code to cater for the change we would be making. As a result, this issue is being opened for discussion, rather than actioned in any way. We need to decide if the breaking change is worthwhile.

[rockfordlhotka]

One other aspect of this to consider, is that the throwing of exceptions is usually what triggers the abort (rollback) of any ongoing transaction. Technologies like ADO.NET, and even the old Enterprise Services (COM+) understand that an exception means a rollback is necessary.

So if we do away with exceptions, then we need a mechanism to (ideally automatically) trigger a transaction rollback. I doubt that's always possible, because we don't necessarily know how the caller created/started the transaction, and so we might have to provide a hook by which the business developer is told that they need to trigger the rollback.

[TheCakeMonster]

Ah you spotted that, good. I knew that myself of course, I was just checking that you did ;-)

No, you're absolutely right, I hadn't thought of that.

In the case of IDataPortalExceptionInspector, an exception has already been raised and caught - that's what we are inspecting - so the only change would be to avoid raising a second. I think we're safe there.

With the other two, the picture is more complex. The implementations we have added both work server-side only, in the sense that they will be the first/outermost requests into the data portal, before any server-side transactions have been initiated (by us) - but that is specific to these implementations. The extensibility mechanisms could be used to add implementations that don't deal with that subtlety, so that might be a problem.

In all cases, the exception is being returned to the caller and rethrown by the client-side data portal code, and I'm not suggesting we change that. If local proxy is in use then client and server are in the same process, so I think the handling of transactions would be correct in LocalProxy scenarios.

The main issue that might remain is remote data portals where the transport is capable of propagating the transaction from the client. It is possible that we would need different behaviours for different server-side data portal implementations I suppose.

When I originally thought about how we might implement these additional checks, I did picture them being done by adding code to the individual server-side data portal implementation classes. That was before I realised that you'd already included extensibility points in the server-side DataPortal class. I'm not sure if that would make much difference, but it might - if I had done it that way, the implementations could have been different by transport, if that were required.

It's difficult to get a full picture of all of the data portal deployment scenarios into my head at once, as there are so many of them, but I think, in general, we'd probably not run into issues with the existing code - but thinking about what other interceptors and authorizers might do is an important part of the puzzle.