Skip to content

Commit 9ffa1e1

Browse files
authored
binder: normalize failed auth future status message
fixes #12873 Previously, `BinderTransportSecurity.ServerAuthInterceptor` handled failed authorization futures differently depending on whether the future was already completed.
1 parent dffaaf0 commit 9ffa1e1

2 files changed

Lines changed: 45 additions & 11 deletions

File tree

binder/src/main/java/io/grpc/binder/internal/BinderTransportSecurity.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,10 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
108108
Status authStatus;
109109
try {
110110
authStatus = Futures.getDone(authStatusFuture);
111-
} catch (ExecutionException | CancellationException e) {
112-
// Failed futures are treated as an internal error rather than a security rejection.
113-
authStatus = Status.INTERNAL.withCause(e);
114-
@Nullable String message = e.getMessage();
115-
if (message != null) {
116-
authStatus = authStatus.withDescription(message);
117-
}
111+
} catch (ExecutionException e) {
112+
authStatus = statusFromFailedAuthorizationFuture(e.getCause());
113+
} catch (CancellationException e) {
114+
authStatus = statusFromFailedAuthorizationFuture(e);
118115
}
119116

120117
if (authStatus.isOk()) {
@@ -147,14 +144,18 @@ public void onSuccess(Status authStatus) {
147144

148145
@Override
149146
public void onFailure(Throwable t) {
150-
call.close(
151-
Status.INTERNAL.withCause(t).withDescription("Authorization future failed"),
152-
new Metadata());
147+
call.close(statusFromFailedAuthorizationFuture(t), new Metadata());
153148
}
154149
},
155150
executor);
156151
return listener;
157152
}
153+
154+
private static Status statusFromFailedAuthorizationFuture(Throwable cause) {
155+
// The actual failure is retained as the cause for debugging, but peers should see a
156+
// uniform transport-level failure instead of the underlying exception message.
157+
return Status.INTERNAL.withCause(cause).withDescription("Authorization future failed");
158+
}
158159
}
159160

160161
/**

binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,40 @@ public void testAsyncServerSecurityPolicy_failedFuture_failsWithCodeInternal() t
164164
ListenableFuture<Status> status = makeCall();
165165
statusesToSet.take().setException(new IllegalStateException("oops"));
166166

167-
assertThat(status.get().getCode()).isEqualTo(Status.Code.INTERNAL);
167+
Status failureStatus = status.get();
168+
assertThat(failureStatus.getCode()).isEqualTo(Status.Code.INTERNAL);
169+
assertThat(failureStatus.getDescription()).isEqualTo("Authorization future failed");
170+
}
171+
172+
@Test
173+
public void testAsyncServerSecurityPolicy_failedFuture_subsequentCallHasOpaqueFailure()
174+
throws Exception {
175+
ListenableFuture<Status> firstStatusFuture = makeCall();
176+
statusesToSet.take().setException(new IOException("ouch"));
177+
178+
Status firstStatus = firstStatusFuture.get();
179+
assertThat(firstStatus.getCode()).isEqualTo(Status.Code.INTERNAL);
180+
assertThat(firstStatus.getDescription()).isEqualTo("Authorization future failed");
181+
182+
// TransportAuthorizationState evicts failed futures so the second call triggers a fresh
183+
// authorization check. Both calls must surface an opaque transport-level failure.
184+
ListenableFuture<Status> secondStatusFuture = makeCall();
185+
statusesToSet.take().setException(new IOException("ouch"));
186+
187+
Status secondStatus = secondStatusFuture.get();
188+
assertThat(secondStatus.getCode()).isEqualTo(Status.Code.INTERNAL);
189+
assertThat(secondStatus.getDescription()).isEqualTo("Authorization future failed");
190+
}
191+
192+
@Test
193+
public void testAsyncServerSecurityPolicy_failedFuture_cancelledFutureIsOpaque()
194+
throws Exception {
195+
ListenableFuture<Status> statusFuture = makeCall();
196+
statusesToSet.take().cancel(false);
197+
198+
Status failureStatus = statusFuture.get();
199+
assertThat(failureStatus.getCode()).isEqualTo(Status.Code.INTERNAL);
200+
assertThat(failureStatus.getDescription()).isEqualTo("Authorization future failed");
168201
}
169202

170203
@Test

0 commit comments

Comments
 (0)