Skip to content

Commit 9313e87

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 76a1f2a commit 9313e87

File tree

1 file changed

+61
-4
lines changed

1 file changed

+61
-4
lines changed

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

Lines changed: 61 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,7 @@ void releaseExecutors() {
136142

137143
@Override
138144
public synchronized void onBound(IBinder binder) {
139-
sendSetupTransaction(binderDecorator.decorate(OneWayBinderProxy.wrap(binder, offloadExecutor)));
145+
handshake.onBound(binderDecorator.decorate(OneWayBinderProxy.wrap(binder, offloadExecutor)));
140146
}
141147

142148
@Override
@@ -340,10 +346,11 @@ protected void handleSetupTransport(Parcel parcel) {
340346
Status.UNAVAILABLE.withDescription("Failed to observe outgoing binder"), true);
341347
return;
342348
}
349+
handshake.handleSetupTransport();
350+
}
343351

344-
int remoteUid = Binder.getCallingUid();
345-
restrictIncomingBinderToCallsFrom(remoteUid);
346-
attributes = setSecurityAttrs(attributes, remoteUid);
352+
@GuardedBy("this")
353+
private void checkServerAuthorization(int remoteUid) {
347354
ListenableFuture<Status> authResultFuture = register(checkServerAuthorizationAsync(remoteUid));
348355
Futures.addCallback(
349356
authResultFuture,
@@ -376,7 +383,11 @@ private synchronized void handleAuthResult(Status authorization) {
376383
shutdownInternal(authorization, true);
377384
return;
378385
}
386+
handshake.onServerAuthorizationOk();
387+
}
379388

389+
@GuardedBy("this")
390+
private void onHandshakeComplete() {
380391
setState(TransportState.READY);
381392
attributes = clientTransportListener.filterTransport(attributes);
382393
clientTransportListener.transportReady();
@@ -397,6 +408,52 @@ protected void handlePingResponse(Parcel parcel) {
397408
pingTracker.onPingResponse(parcel.readInt());
398409
}
399410

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

0 commit comments

Comments
 (0)