From 6e84e356d84dfb7bf26d0977a33557b07cd6ed58 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Mon, 19 Aug 2024 14:28:27 -0700 Subject: [PATCH 1/3] Add IntentNameResolverProvider and use it for every new binder Channel. --- .../java/io/grpc/NameResolverRegistry.java | 31 +- .../grpc/binder/BinderChannelSmokeTest.java | 20 +- .../io/grpc/binder/BinderChannelBuilder.java | 9 + .../binder/IntentNameResolverProvider.java | 253 +++++++++++++ .../IntentNameResolverProviderTest.java | 338 ++++++++++++++++++ .../internal/ManagedChannelImplBuilder.java | 9 +- 6 files changed, 656 insertions(+), 4 deletions(-) create mode 100644 binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java create mode 100644 binder/src/test/java/io/grpc/binder/IntentNameResolverProviderTest.java diff --git a/api/src/main/java/io/grpc/NameResolverRegistry.java b/api/src/main/java/io/grpc/NameResolverRegistry.java index 23eec23fd6a..bf68a3fd748 100644 --- a/api/src/main/java/io/grpc/NameResolverRegistry.java +++ b/api/src/main/java/io/grpc/NameResolverRegistry.java @@ -46,6 +46,7 @@ public final class NameResolverRegistry { private static final Logger logger = Logger.getLogger(NameResolverRegistry.class.getName()); private static NameResolverRegistry instance; + private final NameResolverRegistry parent; private final NameResolver.Factory factory = new NameResolverFactory(); private static final String UNKNOWN_SCHEME = "unknown"; @GuardedBy("this") @@ -58,6 +59,27 @@ public final class NameResolverRegistry { @GuardedBy("this") private ImmutableMap effectiveProviders = ImmutableMap.of(); + /** + * Creates a new empty registry. + */ + public NameResolverRegistry() { + this(null); + } + + /** + * Decorates another {@link NameResolverRegistry} without actually modifying it. + * + *

Lookups will consult both this registry and the parent registry, and will return the highest + * priority matching provider between the two of them. This is most useful when 'parent' is the + * default registry and modifying that global variable would be inappropriate. + * + * @param parent another registry to decorate + */ + @Internal + public NameResolverRegistry(NameResolverRegistry parent) { + this.parent = parent; + } + public synchronized String getDefaultScheme() { return defaultScheme; } @@ -66,7 +88,14 @@ public NameResolverProvider getProviderForScheme(String scheme) { if (scheme == null) { return null; } - return providers().get(scheme.toLowerCase(Locale.US)); + NameResolverProvider parentResult = (parent != null) + ? parent.getProviderForScheme(scheme) : null; + NameResolverProvider selfResult = providers().get(scheme.toLowerCase(Locale.US)); + return (priorityOf(parentResult) > priorityOf(selfResult)) ? parentResult : selfResult; + } + + private static int priorityOf(NameResolverProvider provider) { + return provider != null ? provider.priority() : Integer.MIN_VALUE; } /** diff --git a/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java b/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java index 79f7b98f045..5812809e09a 100644 --- a/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java +++ b/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java @@ -20,6 +20,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.MINUTES; import static java.util.concurrent.TimeUnit.SECONDS; +import static org.junit.Assert.fail; import android.content.Context; import android.content.Intent; @@ -59,6 +60,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicReference; import org.junit.After; @@ -239,6 +241,20 @@ public void testConnectViaTargetUri() throws Exception { assertThat(doCall("Hello").get()).isEqualTo("Hello"); } + @Test + public void testConnectViaIntentTargetUri() throws Exception { + // TODO(jdcormie): Make this test good. + channel = BinderChannelBuilder.forTarget("intent://foo/bar", appContext).build(); + ListenableFuture resultFuture = doCall("Hello"); + try { + resultFuture.get(); + fail(); + } catch (ExecutionException ee) { + StatusRuntimeException sre = (StatusRuntimeException) ee.getCause(); + assertThat(sre.getStatus().getCode()).isEqualTo(Code.UNIMPLEMENTED); + } + } + @Test public void testConnectViaIntentFilter() throws Exception { // Compare with the mapping in AndroidManifest.xml. @@ -263,7 +279,7 @@ public void testUncaughtServerException() throws Exception { CallOptions callOptions = CallOptions.DEFAULT.withDeadlineAfter(5, SECONDS); try { ClientCalls.blockingUnaryCall(interceptedChannel, method, callOptions, "hello"); - Assert.fail(); + fail(); } catch (StatusRuntimeException e) { // We don't care how *our* RPC failed, but make sure we didn't have to rely on the deadline. assertThat(e.getStatus().getCode()).isNotEqualTo(Code.DEADLINE_EXCEEDED); @@ -278,7 +294,7 @@ public void testUncaughtClientException() throws Exception { CallOptions callOptions = CallOptions.DEFAULT.withDeadlineAfter(5, SECONDS); try { ClientCalls.blockingUnaryCall(channel, method, callOptions, "hello"); - Assert.fail(); + fail(); } catch (StatusRuntimeException e) { // We don't care *how* our RPC failed, but make sure we didn't have to rely on the deadline. assertThat(e.getStatus().getCode()).isNotEqualTo(Code.DEADLINE_EXCEEDED); diff --git a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java index d054c8d8ba6..a4b9e1129c1 100644 --- a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java +++ b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java @@ -27,6 +27,7 @@ import io.grpc.ForwardingChannelBuilder; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; +import io.grpc.NameResolverRegistry; import io.grpc.binder.internal.BinderClientTransportFactory; import io.grpc.internal.FixedObjectPool; import io.grpc.internal.ManagedChannelImplBuilder; @@ -179,6 +180,14 @@ private BinderChannelBuilder( } else { managedChannelImplBuilder = new ManagedChannelImplBuilder(target, transportFactoryBuilder, null); + + // We can't use the "default" registry because it's a global variable. 1) That would leak + // 'sourceContext' and 2) our NRP would clash with one installed by any other + // BinderChannelBuilder using a different 'sourceContext'. Instead decorate it. + NameResolverRegistry wrapperRegistry = + new NameResolverRegistry(managedChannelImplBuilder.getNameResolverRegistry()); + wrapperRegistry.register(new IntentNameResolverProvider(sourceContext)); + managedChannelImplBuilder.nameResolverRegistry(wrapperRegistry); } idleTimeout(60, TimeUnit.SECONDS); } diff --git a/binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java b/binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java new file mode 100644 index 00000000000..13c709b404e --- /dev/null +++ b/binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java @@ -0,0 +1,253 @@ +package io.grpc.binder; + +import static android.content.Intent.URI_INTENT_SCHEME; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + +import android.annotation.SuppressLint; +import android.content.BroadcastReceiver; +import android.content.ComponentName; +import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; +import android.content.pm.PackageManager; +import android.content.pm.ResolveInfo; +import android.os.UserHandle; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.MoreExecutors; +import io.grpc.EquivalentAddressGroup; +import io.grpc.NameResolver; +import io.grpc.NameResolver.Args; +import io.grpc.NameResolver.ResolutionResult; +import io.grpc.NameResolverProvider; +import io.grpc.Status; +import io.grpc.StatusException; +import io.grpc.SynchronizationContext; +import java.net.SocketAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.Executor; +import javax.annotation.Nullable; + +/** + * A {@link NameResolverProvider} that resolves an "intent:" target URI to a set of + * AndroidComponentAddress'es + */ +final class IntentNameResolverProvider extends NameResolverProvider { + + /** The URI scheme created by {@link Intent#URI_INTENT_SCHEME}. */ + public static final String ANDROID_INTENT_SCHEME = "intent"; + + private final Context context; + + /** + * Creates a new AndroidIntentNameResolverProvider. + * + * @param context an Android {@link Context} that will outlive this instance + */ + public IntentNameResolverProvider(Context context) { + this.context = context; + } + + @Override + public String getDefaultScheme() { + return ANDROID_INTENT_SCHEME; + } + + @Nullable + @Override + public NameResolver newNameResolver(URI targetUri, final Args args) { + if (Objects.equals(targetUri.getScheme(), ANDROID_INTENT_SCHEME)) { + return new Resolver(targetUri, args); + } else { + return null; + } + } + + @Override + public boolean isAvailable() { + return true; + } + + @Override + public int priority() { + return 5; // default. + } + + @Override + public ImmutableSet> getProducedSocketAddressTypes() { + return ImmutableSet.of(AndroidComponentAddress.class); + } + + private ResolutionResult lookupAndroidComponentAddress(URI targetUri, Args args) + throws StatusException { + Intent targetIntent = parseUri(targetUri); + List resolveInfoList = lookupServices(targetIntent); + + // Model each matching android.app.Service as an individual gRPC server with a single address. + List servers = new ArrayList<>(); + for (ResolveInfo resolveInfo : resolveInfoList) { + // This is needed because using targetIntent directly causes UnsafeIntentLaunchViolation. + Intent copyTargetIntent = new Intent(); + copyTargetIntent.setAction(targetIntent.getAction()); + copyTargetIntent.setData(targetIntent.getData()); + // If Intent has no categories, getCategories() returns Null instead of an empty set of + // categories. + if (targetIntent.getCategories() != null) { + for (String category : targetIntent.getCategories()) { + copyTargetIntent.addCategory(category); + } + } + copyTargetIntent.setComponent( + new ComponentName(resolveInfo.serviceInfo.packageName, resolveInfo.serviceInfo.name)); + servers.add( + new EquivalentAddressGroup(AndroidComponentAddress.forBindIntent(copyTargetIntent))); + } + + return ResolutionResult.newBuilder() + .setAddresses(ImmutableList.copyOf(servers)) + // pick_first is the default load balancing (LB) policy if the service config does not + // specify one. + .setServiceConfig(args.getServiceConfigParser().parseServiceConfig(ImmutableMap.of())) + .build(); + } + + private List lookupServices(Intent intent) throws StatusException { + PackageManager packageManager = context.getPackageManager(); + List intentServices = packageManager.queryIntentServices(intent, 0); + if (intentServices.isEmpty()) { + throw Status.UNIMPLEMENTED + .withDescription("Service not found for intent " + intent) + .asException(); + } + return intentServices; + } + + private Intent parseUri(URI targetUri) throws StatusException { + try { + return Intent.parseUri(targetUri.toString(), URI_INTENT_SCHEME); + } catch (URISyntaxException uriSyntaxException) { + throw Status.INVALID_ARGUMENT + .withCause(uriSyntaxException) + .withDescription("Failed to parse target URI " + targetUri + " as intent") + .asException(); + } + } + + /** A single name resolver. */ + private final class Resolver extends NameResolver { + + private final URI targetUri; + private final Args args; + private final Executor offloadExecutor; + private final Executor sequentialExecutor; + + // Accessed only on `sequentialExecutor` + @Nullable private PackageChangeReceiver receiver; + + // Accessed only on Args#getSynchronizationContext. + private boolean shutdown; + @Nullable private Listener2 listener; + + private Resolver(URI targetUri, Args args) { + this.targetUri = targetUri; + this.args = args; + // This Executor is nominally optional but all grpc-java Channels provide it since 1.25. + this.offloadExecutor = + checkNotNull(args.getOffloadExecutor(), "NameResolver.Args.getOffloadExecutor()"); + this.sequentialExecutor = MoreExecutors.newSequentialExecutor(offloadExecutor); + } + + @Override + public void start(Listener2 listener) { + checkState(this.listener == null, "Already started!"); + checkState(!shutdown, "Resolver is shutdown"); + this.listener = checkNotNull(listener); + resolve(); + } + + @Override + public void refresh() { + checkState(listener != null, "Not started!"); + resolve(); + } + + private void resolve() { + if (shutdown) { + return; + } + // Capture non-final `listener` here while we're on args.getSynchronizationContext(). + Listener2 listener = checkNotNull(this.listener); + Futures.addCallback( + Futures.submit( + () -> { + maybeRegisterReceiver(); + return lookupAndroidComponentAddress(targetUri, args); + }, + sequentialExecutor), + new FutureCallback() { + @Override + public void onSuccess(ResolutionResult result) { + listener.onResult(result); + } + + @Override + public void onFailure(Throwable t) { + listener.onError(Status.fromThrowable(t)); + } + }, + sequentialExecutor); + } + + @Override + public String getServiceAuthority() { + return "localhost"; + } + + @Override + public void shutdown() { + if (!shutdown) { + shutdown = true; + sequentialExecutor.execute(this::maybeUnregisterReceiver); + } + } + + final class PackageChangeReceiver extends BroadcastReceiver { + @Override + public void onReceive(Context context, Intent intent) { + // Get off the main thread and into the correct SynchronizationContext. + SynchronizationContext syncContext = args.getSynchronizationContext(); + syncContext.executeLater(Resolver.this::resolve); + offloadExecutor.execute(syncContext::drain); + } + } + + @SuppressLint("UnprotectedReceiver") // All of these are protected system broadcasts. + private void maybeRegisterReceiver() { + if (receiver == null) { + receiver = new PackageChangeReceiver(); + IntentFilter filter = new IntentFilter(); + filter.addDataScheme("package"); + filter.addAction(Intent.ACTION_PACKAGE_ADDED); + filter.addAction(Intent.ACTION_PACKAGE_CHANGED); + filter.addAction(Intent.ACTION_PACKAGE_REMOVED); + filter.addAction(Intent.ACTION_PACKAGE_REPLACED); + context.registerReceiver(receiver, filter); + } + } + + private void maybeUnregisterReceiver() { + if (receiver != null) { + context.unregisterReceiver(receiver); + receiver = null; + } + } + } +} diff --git a/binder/src/test/java/io/grpc/binder/IntentNameResolverProviderTest.java b/binder/src/test/java/io/grpc/binder/IntentNameResolverProviderTest.java new file mode 100644 index 00000000000..4549c06d066 --- /dev/null +++ b/binder/src/test/java/io/grpc/binder/IntentNameResolverProviderTest.java @@ -0,0 +1,338 @@ +package io.grpc.binder; + +import static android.content.Intent.ACTION_PACKAGE_REPLACED; +import static android.content.Intent.URI_INTENT_SCHEME; +import static android.os.Looper.getMainLooper; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.robolectric.Shadows.shadowOf; + +import android.app.Application; +import android.content.ComponentName; +import android.content.Intent; +import android.content.IntentFilter; +import android.net.Uri; +import androidx.core.content.ContextCompat; +import androidx.test.core.app.ApplicationProvider; +import io.grpc.EquivalentAddressGroup; +import io.grpc.NameResolver; +import io.grpc.NameResolver.ResolutionResult; +import io.grpc.NameResolver.ServiceConfigParser; +import io.grpc.NameResolverProvider; +import io.grpc.Status; +import io.grpc.SynchronizationContext; +import java.net.URI; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoTestRule; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.shadows.ShadowPackageManager; + +/** A test for IntentNameResolverProvider. */ +@RunWith(RobolectricTestRunner.class) +public final class IntentNameResolverProviderTest { + + private static final ComponentName SOME_COMPONENT_NAME = + new ComponentName("com.foo.bar", "SomeComponent"); + private static final ComponentName ANOTHER_COMPONENT_NAME = + new ComponentName("org.blah", "AnotherComponent"); + private final Application appContext = ApplicationProvider.getApplicationContext(); + private final NameResolver.Args args = newNameResolverArgs(); + + private final ShadowPackageManager shadowPackageManager = + shadowOf(appContext.getPackageManager()); + private NameResolverProvider provider; + + @Rule public MockitoTestRule mockitoTestRule = MockitoJUnit.testRule(this); + @Mock public NameResolver.Listener2 mockListener; + @Captor public ArgumentCaptor resultCaptor; + + @Before + public void setUp() { + provider = new IntentNameResolverProvider(appContext); + } + + @Test + public void testProviderScheme_returnsIntentScheme() throws Exception { + assertThat(provider.getDefaultScheme()) + .isEqualTo(IntentNameResolverProvider.ANDROID_INTENT_SCHEME); + } + + @Test + public void testNoResolverForUnknownScheme_returnsNull() throws Exception { + assertThat(provider.newNameResolver(new URI("random://uri"), args)).isNull(); + } + + @Test + public void testResolverForIntentScheme_returnsResolverWithLocalHostAuthority() throws Exception { + URI uri = getIntentUri(newIntent()); + NameResolver resolver = provider.newNameResolver(uri, args); + assertThat(resolver).isNotNull(); + assertThat(resolver.getServiceAuthority()).isEqualTo("localhost"); + } + + @Test + public void testResolutionWithoutServicesAvailable_returnsUnimplemented() throws Exception { + Status resolutionError = resolveExpectingError(getIntentUri(newIntent())); + assertThat(resolutionError).isNotNull(); + assertThat(resolutionError.getCode()).isEqualTo(Status.UNIMPLEMENTED.getCode()); + } + + @Test + public void testResolutionWithBadUri_returnsIllegalArg() throws Exception { + Status resolutionError = resolveExpectingError(new URI("intent:xxx#Intent;e.x=1;end;")); + assertThat(resolutionError.getCode()).isEqualTo(Status.INVALID_ARGUMENT.getCode()); + } + + @Test + public void testResolutionWithMultipleServicesAvailable_returnsAndroidComponentAddresses() + throws Exception { + Intent intent = newIntent(); + IntentFilter serviceIntentFilter = newFilterMatching(intent); + + shadowPackageManager.addServiceIfNotPresent(SOME_COMPONENT_NAME); + shadowPackageManager.addIntentFilterForService(SOME_COMPONENT_NAME, serviceIntentFilter); + + // Adds another valid Service + shadowPackageManager.addServiceIfNotPresent(ANOTHER_COMPONENT_NAME); + shadowPackageManager.addIntentFilterForService(ANOTHER_COMPONENT_NAME, serviceIntentFilter); + + NameResolver nameResolver = provider.newNameResolver(getIntentUri(intent), args); + nameResolver.start(mockListener); + shadowOf(getMainLooper()).idle(); + + verify(mockListener, never()).onError(any()); + verify(mockListener).onResult(resultCaptor.capture()); + assertThat(resultCaptor.getValue().getAddresses()) + .containsExactly( + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(SOME_COMPONENT_NAME))), + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(ANOTHER_COMPONENT_NAME)))); + + nameResolver.shutdown(); + shadowOf(getMainLooper()).idle(); + } + + @Test + public void testServiceRemoved_pushesUpdatedAndroidComponentAddresses() throws Exception { + Intent intent = newIntent(); + IntentFilter serviceIntentFilter = newFilterMatching(intent); + + shadowPackageManager.addServiceIfNotPresent(SOME_COMPONENT_NAME); + shadowPackageManager.addIntentFilterForService(SOME_COMPONENT_NAME, serviceIntentFilter); + + // Adds another valid Service + shadowPackageManager.addServiceIfNotPresent(ANOTHER_COMPONENT_NAME); + shadowPackageManager.addIntentFilterForService(ANOTHER_COMPONENT_NAME, serviceIntentFilter); + + NameResolver nameResolver = provider.newNameResolver(getIntentUri(intent), args); + nameResolver.start(mockListener); + shadowOf(getMainLooper()).idle(); + + verify(mockListener, never()).onError(any()); + verify(mockListener).onResult(resultCaptor.capture()); + assertThat(resultCaptor.getValue().getAddresses()) + .containsExactly( + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(SOME_COMPONENT_NAME))), + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(ANOTHER_COMPONENT_NAME)))); + + shadowPackageManager.removeService(ANOTHER_COMPONENT_NAME); + broadcastPackageChange(ACTION_PACKAGE_REPLACED, ANOTHER_COMPONENT_NAME.getPackageName()); + shadowOf(getMainLooper()).idle(); + + verify(mockListener, never()).onError(any()); + verify(mockListener, times(2)).onResult(resultCaptor.capture()); + assertThat(resultCaptor.getValue().getAddresses()) + .containsExactly( + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(SOME_COMPONENT_NAME)))); + + nameResolver.shutdown(); + shadowOf(getMainLooper()).idle(); + + // No Listener callbacks post-shutdown(). + verifyNoMoreInteractions(mockListener); + // No leaked receivers. + assertThat(shadowOf(appContext).getRegisteredReceivers()).isEmpty(); + } + + @Test + public void testRefresh_returnsSameAndroidComponentAddresses() throws Exception { + Intent intent = newIntent(); + IntentFilter serviceIntentFilter = newFilterMatching(intent); + + shadowPackageManager.addServiceIfNotPresent(SOME_COMPONENT_NAME); + shadowPackageManager.addIntentFilterForService(SOME_COMPONENT_NAME, serviceIntentFilter); + + // Adds another valid Service + shadowPackageManager.addServiceIfNotPresent(ANOTHER_COMPONENT_NAME); + shadowPackageManager.addIntentFilterForService(ANOTHER_COMPONENT_NAME, serviceIntentFilter); + + NameResolver nameResolver = provider.newNameResolver(getIntentUri(intent), args); + nameResolver.start(mockListener); + shadowOf(getMainLooper()).idle(); + + verify(mockListener, never()).onError(any()); + verify(mockListener).onResult(resultCaptor.capture()); + assertThat(resultCaptor.getValue().getAddresses()) + .containsExactly( + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(SOME_COMPONENT_NAME))), + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(ANOTHER_COMPONENT_NAME)))); + + nameResolver.refresh(); + shadowOf(getMainLooper()).idle(); + verify(mockListener, never()).onError(any()); + verify(mockListener, times(2)).onResult(resultCaptor.capture()); + assertThat(resultCaptor.getValue().getAddresses()) + .containsExactly( + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(SOME_COMPONENT_NAME))), + new EquivalentAddressGroup( + AndroidComponentAddress.forBindIntent( + intent.cloneFilter().setComponent(ANOTHER_COMPONENT_NAME)))); + + nameResolver.shutdown(); + shadowOf(getMainLooper()).idle(); + assertThat(shadowOf(appContext).getRegisteredReceivers()).isEmpty(); + } + + private void broadcastPackageChange(String action, String pkgName) { + Intent broadcast = new Intent(); + broadcast.setAction(action); + broadcast.setData(Uri.parse("package:" + pkgName)); + appContext.sendBroadcast(broadcast); + } + + @Test + public void testResolutionOnResultThrows_onErrorNotCalled() throws Exception { + Intent intent = newIntent(); + shadowPackageManager.addServiceIfNotPresent(SOME_COMPONENT_NAME); + shadowPackageManager.addIntentFilterForService(SOME_COMPONENT_NAME, newFilterMatching(intent)); + + doThrow(SomeRuntimeException.class).when(mockListener).onResult(any()); + + provider.newNameResolver(getIntentUri(intent), args).start(mockListener); + try { + shadowOf(getMainLooper()).idle(); + } catch (SomeRuntimeException e) { + // Permitted. + } + + verify(mockListener).onResult(any()); + verify(mockListener, never()).onError(any()); + } + + final class SomeRuntimeException extends RuntimeException { + static final long serialVersionUID = 0; + } + + private Status resolveExpectingError(URI target) throws Exception { + AtomicReference statusResult = new AtomicReference<>(); + AtomicReference resolutionResult = new AtomicReference<>(); + provider + .newNameResolver(target, args) + .start( + new NameResolver.Listener2() { + @Override + public void onResult(ResolutionResult result) { + resolutionResult.set(result); + } + + @Override + public void onError(Status status) { + statusResult.set(status); + } + }); + if (resolutionResult.get() != null) { + Assert.fail("Expected error, but got result: " + resolutionResult.get()); + } + shadowOf(getMainLooper()).idle(); + return checkNotNull(statusResult.get()); + } + + private static Intent newIntent() throws Exception { + Intent intent = new Intent(); + intent.setAction("test.action"); + intent.setData(Uri.parse("grpc:ServiceName")); + return intent; + } + + private static IntentFilter newFilterMatching(Intent intent) throws Exception { + IntentFilter filter = new IntentFilter(); + if (intent.getAction() != null) { + filter.addAction(intent.getAction()); + } + Uri data = intent.getData(); + if (data != null) { + if (data.getScheme() != null) { + filter.addDataScheme(data.getScheme()); + } + if (data.getSchemeSpecificPart() != null) { + filter.addDataSchemeSpecificPart(data.getSchemeSpecificPart(), 0); + } + } + Set categories = intent.getCategories(); + if (categories != null) { + for (String category : categories) { + filter.addCategory(category); + } + } + return filter; + } + + private static URI getIntentUri(Intent intent) throws Exception { + return new URI(intent.toUri(URI_INTENT_SCHEME)); + } + + /** Returns a new test-specific {@link NameResolver.Args} instance. */ + private NameResolver.Args newNameResolverArgs() { + return NameResolver.Args.newBuilder() + .setDefaultPort(-1) + .setProxyDetector((target) -> null) // No proxies here. + .setSynchronizationContext(synchronizationContext()) + .setOffloadExecutor(ContextCompat.getMainExecutor(appContext)) + .setServiceConfigParser(mock(ServiceConfigParser.class)) + .build(); + } + + /** + * Returns a test {@link SynchronizationContext}. + * + *

Exceptions will cause the test to fail with {@link AssertionError}. + */ + private static SynchronizationContext synchronizationContext() { + return new SynchronizationContext( + (thread, exception) -> { + throw new AssertionError(exception); + }); + } +} diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 7da9125087e..390e1cbd3a0 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -429,7 +429,7 @@ public ManagedChannelImplBuilder nameResolverFactory(NameResolver.Factory resolv return this; } - ManagedChannelImplBuilder nameResolverRegistry(NameResolverRegistry resolverRegistry) { + public ManagedChannelImplBuilder nameResolverRegistry(NameResolverRegistry resolverRegistry) { this.nameResolverRegistry = resolverRegistry; return this; } @@ -930,4 +930,11 @@ public ClientCall interceptCall( public ObjectPool getOffloadExecutorPool() { return this.offloadExecutorPool; } + + /** + * + */ + public NameResolverRegistry getNameResolverRegistry() { + return nameResolverRegistry; + } } From 646906ff3708d423e4dc26bea0648991d127dc56 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Mon, 19 Aug 2024 15:05:09 -0700 Subject: [PATCH 2/3] Fix javadoc --- .../main/java/io/grpc/internal/ManagedChannelImplBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index 390e1cbd3a0..81a39023197 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -932,7 +932,7 @@ public ObjectPool getOffloadExecutorPool() { } /** - * + * Returns the NameResolverRegistry. */ public NameResolverRegistry getNameResolverRegistry() { return nameResolverRegistry; From 57b29c6a2bb6a4487ca0aabdf7b2d558117e994f Mon Sep 17 00:00:00 2001 From: John Cormie Date: Fri, 13 Sep 2024 14:46:37 -0700 Subject: [PATCH 3/3] BinderChannelBuilder sends the target user to INRP via an Attribute --- api/src/main/java/io/grpc/Grpc.java | 9 +++ api/src/main/java/io/grpc/NameResolver.java | 25 ++++++++ .../grpc/binder/BinderChannelSmokeTest.java | 1 + .../java/io/grpc/binder/ApiConstants.java | 7 +++ .../io/grpc/binder/BinderChannelBuilder.java | 20 ++++--- .../binder/IntentNameResolverProvider.java | 59 ++++++++++++++++++- .../BinderClientTransportFactory.java | 8 +++ .../io/grpc/internal/ManagedChannelImpl.java | 1 + .../internal/ManagedChannelImplBuilder.java | 15 +++++ 9 files changed, 133 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/io/grpc/Grpc.java b/api/src/main/java/io/grpc/Grpc.java index baa9f5f0ab6..3e8ddcac289 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/5678") + @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..3f3ffe6f2c0 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; + @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, + 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,15 @@ public ServiceConfigParser getServiceConfigParser() { return serviceConfigParser; } + /** + * Returns the {@link ServiceConfigParser}. + * + * @since 1.21.0 + */ + public Attributes getChannelAttributes() { + return channelAttributes; + } + /** * Returns the {@link ChannelLogger} for the Channel served by this NameResolver. * @@ -452,6 +465,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 +524,16 @@ public Builder setServiceConfigParser(ServiceConfigParser parser) { return this; } + /** + * See {@link Args#getChannelAttributes}. This is a required field, default empty. + * + * @since 1.XXX + */ + public Builder setChannelAttributes(@ChannelAttr Attributes channelAttributes) { + this.channelAttributes = checkNotNull(channelAttributes); + return this; + } + /** * See {@link Args#getChannelLogger}. * @@ -551,6 +575,7 @@ public Args build() { 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 5812809e09a..4819a6f1836 100644 --- a/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java +++ b/binder/src/androidTest/java/io/grpc/binder/BinderChannelSmokeTest.java @@ -245,6 +245,7 @@ public void testConnectViaTargetUri() throws Exception { public void testConnectViaIntentTargetUri() throws Exception { // TODO(jdcormie): Make this test good. channel = BinderChannelBuilder.forTarget("intent://foo/bar", appContext).build(); + // channel = BinderChannelBuilder.forTarget("android-app://com.foo.bar/authoritaaay", appContext).build(); ListenableFuture resultFuture = doCall("Hello"); try { resultFuture.get(); diff --git a/binder/src/main/java/io/grpc/binder/ApiConstants.java b/binder/src/main/java/io/grpc/binder/ApiConstants.java index 43e94338fdc..416d41d8517 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,8 @@ private ApiConstants() {} * themselves in a {@link android.app.Service#onBind(Intent)} call. */ public static final String ACTION_BIND = "grpc.io.action.BIND"; + + @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 a4b9e1129c1..369e14ba2cf 100644 --- a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java +++ b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java @@ -23,11 +23,11 @@ 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; import io.grpc.ManagedChannelBuilder; -import io.grpc.NameResolverRegistry; import io.grpc.binder.internal.BinderClientTransportFactory; import io.grpc.internal.FixedObjectPool; import io.grpc.internal.ManagedChannelImplBuilder; @@ -160,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; @@ -180,14 +181,13 @@ private BinderChannelBuilder( } else { managedChannelImplBuilder = new ManagedChannelImplBuilder(target, transportFactoryBuilder, null); - - // We can't use the "default" registry because it's a global variable. 1) That would leak - // 'sourceContext' and 2) our NRP would clash with one installed by any other - // BinderChannelBuilder using a different 'sourceContext'. Instead decorate it. - NameResolverRegistry wrapperRegistry = - new NameResolverRegistry(managedChannelImplBuilder.getNameResolverRegistry()); - wrapperRegistry.register(new IntentNameResolverProvider(sourceContext)); - managedChannelImplBuilder.nameResolverRegistry(wrapperRegistry); + if (target.startsWith("intent:")) { + // We register our resolver here instead of using SPI: + // 1) So the 99% of Android apps that don't use grpc-binder and don't use indirect + // addressing don't have to pay to load our classes. + // 2) It's our first opportunity to access the Application Context. + IntentNameResolverProvider.createAndRegisterSingletonOnce(sourceContext.getApplicationContext()); + } } idleTimeout(60, TimeUnit.SECONDS); } @@ -259,6 +259,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; } @@ -293,6 +294,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/binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java b/binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java index 13c709b404e..bd55af76c4b 100644 --- a/binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java +++ b/binder/src/main/java/io/grpc/binder/IntentNameResolverProvider.java @@ -19,14 +19,18 @@ import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.MoreExecutors; +import io.grpc.Attributes; import io.grpc.EquivalentAddressGroup; import io.grpc.NameResolver; import io.grpc.NameResolver.Args; import io.grpc.NameResolver.ResolutionResult; import io.grpc.NameResolverProvider; +import io.grpc.NameResolverRegistry; import io.grpc.Status; import io.grpc.StatusException; import io.grpc.SynchronizationContext; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.SocketAddress; import java.net.URI; import java.net.URISyntaxException; @@ -34,6 +38,7 @@ import java.util.List; import java.util.Objects; import java.util.concurrent.Executor; +import java.util.logging.Logger; import javax.annotation.Nullable; /** @@ -41,12 +46,15 @@ * AndroidComponentAddress'es */ final class IntentNameResolverProvider extends NameResolverProvider { + private static final Logger logger = Logger.getLogger(IntentNameResolverProvider.class.getName()); /** The URI scheme created by {@link Intent#URI_INTENT_SCHEME}. */ public static final String ANDROID_INTENT_SCHEME = "intent"; private final Context context; + private static IntentNameResolverProvider INSTANCE; + /** * Creates a new AndroidIntentNameResolverProvider. * @@ -56,11 +64,34 @@ public IntentNameResolverProvider(Context context) { this.context = context; } + /** + * + *

This is safe even in the rare case where a single process hosts multiple Applications + * because each one gets its own classloader and thus instance of this class. + * + * @param application + * @return + */ + public static IntentNameResolverProvider createAndRegisterSingletonOnce(Context application) { + synchronized (IntentNameResolverProvider.class) { + if (INSTANCE == null) { + INSTANCE = new IntentNameResolverProvider(application); + NameResolverRegistry.getDefaultRegistry().register(INSTANCE); + } + return INSTANCE; + } + } + @Override public String getDefaultScheme() { return ANDROID_INTENT_SCHEME; } + @Override + public String getScheme() { + return ANDROID_INTENT_SCHEME; + } + @Nullable @Override public NameResolver newNameResolver(URI targetUri, final Args args) { @@ -89,7 +120,7 @@ public ImmutableSet> getProducedSocketAddressType private ResolutionResult lookupAndroidComponentAddress(URI targetUri, Args args) throws StatusException { Intent targetIntent = parseUri(targetUri); - List resolveInfoList = lookupServices(targetIntent); + List resolveInfoList = lookupServices(targetIntent, args); // Model each matching android.app.Service as an individual gRPC server with a single address. List servers = new ArrayList<>(); @@ -111,17 +142,38 @@ private ResolutionResult lookupAndroidComponentAddress(URI targetUri, Args args) new EquivalentAddressGroup(AndroidComponentAddress.forBindIntent(copyTargetIntent))); } + Attributes.Builder attributes = Attributes.newBuilder(); + UserHandle targetUser = args.getChannelAttributes().get(ApiConstants.CHANNEL_ATTR_TARGET_USER); + if (targetUser != null) { + attributes.set(ApiConstants.CHANNEL_ATTR_TARGET_USER, targetUser); + } + return ResolutionResult.newBuilder() .setAddresses(ImmutableList.copyOf(servers)) // pick_first is the default load balancing (LB) policy if the service config does not // specify one. .setServiceConfig(args.getServiceConfigParser().parseServiceConfig(ImmutableMap.of())) + .setAttributes(attributes.build()) .build(); } - private List lookupServices(Intent intent) throws StatusException { + @SuppressWarnings("unchecked") // Reflection + private List lookupServices(Intent intent, Args args) throws StatusException { PackageManager packageManager = context.getPackageManager(); - List intentServices = packageManager.queryIntentServices(intent, 0); + List intentServices; + UserHandle targetUser = args.getChannelAttributes().get(ApiConstants.CHANNEL_ATTR_TARGET_USER); + if (targetUser != null) { + // queryIntentServicesAsUser() is a @SystemApi + try { + Method queryMethod = packageManager.getClass() + .getMethod("queryIntentServicesAsUser", Intent.class, int.class, UserHandle.class); + intentServices = (List) queryMethod.invoke(packageManager, intent, 0, targetUser); + } catch (InvocationTargetException | IllegalAccessException | NoSuchMethodException e) { + throw Status.INTERNAL.withCause(e).asException(); + } + } else { + intentServices = packageManager.queryIntentServices(intent, 0); + } if (intentServices.isEmpty()) { throw Status.UNIMPLEMENTED .withDescription("Service not found for intent " + intent) @@ -231,6 +283,7 @@ public void onReceive(Context context, Intent intent) { @SuppressLint("UnprotectedReceiver") // All of these are protected system broadcasts. private void maybeRegisterReceiver() { + // TODO: Move this to the NRP layer so that every Channel doesn't repeat the same work. if (receiver == null) { receiver = new PackageChangeReceiver(); IntentFilter filter = new IntentFilter(); diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java index 1e2b80b2fdb..1a436c52d05 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java @@ -165,11 +165,19 @@ public Builder setSecurityPolicy(SecurityPolicy securityPolicy) { return this; } + public SecurityPolicy getSecurityPolicy() { + return securityPolicy; + } + public Builder setTargetUserHandle(@Nullable UserHandle targetUserHandle) { this.targetUserHandle = targetUserHandle; return this; } + public UserHandle getTargetUserHandle() { + return targetUserHandle; + } + public Builder setBindServiceFlags(BindServiceFlags bindServiceFlags) { this.bindServiceFlags = checkNotNull(bindServiceFlags, "bindServiceFlags"); return this; 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 81a39023197..971e5949ee5 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,9 @@ 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 +667,10 @@ public void setTracingEnabled(boolean value) { tracingEnabled = value; } + public void setChannelAttributes(@ChannelAttr Attributes channelAttributes) { + this.channelAttributes = channelAttributes; + } + /** * Verifies the authority is valid. */ @@ -937,4 +945,11 @@ public ObjectPool getOffloadExecutorPool() { public NameResolverRegistry getNameResolverRegistry() { return nameResolverRegistry; } + + /** + * Returns the target string. + */ + public String getTarget() { + return target; + } }