From 6025669717f53233b4cd83b00f864738e73c3a38 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Fri, 13 Sep 2024 16:20:16 -0700 Subject: [PATCH 1/3] Introduce channel Attributes and use them to pass the binder target user to NameResolvers --- api/src/main/java/io/grpc/Grpc.java | 9 ++++++ api/src/main/java/io/grpc/NameResolver.java | 31 ++++++++++++++++--- .../grpc/binder/BinderChannelSmokeTest.java | 24 ++++++++++++++ .../java/io/grpc/binder/ApiConstants.java | 19 ++++++++++++ .../io/grpc/binder/BinderChannelBuilder.java | 4 +++ .../io/grpc/internal/ManagedChannelImpl.java | 1 + .../internal/ManagedChannelImplBuilder.java | 21 +++++++++++++ .../testing/FakeNameResolverProvider.java | 27 ++++++++++++++-- 8 files changed, 129 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/io/grpc/Grpc.java b/api/src/main/java/io/grpc/Grpc.java index baa9f5f0ab6..efd6df482e9 100644 --- a/api/src/main/java/io/grpc/Grpc.java +++ b/api/src/main/java/io/grpc/Grpc.java @@ -65,6 +65,15 @@ private Grpc() { @Documented public @interface TransportAttr {} + /** + * Annotation for Channel attributes. It follows the annotation semantics defined by {@link + * Attributes}. + */ + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/00000000") + @Retention(RetentionPolicy.SOURCE) + @Documented + public @interface ChannelAttr {} + /** * Creates a channel builder with a target string and credentials. The target can be either a * valid {@link NameResolver}-compliant URI, or an authority string. diff --git a/api/src/main/java/io/grpc/NameResolver.java b/api/src/main/java/io/grpc/NameResolver.java index bfb9c2a43a1..c86b6a86186 100644 --- a/api/src/main/java/io/grpc/NameResolver.java +++ b/api/src/main/java/io/grpc/NameResolver.java @@ -22,6 +22,7 @@ import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.errorprone.annotations.InlineMe; +import io.grpc.Grpc.ChannelAttr; import java.lang.annotation.Documented; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -281,6 +282,7 @@ public static final class Args { private final ProxyDetector proxyDetector; private final SynchronizationContext syncContext; private final ServiceConfigParser serviceConfigParser; + @ChannelAttr @Nullable private final Attributes channelAttributes; @Nullable private final ScheduledExecutorService scheduledExecutorService; @Nullable private final ChannelLogger channelLogger; @Nullable private final Executor executor; @@ -291,6 +293,7 @@ private Args( ProxyDetector proxyDetector, SynchronizationContext syncContext, ServiceConfigParser serviceConfigParser, + @ChannelAttr Attributes channelAttributes, @Nullable ScheduledExecutorService scheduledExecutorService, @Nullable ChannelLogger channelLogger, @Nullable Executor executor, @@ -299,6 +302,7 @@ private Args( this.proxyDetector = checkNotNull(proxyDetector, "proxyDetector not set"); this.syncContext = checkNotNull(syncContext, "syncContext not set"); this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser not set"); + this.channelAttributes = checkNotNull(channelAttributes, "channelAttributes not set"); this.scheduledExecutorService = scheduledExecutorService; this.channelLogger = channelLogger; this.executor = executor; @@ -363,6 +367,12 @@ public ServiceConfigParser getServiceConfigParser() { return serviceConfigParser; } + /** Returns {@link Attributes} of the associated Channel. */ + @ChannelAttr + public Attributes getChannelAttributes() { + return channelAttributes; + } + /** * Returns the {@link ChannelLogger} for the Channel served by this NameResolver. * @@ -452,6 +462,7 @@ public static final class Builder { private ProxyDetector proxyDetector; private SynchronizationContext syncContext; private ServiceConfigParser serviceConfigParser; + private Attributes channelAttributes = Attributes.EMPTY; private ScheduledExecutorService scheduledExecutorService; private ChannelLogger channelLogger; private Executor executor; @@ -510,6 +521,12 @@ public Builder setServiceConfigParser(ServiceConfigParser parser) { return this; } + /** See {@link Args#getChannelAttributes}. This is a required field, default empty. */ + public Builder setChannelAttributes(@ChannelAttr Attributes channelAttributes) { + this.channelAttributes = checkNotNull(channelAttributes); + return this; + } + /** * See {@link Args#getChannelLogger}. * @@ -548,10 +565,16 @@ public Builder setOverrideAuthority(String authority) { * @since 1.21.0 */ public Args build() { - return - new Args( - defaultPort, proxyDetector, syncContext, serviceConfigParser, - scheduledExecutorService, channelLogger, executor, overrideAuthority); + return new Args( + defaultPort, + proxyDetector, + syncContext, + serviceConfigParser, + channelAttributes, + scheduledExecutorService, + channelLogger, + executor, + overrideAuthority); } } } diff --git a/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java b/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java index 79f7b98f045..c605127e067 100644 --- a/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java +++ b/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java @@ -25,6 +25,8 @@ import android.content.Intent; import android.os.Parcel; import android.os.Parcelable; +import android.os.UserHandle; +import android.os.UserManager; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.io.ByteStreams; @@ -50,6 +52,7 @@ import io.grpc.StatusRuntimeException; import io.grpc.internal.GrpcUtil; import io.grpc.internal.testing.FakeNameResolverProvider; +import io.grpc.internal.testing.FakeNameResolverProvider.FakeNameResolver; import io.grpc.stub.ClientCalls; import io.grpc.stub.MetadataUtils; import io.grpc.stub.ServerCalls; @@ -59,6 +62,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicReference; import org.junit.After; @@ -251,6 +255,26 @@ public void testConnectViaIntentFilter() throws Exception { assertThat(doCall("Hello").get()).isEqualTo("Hello"); } + @Test + public void testTargetUserChannelAttribute() throws Exception { + UserManager userManager = appContext.getSystemService(UserManager.class); + List userProfiles = userManager.getUserProfiles(); + UserHandle someUser = userProfiles.get(0); // Guaranteed to be non-empty. + + channel = + BinderChannelBuilder.forTarget(SERVER_TARGET_URI, appContext).bindAsUser(someUser).build(); + assertThat(doCall("Hello").get()).isNotNull(); + + FakeNameResolver fakeNameResolver = + fakeNameResolverProvider.pollNextResolverCreated(5, SECONDS); + assertThat( + fakeNameResolver + .getArgs() + .getChannelAttributes() + .get(ApiConstants.CHANNEL_ATTR_TARGET_USER)) + .isEqualTo(someUser); + } + @Test public void testUncaughtServerException() throws Exception { // Use a poison parcelable to cause an unexpected Exception in the server's onTransact(). diff --git a/binder/src/main/java/io/grpc/binder/ApiConstants.java b/binder/src/main/java/io/grpc/binder/ApiConstants.java index 43e94338fdc..461adf8ab17 100644 --- a/binder/src/main/java/io/grpc/binder/ApiConstants.java +++ b/binder/src/main/java/io/grpc/binder/ApiConstants.java @@ -17,7 +17,10 @@ package io.grpc.binder; import android.content.Intent; +import android.os.UserHandle; +import io.grpc.Attributes; import io.grpc.ExperimentalApi; +import io.grpc.Grpc.ChannelAttr; /** Constant parts of the gRPC binder transport public API. */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/8022") @@ -29,4 +32,20 @@ private ApiConstants() {} * themselves in a {@link android.app.Service#onBind(Intent)} call. */ public static final String ACTION_BIND = "grpc.io.action.BIND"; + + /** + * The target Android user of a binder Channel. + * + *

In multi-user Android, the target user for an Intent is unfortunately not part of the intent + * itself. Instead, it's passed around as a separate argument wherever that Intent is needed. + * Following suit, this implementation of the binder transport accepts the target Android user as + * a parameter to BinderChannelBuilder -- it's not in the target URI or the SocketAddress. + * Instead, downstream plugins such as {@link io.grpc.NameResolver}s and {@link + * io.grpc.LoadBalancer}s can use this attribute to obtain the Channel's target UserHandle. If the + * attribute is not set, the Channel's target is the Android user hosting the current process (the + * default). + */ + @ChannelAttr + public static final Attributes.Key CHANNEL_ATTR_TARGET_USER = + Attributes.Key.create("io.grpc.binder.CHANNEL_ATTR_TARGET_USER"); } diff --git a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java index d054c8d8ba6..9d5fadf829a 100644 --- a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java +++ b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java @@ -23,6 +23,7 @@ import android.os.UserHandle; import androidx.annotation.RequiresApi; import com.google.errorprone.annotations.DoNotCall; +import io.grpc.Attributes; import io.grpc.ExperimentalApi; import io.grpc.ForwardingChannelBuilder; import io.grpc.ManagedChannel; @@ -159,6 +160,7 @@ public static BinderChannelBuilder forTarget(String target) { private final ManagedChannelImplBuilder managedChannelImplBuilder; private final BinderClientTransportFactory.Builder transportFactoryBuilder; + private final Attributes.Builder channelAttrBuilder = Attributes.newBuilder(); private boolean strictLifecycleManagement; @@ -250,6 +252,7 @@ public BinderChannelBuilder securityPolicy(SecurityPolicy securityPolicy) { @RequiresApi(30) public BinderChannelBuilder bindAsUser(UserHandle targetUserHandle) { transportFactoryBuilder.setTargetUserHandle(targetUserHandle); + channelAttrBuilder.set(ApiConstants.CHANNEL_ATTR_TARGET_USER, targetUserHandle); return this; } @@ -284,6 +287,7 @@ public BinderChannelBuilder idleTimeout(long value, TimeUnit unit) { public ManagedChannel build() { transportFactoryBuilder.setOffloadExecutorPool( managedChannelImplBuilder.getOffloadExecutorPool()); + managedChannelImplBuilder.setChannelAttributes(channelAttrBuilder.build()); return super.build(); } } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index 7f45ca967ea..62a093abd08 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -590,6 +590,7 @@ ClientStream newSubstream( this.authorityOverride = builder.authorityOverride; this.nameResolverArgs = NameResolver.Args.newBuilder() + .setChannelAttributes(builder.channelAttributes) .setDefaultPort(builder.getDefaultPort()) .setProxyDetector(proxyDetector) .setSynchronizationContext(syncContext) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 7da9125087e..90931187481 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -35,6 +35,7 @@ import io.grpc.CompressorRegistry; import io.grpc.DecompressorRegistry; import io.grpc.EquivalentAddressGroup; +import io.grpc.Grpc.ChannelAttr; import io.grpc.InternalChannelz; import io.grpc.InternalConfiguratorRegistry; import io.grpc.ManagedChannel; @@ -197,6 +198,8 @@ public static ManagedChannelBuilder forTarget(String target) { @Nullable ProxyDetector proxyDetector; + @ChannelAttr Attributes channelAttributes = Attributes.EMPTY; + private boolean authorityCheckerDisabled; private boolean statsEnabled = true; private boolean recordStartedRpcs = true; @@ -663,6 +666,14 @@ public void setTracingEnabled(boolean value) { tracingEnabled = value; } + /** + * Makes the specified Attributes available to downstream plugins such as resolvers and load + * balancers. + */ + public void setChannelAttributes(@ChannelAttr Attributes channelAttributes) { + this.channelAttributes = channelAttributes; + } + /** * Verifies the authority is valid. */ @@ -930,4 +941,14 @@ public ClientCall interceptCall( public ObjectPool getOffloadExecutorPool() { return this.offloadExecutorPool; } + + /** Returns the NameResolverRegistry. */ + public NameResolverRegistry getNameResolverRegistry() { + return nameResolverRegistry; + } + + /** Returns the target string. */ + public String getTarget() { + return target; + } } diff --git a/testing/src/main/java/io/grpc/internal/testing/FakeNameResolverProvider.java b/testing/src/main/java/io/grpc/internal/testing/FakeNameResolverProvider.java index 4664dbcc436..d1fb4ca8207 100644 --- a/testing/src/main/java/io/grpc/internal/testing/FakeNameResolverProvider.java +++ b/testing/src/main/java/io/grpc/internal/testing/FakeNameResolverProvider.java @@ -25,12 +25,16 @@ import java.net.URI; import java.util.Collection; import java.util.Collections; +import java.util.concurrent.BlockingDeque; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.TimeUnit; /** A name resolver to always resolve the given URI into the given address. */ public final class FakeNameResolverProvider extends NameResolverProvider { private final URI targetUri; private final SocketAddress address; + private final BlockingDeque resolversCreated = new LinkedBlockingDeque<>(); public FakeNameResolverProvider(String targetUri, SocketAddress address) { this.targetUri = URI.create(targetUri); @@ -40,7 +44,9 @@ public FakeNameResolverProvider(String targetUri, SocketAddress address) { @Override public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) { if (targetUri.equals(this.targetUri)) { - return new FakeNameResolver(address); + FakeNameResolver result = new FakeNameResolver(address, args); + resolversCreated.add(result); + return result; } return null; } @@ -65,15 +71,26 @@ public Collection> getProducedSocketAddressTypes( return Collections.singleton(address.getClass()); } + /** + * Returns the next FakeNameResolver created by this provider, or null if none were created before + * the timeout lapsed. + */ + public FakeNameResolver pollNextResolverCreated(long timeout, TimeUnit unit) + throws InterruptedException { + return resolversCreated.poll(timeout, unit); + } + /** A single name resolver. */ - private static final class FakeNameResolver extends NameResolver { + public static final class FakeNameResolver extends NameResolver { private static final String AUTHORITY = "fake-authority"; private final SocketAddress address; + private final Args args; private volatile boolean shutdown; - private FakeNameResolver(SocketAddress address) { + private FakeNameResolver(SocketAddress address, Args args) { this.address = address; + this.args = args; } @Override @@ -97,5 +114,9 @@ public String getServiceAuthority() { public void shutdown() { shutdown = true; } + + public Args getArgs() { + return this.args; + } } } From 926dd4bbc6625a0633baaa4a85ff4eee1d03309c Mon Sep 17 00:00:00 2001 From: John Cormie Date: Fri, 13 Sep 2024 16:31:37 -0700 Subject: [PATCH 2/3] unfortunately --- binder/src/main/java/io/grpc/binder/ApiConstants.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/binder/src/main/java/io/grpc/binder/ApiConstants.java b/binder/src/main/java/io/grpc/binder/ApiConstants.java index 461adf8ab17..f73ffd288f9 100644 --- a/binder/src/main/java/io/grpc/binder/ApiConstants.java +++ b/binder/src/main/java/io/grpc/binder/ApiConstants.java @@ -39,8 +39,8 @@ private ApiConstants() {} *

In multi-user Android, the target user for an Intent is unfortunately not part of the intent * itself. Instead, it's passed around as a separate argument wherever that Intent is needed. * Following suit, this implementation of the binder transport accepts the target Android user as - * a parameter to BinderChannelBuilder -- it's not in the target URI or the SocketAddress. - * Instead, downstream plugins such as {@link io.grpc.NameResolver}s and {@link + * a parameter to BinderChannelBuilder -- unfortunately it's not in the target URI or the + * SocketAddress. Instead, downstream plugins such as {@link io.grpc.NameResolver}s and {@link * io.grpc.LoadBalancer}s can use this attribute to obtain the Channel's target UserHandle. If the * attribute is not set, the Channel's target is the Android user hosting the current process (the * default). From e5d35455965fc15e6e7f7897e3ec37c76c964ad0 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Fri, 13 Sep 2024 16:34:02 -0700 Subject: [PATCH 3/3] unused --- .../io/grpc/internal/ManagedChannelImplBuilder.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 90931187481..d7dab13a31d 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -941,14 +941,4 @@ public ClientCall interceptCall( public ObjectPool getOffloadExecutorPool() { return this.offloadExecutorPool; } - - /** Returns the NameResolverRegistry. */ - public NameResolverRegistry getNameResolverRegistry() { - return nameResolverRegistry; - } - - /** Returns the target string. */ - public String getTarget() { - return target; - } }