Skip to content

Commit 58a3aeb

Browse files
committed
Pre-factor out the guts of the BinderClientTransport handshake.
Moves the existing handshake logic behind an interface using the strategy pattern. No functional change.
1 parent 89d77e0 commit 58a3aeb

File tree

1 file changed

+64
-4
lines changed

1 file changed

+64
-4
lines changed

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

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import android.os.IBinder;
2626
import android.os.Parcel;
2727
import android.os.Process;
28+
import androidx.annotation.BinderThread;
29+
import androidx.annotation.MainThread;
2830
import com.google.common.base.Ticker;
2931
import com.google.common.util.concurrent.FutureCallback;
3032
import com.google.common.util.concurrent.Futures;
@@ -72,6 +74,9 @@ public final class BinderClientTransport extends BinderTransport
7274
private final SecurityPolicy securityPolicy;
7375
private final Bindable serviceBinding;
7476

77+
@GuardedBy("this")
78+
private final ClientHandshake handshake;
79+
7580
/** Number of ongoing calls which keep this transport "in-use". */
7681
private final AtomicInteger numInUseStreams;
7782

@@ -114,6 +119,7 @@ public BinderClientTransport(
114119
Boolean preAuthServerOverride = options.getEagAttributes().get(PRE_AUTH_SERVER_OVERRIDE);
115120
this.preAuthorizeServer =
116121
preAuthServerOverride != null ? preAuthServerOverride : factory.preAuthorizeServers;
122+
this.handshake = new LegacyClientHandshake();
117123
numInUseStreams = new AtomicInteger();
118124
pingTracker = new PingTracker(Ticker.systemTicker(), (id) -> sendPing(id));
119125

@@ -136,7 +142,9 @@ void releaseExecutors() {
136142

137143
@Override
138144
public synchronized void onBound(IBinder binder) {
139-
sendSetupTransaction(binderDecorator.decorate(OneWayBinderProxy.wrap(binder, offloadExecutor)));
145+
OneWayBinderProxy binderProxy = OneWayBinderProxy.wrap(binder, offloadExecutor);
146+
binderProxy = binderDecorator.decorate(binderProxy);
147+
handshake.onBound(binderProxy);
140148
}
141149

142150
@Override
@@ -340,10 +348,11 @@ protected void handleSetupTransport(Parcel parcel) {
340348
Status.UNAVAILABLE.withDescription("Failed to observe outgoing binder"), true);
341349
return;
342350
}
351+
handshake.handleSetupTransport();
352+
}
343353

344-
int remoteUid = Binder.getCallingUid();
345-
restrictIncomingBinderToCallsFrom(remoteUid);
346-
attributes = setSecurityAttrs(attributes, remoteUid);
354+
@GuardedBy("this")
355+
private void checkServerAuthorization(int remoteUid) {
347356
ListenableFuture<Status> authResultFuture = register(checkServerAuthorizationAsync(remoteUid));
348357
Futures.addCallback(
349358
authResultFuture,
@@ -376,7 +385,11 @@ private synchronized void handleAuthResult(Status authorization) {
376385
shutdownInternal(authorization, true);
377386
return;
378387
}
388+
handshake.onServerAuthorizationOk();
389+
}
379390

391+
@GuardedBy("this")
392+
private void onHandshakeComplete() {
380393
setState(TransportState.READY);
381394
attributes = clientTransportListener.filterTransport(attributes);
382395
clientTransportListener.transportReady();
@@ -386,6 +399,7 @@ private synchronized void handleAuthResult(Status authorization) {
386399
}
387400
}
388401

402+
389403
private synchronized void handleAuthResult(Throwable t) {
390404
shutdownInternal(
391405
Status.INTERNAL.withDescription("Could not evaluate SecurityPolicy").withCause(t), true);
@@ -397,6 +411,52 @@ protected void handlePingResponse(Parcel parcel) {
397411
pingTracker.onPingResponse(parcel.readInt());
398412
}
399413

414+
/**
415+
* A base for all implementations of the client handshake.
416+
*
417+
* <p>Supports a clean migration away from the legacy approach, one client at a time.
418+
*/
419+
private interface ClientHandshake {
420+
/**
421+
* Notifies the implementation that the binding has succeeded and we are now connected to the
422+
* server's "endpoint" which can be reached at 'endpointBinder'.
423+
*/
424+
@MainThread
425+
void onBound(OneWayBinderProxy endpointBinder);
426+
427+
/** Notifies the implementation that we've received a valid SETUP_TRANSPORT transaction. */
428+
@BinderThread
429+
void handleSetupTransport();
430+
431+
/** Notifies the implementation that the SecurityPolicy check of the server succeeded. */
432+
void onServerAuthorizationOk();
433+
}
434+
435+
private final class LegacyClientHandshake implements ClientHandshake {
436+
@Override
437+
@MainThread
438+
@GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member.
439+
public void onBound(OneWayBinderProxy binder) {
440+
sendSetupTransaction(binder);
441+
}
442+
443+
@Override
444+
@BinderThread
445+
@GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member.
446+
public void handleSetupTransport() {
447+
int remoteUid = Binder.getCallingUid();
448+
restrictIncomingBinderToCallsFrom(remoteUid);
449+
attributes = setSecurityAttrs(attributes, remoteUid);
450+
checkServerAuthorization(remoteUid);
451+
}
452+
453+
@Override
454+
@GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member.
455+
public void onServerAuthorizationOk() {
456+
onHandshakeComplete();
457+
}
458+
}
459+
400460
private static ClientStream newFailingClientStream(
401461
Status failure, Attributes attributes, Metadata headers, ClientStreamTracer[] tracers) {
402462
StatsTraceContext statsTraceContext =

0 commit comments

Comments
 (0)