xds: Add ExtAuthzClientCall

[sauravzg]

Introduces ExtAuthzClientCall, which buffers outgoing RPC frames while performing async ext_authz checks. Because the authorization call is asynchronous, outgoing sendMessage() and halfClose() frames are buffered in ExtAuthzClientCall until a decision arrives. On allow, buffered operations are replayed with any header mutations applied. On deny, the call is terminated immediately.

Key classes:

• AuthzCallbackObserver — async listener on the authz gRPC stream that signals the buffering call on completion.
• ExtAuthzClientCall — buffers outgoing frames while authz is pending; drains or cancels based on the decision.
• MutatingClientCall — wraps the real call to inject header mutations from the authz response.
• FailingClientCall / FailingCallWithTrailerMutations — immediate termination paths for denied or disabled-filter scenarios.

[kannanjgithub]

Uncaught exception from onNext causes the stub to cancel the stream and route directly into onError(), which closes authzContext. Its closure does not rely on GC of the stream observer, and it cannot, because it has a direct reference to it from ExtAuthzClientCall.

In a unary rpc, onNext() delivers an inbound response payload; it does not signify that the RPC is complete. A unary RPC completes only when onClose / onCompleted() / onError() is invoked.

The comment needs to be corrected on both the above fronts.

Is relying on this mechanism "Valid Code"?

Mechanically, yes, gRPC will catch the exception and route to onError(). If an unhandled RuntimeException escapes onNext(), ClientCallImpl cancels the stream and delivers onError(), which drains delayedCall and closes authzContext.

However, intentionally relying on this as a design pattern is fragile and carries real risks:

A. Security Risk: Accidentally Failing Open on a DENY

This is the biggest risk:

// in onNext():
AuthzResponse authzResponse = responseHandler.handleResponse(value);
if (authzResponse.decision() == AuthzResponse.Decision.ALLOW) { ... }
else {
  // Suppose decision is DENY, but building FailingCallWithTrailerMutations throws an exception
  ...
}

If the external authorization server returned an explicit DENY response, but an unexpected RuntimeException is thrown while preparing the rejection:

1. The exception escapes onNext().
2. The stub cancels the stream and invokes onError(t).
3. In onError():

   if (config.failureModeAllow()) {
     // FAILS OPEN: creates next.newCall() and forwards request to backend!
   }

If failure_mode_allow: true is configured (which is only intended for network/server outages reaching the authz service), a request that the authz server explicitly denied would be forwarded to the backend!

B. The "Throwing to Trigger Another Callback on Yourself" Anti-Pattern

Relying on throwing an unhandled exception out of onNext() so that the underlying framework cancels the transport stream and calls back into your own onError() is an indirect, surprising control flow:

• It generates unnecessary transport RST_STREAM frames and Failed to read message error logs from ClientCallImpl exception handling.
• It makes debugging difficult because the root cause in onNext gets swallowed into a transport Status.CANCELLED exception passed to onError.

C. Partial Failure State

If an exception occurs after delayedCall.setCall(call) was executed (for example, if callExecutor.execute(drain) throws a RejectedExecutionException):

• When onError() subsequently runs, its call to delayedCall.setCall(...) returns null because realCall != null was already set. The failure call cannot be attached, leaving the call in an inconsistent state.

[sauravzg]

This was interesting.
So, practically it largely doesn't matter right now, because the current code is exception safe. So, a runtime exception shouldn't happen.

But given the security considerations, maybe we should be defensive, I'll have to see if other implementations in flight are as defensive and make changes. Keeping his comment open for now.

[kannanjgithub]

I agree the code is exception safe. We can just remove the comment.

[sauravzg]

I haven't removed the code but rephrased it. I have added that the code is exception safe, but added a todo to revisit this if this changes sometime in future just as a defensive callout.