diff options
4 files changed, 237 insertions, 32 deletions
diff --git a/core/java/android/accessibilityservice/AccessibilityService.java b/core/java/android/accessibilityservice/AccessibilityService.java index f7d75222a6f7..42c32723fd72 100644 --- a/core/java/android/accessibilityservice/AccessibilityService.java +++ b/core/java/android/accessibilityservice/AccessibilityService.java @@ -69,6 +69,7 @@ import android.view.accessibility.AccessibilityNodeInfo.AccessibilityAction; import android.view.accessibility.AccessibilityWindowInfo; import android.view.inputmethod.EditorInfo; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.inputmethod.CancellationGroup; import com.android.internal.inputmethod.IAccessibilityInputMethodSession; import com.android.internal.inputmethod.IAccessibilityInputMethodSessionCallback; @@ -1388,7 +1389,9 @@ public abstract class AccessibilityService extends Service { getFingerprintGestureController().onGesture(gesture); } - int getConnectionId() { + /** @hide */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public int getConnectionId() { return mConnectionId; } diff --git a/core/tests/coretests/src/android/accessibilityservice/BrailleDisplayControllerImplTest.java b/core/tests/coretests/src/android/accessibilityservice/BrailleDisplayControllerImplTest.java new file mode 100644 index 000000000000..aaa199d4c814 --- /dev/null +++ b/core/tests/coretests/src/android/accessibilityservice/BrailleDisplayControllerImplTest.java @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.accessibilityservice; + +import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +import android.hardware.usb.UsbDevice; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; +import android.view.accessibility.AccessibilityEvent; +import android.view.accessibility.AccessibilityInteractionClient; +import android.view.accessibility.Flags; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; + +import java.util.concurrent.Executor; + +/** + * Tests for internal details of BrailleDisplayControllerImpl. + * + * <p>Prefer adding new tests in CTS where possible. + */ +@SmallTest +@RunWith(AndroidJUnit4.class) +@RequiresFlagsEnabled(Flags.FLAG_BRAILLE_DISPLAY_HID) +public class BrailleDisplayControllerImplTest { + private static final int TEST_SERVICE_CONNECTION_ID = 123; + + @Rule + public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule(); + + private BrailleDisplayController mBrailleDisplayController; + + @Mock + private IAccessibilityServiceConnection mAccessibilityServiceConnection; + @Mock + private BrailleDisplayController.BrailleDisplayCallback mBrailleDisplayCallback; + + public static class TestAccessibilityService extends AccessibilityService { + public void onAccessibilityEvent(AccessibilityEvent event) { + } + + public void onInterrupt() { + } + } + + @Before + public void test() { + MockitoAnnotations.initMocks(this); + AccessibilityService accessibilityService = spy(new TestAccessibilityService()); + doReturn((Executor) Runnable::run).when(accessibilityService).getMainExecutor(); + doReturn(TEST_SERVICE_CONNECTION_ID).when(accessibilityService).getConnectionId(); + AccessibilityInteractionClient.addConnection(TEST_SERVICE_CONNECTION_ID, + mAccessibilityServiceConnection, /*initializeCache=*/false); + mBrailleDisplayController = accessibilityService.getBrailleDisplayController(); + } + + // Automated CTS tests only use the BluetoothDevice version of BrailleDisplayController#connect + // because fake UsbDevice objects cannot be created in CTS. This internal test can mock the + // UsbDevice object and at least validate that the correct system_server AIDL call is made. + @Test + public void connect_withUsbDevice_callsConnectUsbBrailleDisplay() throws Exception { + UsbDevice usbDevice = Mockito.mock(UsbDevice.class); + + mBrailleDisplayController.connect(usbDevice, mBrailleDisplayCallback); + + verify(mAccessibilityServiceConnection).connectUsbBrailleDisplay(eq(usbDevice), any()); + } + + @Test + public void connect_serviceNotConnected_throwsIllegalStateException() { + AccessibilityInteractionClient.removeConnection(TEST_SERVICE_CONNECTION_ID); + UsbDevice usbDevice = Mockito.mock(UsbDevice.class); + + assertThrows(IllegalStateException.class, + () -> mBrailleDisplayController.connect(usbDevice, mBrailleDisplayCallback)); + } +} diff --git a/services/accessibility/java/com/android/server/accessibility/BrailleDisplayConnection.java b/services/accessibility/java/com/android/server/accessibility/BrailleDisplayConnection.java index 1f18e15bb646..9b27dd347caf 100644 --- a/services/accessibility/java/com/android/server/accessibility/BrailleDisplayConnection.java +++ b/services/accessibility/java/com/android/server/accessibility/BrailleDisplayConnection.java @@ -42,7 +42,6 @@ import com.android.internal.annotations.VisibleForTesting; import java.io.File; import java.io.FileInputStream; -import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; @@ -112,7 +111,7 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { BrailleDisplayConnection(@NonNull Object lock, @NonNull AccessibilityServiceConnection serviceConnection) { this.mLock = Objects.requireNonNull(lock); - this.mScanner = getDefaultNativeScanner(getDefaultNativeInterface()); + this.mScanner = getDefaultNativeScanner(new DefaultNativeInterface()); this.mServiceConnection = Objects.requireNonNull(serviceConnection); } @@ -128,7 +127,7 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { */ @VisibleForTesting interface BrailleDisplayScanner { - Collection<Path> getHidrawNodePaths(); + Collection<Path> getHidrawNodePaths(@NonNull Path directory); byte[] getDeviceReportDescriptor(@NonNull Path path); @@ -158,8 +157,9 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { Objects.requireNonNull(expectedUniqueId); this.mController = Objects.requireNonNull(controller); + final Path devicePath = Path.of("/dev"); final List<Pair<File, byte[]>> result = new ArrayList<>(); - final Collection<Path> hidrawNodePaths = mScanner.getHidrawNodePaths(); + final Collection<Path> hidrawNodePaths = mScanner.getHidrawNodePaths(devicePath); if (hidrawNodePaths == null) { Slog.w(LOG_TAG, "Unable to access the HIDRAW node directory"); sendConnectionErrorLocked(FLAG_ERROR_CANNOT_ACCESS); @@ -285,6 +285,9 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { if (buffer.length > IBinder.getSuggestedMaxIpcSizeBytes()) { Slog.e(LOG_TAG, "Requested write of size " + buffer.length + " which is larger than maximum " + IBinder.getSuggestedMaxIpcSizeBytes()); + // The caller only got here by bypassing the AccessibilityService-side check with + // reflection, so disconnect this connection to prevent further attempts. + disconnect(); return; } synchronized (mLock) { @@ -292,7 +295,7 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { if (mOutputThread == null) { try { mOutputStream = new FileOutputStream(mHidrawNode); - } catch (FileNotFoundException e) { + } catch (Exception e) { Slog.e(LOG_TAG, "Unable to create write stream", e); disconnect(); return; @@ -387,14 +390,13 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { BrailleDisplayScanner getDefaultNativeScanner(@NonNull NativeInterface nativeInterface) { Objects.requireNonNull(nativeInterface); return new BrailleDisplayScanner() { - private static final Path DEVICE_DIR = Path.of("/dev"); private static final String HIDRAW_DEVICE_GLOB = "hidraw*"; @Override - public Collection<Path> getHidrawNodePaths() { + public Collection<Path> getHidrawNodePaths(@NonNull Path directory) { final List<Path> result = new ArrayList<>(); try (DirectoryStream<Path> hidrawNodePaths = Files.newDirectoryStream( - DEVICE_DIR, HIDRAW_DEVICE_GLOB)) { + directory, HIDRAW_DEVICE_GLOB)) { for (Path path : hidrawNodePaths) { result.add(path); } @@ -458,8 +460,8 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { synchronized (mLock) { mScanner = new BrailleDisplayScanner() { @Override - public Collection<Path> getHidrawNodePaths() { - return brailleDisplayMap.keySet(); + public Collection<Path> getHidrawNodePaths(@NonNull Path directory) { + return brailleDisplayMap.isEmpty() ? null : brailleDisplayMap.keySet(); } @Override @@ -490,38 +492,56 @@ class BrailleDisplayConnection extends IBrailleDisplayConnection.Stub { */ @VisibleForTesting interface NativeInterface { + /** + * Returns the HIDRAW descriptor size for the file descriptor. + * + * @return the result of ioctl(HIDIOCGRDESCSIZE), or -1 if the ioctl fails. + */ int getHidrawDescSize(int fd); + /** + * Returns the HIDRAW descriptor for the file descriptor. + * + * @return the result of ioctl(HIDIOCGRDESC), or null if the ioctl fails. + */ byte[] getHidrawDesc(int fd, int descSize); + /** + * Returns the HIDRAW unique identifier for the file descriptor. + * + * @return the result of ioctl(HIDIOCGRAWUNIQ), or null if the ioctl fails. + */ String getHidrawUniq(int fd); + /** + * Returns the HIDRAW bus type for the file descriptor. + * + * @return the result of ioctl(HIDIOCGRAWINFO).bustype, or -1 if the ioctl fails. + */ int getHidrawBusType(int fd); } /** Native interface that actually calls native HIDRAW ioctls. */ - private NativeInterface getDefaultNativeInterface() { - return new NativeInterface() { - @Override - public int getHidrawDescSize(int fd) { - return nativeGetHidrawDescSize(fd); - } + private class DefaultNativeInterface implements NativeInterface { + @Override + public int getHidrawDescSize(int fd) { + return nativeGetHidrawDescSize(fd); + } - @Override - public byte[] getHidrawDesc(int fd, int descSize) { - return nativeGetHidrawDesc(fd, descSize); - } + @Override + public byte[] getHidrawDesc(int fd, int descSize) { + return nativeGetHidrawDesc(fd, descSize); + } - @Override - public String getHidrawUniq(int fd) { - return nativeGetHidrawUniq(fd); - } + @Override + public String getHidrawUniq(int fd) { + return nativeGetHidrawUniq(fd); + } - @Override - public int getHidrawBusType(int fd) { - return nativeGetHidrawBusType(fd); - } - }; + @Override + public int getHidrawBusType(int fd) { + return nativeGetHidrawBusType(fd); + } } private native int nativeGetHidrawDescSize(int fd); diff --git a/services/tests/servicestests/src/com/android/server/accessibility/BrailleDisplayConnectionTest.java b/services/tests/servicestests/src/com/android/server/accessibility/BrailleDisplayConnectionTest.java index 7c278cedbcc5..b322dd709c2d 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/BrailleDisplayConnectionTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/BrailleDisplayConnectionTest.java @@ -18,22 +18,31 @@ package com.android.server.accessibility; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.accessibilityservice.BrailleDisplayController; +import android.content.Context; import android.os.Bundle; +import android.os.IBinder; import android.testing.DexmakerShareClassLoaderRule; +import androidx.test.platform.app.InstrumentationRegistry; + import com.google.common.truth.Expect; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import java.io.File; import java.nio.file.Path; import java.util.List; @@ -54,6 +63,8 @@ public class BrailleDisplayConnectionTest { @Rule public final Expect expect = Expect.create(); + private Context mContext; + // To mock package-private class @Rule public final DexmakerShareClassLoaderRule mDexmakerShareClassLoaderRule = @@ -62,7 +73,34 @@ public class BrailleDisplayConnectionTest { @Before public void setup() { MockitoAnnotations.initMocks(this); - mBrailleDisplayConnection = new BrailleDisplayConnection(new Object(), mServiceConnection); + mContext = InstrumentationRegistry.getInstrumentation().getContext(); + when(mServiceConnection.isConnectedLocked()).thenReturn(true); + mBrailleDisplayConnection = + spy(new BrailleDisplayConnection(new Object(), mServiceConnection)); + } + + @Test + public void defaultNativeScanner_getHidrawNodePaths_returnsHidrawPaths() throws Exception { + File testDir = mContext.getFilesDir(); + Path hidrawNode0 = Path.of(testDir.getPath(), "hidraw0"); + Path hidrawNode1 = Path.of(testDir.getPath(), "hidraw1"); + Path otherDevice = Path.of(testDir.getPath(), "otherDevice"); + Path[] nodePaths = {hidrawNode0, hidrawNode1, otherDevice}; + try { + for (Path node : nodePaths) { + assertThat(node.toFile().createNewFile()).isTrue(); + } + + BrailleDisplayConnection.BrailleDisplayScanner scanner = + mBrailleDisplayConnection.getDefaultNativeScanner(mNativeInterface); + + assertThat(scanner.getHidrawNodePaths(testDir.toPath())) + .containsExactly(hidrawNode0, hidrawNode1); + } finally { + for (Path node : nodePaths) { + node.toFile().delete(); + } + } } @Test @@ -123,9 +161,38 @@ public class BrailleDisplayConnectionTest { .isEqualTo(BrailleDisplayConnection.BUS_BLUETOOTH); } + @Test + public void write_bypassesServiceSideCheckWithLargeBuffer_disconnects() { + Mockito.doNothing().when(mBrailleDisplayConnection).disconnect(); + mBrailleDisplayConnection.write( + new byte[IBinder.getSuggestedMaxIpcSizeBytes() * 2]); + + verify(mBrailleDisplayConnection).disconnect(); + } + + @Test + public void write_notConnected_throwsIllegalStateException() { + when(mServiceConnection.isConnectedLocked()).thenReturn(false); + + assertThrows(IllegalStateException.class, + () -> mBrailleDisplayConnection.write(new byte[1])); + } + + @Test + public void write_unableToCreateWriteStream_disconnects() { + Mockito.doNothing().when(mBrailleDisplayConnection).disconnect(); + // mBrailleDisplayConnection#connectLocked was never called so the + // connection's mHidrawNode is still null. This will throw an exception + // when attempting to create FileOutputStream on the node. + mBrailleDisplayConnection.write(new byte[1]); + + verify(mBrailleDisplayConnection).disconnect(); + } + // BrailleDisplayConnection#setTestData() is used to enable CTS testing with // test Braille display data, but its own implementation should also be tested // so that issues in this helper don't cause confusing failures in CTS. + @Test public void setTestData_scannerReturnsTestData() { Bundle bd1 = new Bundle(), bd2 = new Bundle(); @@ -148,7 +215,7 @@ public class BrailleDisplayConnectionTest { BrailleDisplayConnection.BrailleDisplayScanner scanner = mBrailleDisplayConnection.setTestData(List.of(bd1, bd2)); - expect.that(scanner.getHidrawNodePaths()).containsExactly(path1, path2); + expect.that(scanner.getHidrawNodePaths(Path.of("/dev"))).containsExactly(path1, path2); expect.that(scanner.getDeviceReportDescriptor(path1)).isEqualTo(desc1); expect.that(scanner.getDeviceReportDescriptor(path2)).isEqualTo(desc2); expect.that(scanner.getUniqueId(path1)).isEqualTo(uniq1); @@ -156,4 +223,12 @@ public class BrailleDisplayConnectionTest { expect.that(scanner.getDeviceBusType(path1)).isEqualTo(bus1); expect.that(scanner.getDeviceBusType(path2)).isEqualTo(bus2); } + + @Test + public void setTestData_emptyTestData_returnsNullNodePaths() { + BrailleDisplayConnection.BrailleDisplayScanner scanner = + mBrailleDisplayConnection.setTestData(List.of()); + + expect.that(scanner.getHidrawNodePaths(Path.of("/dev"))).isNull(); + } } |