diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java index 6f95ef8a83c..4f6778e2653 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java @@ -109,12 +109,7 @@ public ServerCall.Listener interceptCall( try { authStatus = Futures.getDone(authStatusFuture); } catch (ExecutionException | CancellationException e) { - // Failed futures are treated as an internal error rather than a security rejection. - authStatus = Status.INTERNAL.withCause(e); - @Nullable String message = e.getMessage(); - if (message != null) { - authStatus = authStatus.withDescription(message); - } + authStatus = statusFromFailedAuthorizationFuture(e); } if (authStatus.isOk()) { @@ -147,14 +142,19 @@ public void onSuccess(Status authStatus) { @Override public void onFailure(Throwable t) { - call.close( - Status.INTERNAL.withCause(t).withDescription("Authorization future failed"), - new Metadata()); + call.close(statusFromFailedAuthorizationFuture(t), new Metadata()); } }, executor); return listener; } + + private static Status statusFromFailedAuthorizationFuture(Throwable t) { + // The actual failure is retained as the cause for debugging, but peers should see a + // uniform transport-level failure instead of the underlying exception message. + Throwable cause = t instanceof ExecutionException && t.getCause() != null ? t.getCause() : t; + return Status.INTERNAL.withCause(cause).withDescription("Authorization future failed"); + } } /** diff --git a/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java b/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java index ffd1d89e69c..b5cef7856dc 100644 --- a/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java +++ b/binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java @@ -164,7 +164,23 @@ public void testAsyncServerSecurityPolicy_failedFuture_failsWithCodeInternal() t ListenableFuture status = makeCall(); statusesToSet.take().setException(new IllegalStateException("oops")); - assertThat(status.get().getCode()).isEqualTo(Status.Code.INTERNAL); + Status failureStatus = status.get(); + assertThat(failureStatus.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(failureStatus.getDescription()).isEqualTo("Authorization future failed"); + } + + @Test + public void testAsyncServerSecurityPolicy_failedFuture_cachedFailureIsOpaque() throws Exception { + ListenableFuture firstStatusFuture = makeCall(); + statusesToSet.take().setException(new IOException("ouch")); + + Status firstStatus = firstStatusFuture.get(); + assertThat(firstStatus.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(firstStatus.getDescription()).isEqualTo("Authorization future failed"); + + Status secondStatus = makeCall().get(); + assertThat(secondStatus.getCode()).isEqualTo(Status.Code.INTERNAL); + assertThat(secondStatus.getDescription()).isEqualTo("Authorization future failed"); } @Test