Add ScheduleInfo validity check.
Define the max task data size. Requires remote access HAL to return
invalid arg if ScheduleInfo is not valid.
Updated the reference impl to add the checks.
Test: atest RemoteAccessServiceUnitTest
Bug: 317405128
Change-Id: Ia17dda2683c3bcc861542cb2fbd812ce8bd368aa
diff --git a/automotive/remoteaccess/aidl_api/android.hardware.automotive.remoteaccess/current/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl b/automotive/remoteaccess/aidl_api/android.hardware.automotive.remoteaccess/current/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl
index a5d81cf..ec8733a 100644
--- a/automotive/remoteaccess/aidl_api/android.hardware.automotive.remoteaccess/current/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl
+++ b/automotive/remoteaccess/aidl_api/android.hardware.automotive.remoteaccess/current/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl
@@ -41,4 +41,5 @@
int count;
long startTimeInEpochSeconds;
long periodicInSeconds;
+ const int MAX_TASK_DATA_SIZE_IN_BYTES = 10240;
}
diff --git a/automotive/remoteaccess/android/hardware/automotive/remoteaccess/IRemoteAccess.aidl b/automotive/remoteaccess/android/hardware/automotive/remoteaccess/IRemoteAccess.aidl
index 705cdbd..f0468c4 100644
--- a/automotive/remoteaccess/android/hardware/automotive/remoteaccess/IRemoteAccess.aidl
+++ b/automotive/remoteaccess/android/hardware/automotive/remoteaccess/IRemoteAccess.aidl
@@ -167,6 +167,9 @@
* {@code scheduleId} for this client exists.
*
* <p>Must return {@code EX_ILLEGAL_ARGUMENT} if the task type is not supported.
+ *
+ * <p>Must return {@code EX_ILLEGLA_ARGUMENT} if the scheduleInfo is not valid (e.g. count is
+ * a negative number).
*/
void scheduleTask(in ScheduleInfo scheduleInfo);
diff --git a/automotive/remoteaccess/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl b/automotive/remoteaccess/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl
index 40fba6f..4f2537c 100644
--- a/automotive/remoteaccess/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl
+++ b/automotive/remoteaccess/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl
@@ -21,6 +21,7 @@
@VintfStability
@JavaDerive(equals=true, toString=true)
parcelable ScheduleInfo {
+ const int MAX_TASK_DATA_SIZE_IN_BYTES = 10240;
/**
* The ID used to identify the client this schedule is for. This must be one of the
* preconfigured remote access serverless client ID defined in car service resource
@@ -41,6 +42,8 @@
* executed. It is not interpreted/parsed by the Android system.
*
* <p>This is only used for {@code TaskType.CUSTOM}.
+ *
+ * <p>The data size must be less than {@link MAX_TASK_DATA_SIZE_IN_BYTES}.
*/
byte[] taskData;
/**
diff --git a/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp b/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp
index 2a7f209..1b42a1f 100644
--- a/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp
+++ b/automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp
@@ -331,6 +331,24 @@
ClientContext context;
ScheduleTaskRequest request = {};
ScheduleTaskResponse response = {};
+
+ if (scheduleInfo.count < 0) {
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "count must be >= 0");
+ }
+ if (scheduleInfo.startTimeInEpochSeconds < 0) {
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "startTimeInEpochSeconds must be >= 0");
+ }
+ if (scheduleInfo.periodicInSeconds < 0) {
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "periodicInSeconds must be >= 0");
+ }
+ if (scheduleInfo.taskData.size() > scheduleInfo.MAX_TASK_DATA_SIZE_IN_BYTES) {
+ return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,
+ "task data too big");
+ }
+
request.mutable_scheduleinfo()->set_clientid(scheduleInfo.clientId);
request.mutable_scheduleinfo()->set_scheduleid(scheduleInfo.scheduleId);
request.mutable_scheduleinfo()->set_data(scheduleInfo.taskData.data(),
@@ -348,7 +366,8 @@
case ErrorCode::OK:
return ScopedAStatus::ok();
case ErrorCode::INVALID_ARG:
- return ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
+ return ScopedAStatus::fromExceptionCodeWithMessage(
+ EX_ILLEGAL_ARGUMENT, "received invalid_arg from grpc server");
default:
// Should not happen.
return ScopedAStatus::fromServiceSpecificErrorWithMessage(
diff --git a/automotive/remoteaccess/hal/default/test/RemoteAccessServiceUnitTest.cpp b/automotive/remoteaccess/hal/default/test/RemoteAccessServiceUnitTest.cpp
index a46a983..7992a50 100644
--- a/automotive/remoteaccess/hal/default/test/RemoteAccessServiceUnitTest.cpp
+++ b/automotive/remoteaccess/hal/default/test/RemoteAccessServiceUnitTest.cpp
@@ -473,7 +473,71 @@
EXPECT_EQ(grpcRequest.scheduleinfo().periodicinseconds(), kTestPeriodicInSeconds);
}
-TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidArg) {
+TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidCount) {
+ ScheduleInfo scheduleInfo = {
+ .clientId = kTestClientId,
+ .scheduleId = kTestScheduleId,
+ .taskData = kTestData,
+ .count = -1,
+ .startTimeInEpochSeconds = kTestStartTimeInEpochSeconds,
+ .periodicInSeconds = kTestPeriodicInSeconds,
+ };
+
+ ScopedAStatus status = getService()->scheduleTask(scheduleInfo);
+
+ ASSERT_FALSE(status.isOk());
+ ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);
+}
+
+TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidStartTimeInEpochSeconds) {
+ ScheduleInfo scheduleInfo = {
+ .clientId = kTestClientId,
+ .scheduleId = kTestScheduleId,
+ .taskData = kTestData,
+ .count = kTestCount,
+ .startTimeInEpochSeconds = -1,
+ .periodicInSeconds = kTestPeriodicInSeconds,
+ };
+
+ ScopedAStatus status = getService()->scheduleTask(scheduleInfo);
+
+ ASSERT_FALSE(status.isOk());
+ ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);
+}
+
+TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidPeriodicInSeconds) {
+ ScheduleInfo scheduleInfo = {
+ .clientId = kTestClientId,
+ .scheduleId = kTestScheduleId,
+ .taskData = kTestData,
+ .count = kTestCount,
+ .startTimeInEpochSeconds = kTestStartTimeInEpochSeconds,
+ .periodicInSeconds = -1,
+ };
+
+ ScopedAStatus status = getService()->scheduleTask(scheduleInfo);
+
+ ASSERT_FALSE(status.isOk());
+ ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);
+}
+
+TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_TaskDataTooLarge) {
+ ScheduleInfo scheduleInfo = {
+ .clientId = kTestClientId,
+ .scheduleId = kTestScheduleId,
+ .taskData = std::vector<uint8_t>(ScheduleInfo::MAX_TASK_DATA_SIZE_IN_BYTES + 1),
+ .count = kTestCount,
+ .startTimeInEpochSeconds = kTestStartTimeInEpochSeconds,
+ .periodicInSeconds = kTestPeriodicInSeconds,
+ };
+
+ ScopedAStatus status = getService()->scheduleTask(scheduleInfo);
+
+ ASSERT_FALSE(status.isOk());
+ ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);
+}
+
+TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidArgFromGrpcServer) {
EXPECT_CALL(*getGrpcWakeupClientStub(), ScheduleTask)
.WillOnce([]([[maybe_unused]] ClientContext* context,
[[maybe_unused]] const ScheduleTaskRequest& request,