commit 214f6e62d77ddad430ebbb15854ac9852d9ee5c8
author malikakash <malikakash@google.com> Thu Aug 10 23:50:56 2023 +0000
committer Akash Malik <malikakash@google.com> Fri Aug 11 05:40:27 2023 +0000
tree 12b3bcd8629fd041c90543dba36804b59d667c91
parent 82ed435e2cb0ab45364202917a17d5bd241abace

Harden remapCameraApi() API

- Pluralize list fields, it is easier
  to work with.
- Change String[] to List<String>. This
  has no effect on the cpp handling, it
  just makes the java client code easier
  to work with.
- Check against more edge cases.
- Fix small bug in parsing found after
  E2E testing (pass argument by reference)

Bug: 286287541
Test: Tested the binder calls using custom service.
      Was able to trigger all edge cases.
Change-Id: I9dedf5aeace3ac7da32b6a362c1fc9c7fd7c98ce
Merged-In: I176959e56563bc587c4fca594cfc242941b503d6

services/camera/libcameraservice/CameraService.cpp

diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 7551bd7..9f52ddf 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -766,7 +766,7 @@
                 "Permission Denial: no permission to configure camera id mapping");
     }
     TCameraIdRemapping cameraIdRemappingMap{};
-    binder::Status parseStatus = parseCameraIdRemapping(cameraIdRemapping, cameraIdRemappingMap);
+    binder::Status parseStatus = parseCameraIdRemapping(cameraIdRemapping, &cameraIdRemappingMap);
     if (!parseStatus.isOk()) {
         return parseStatus;
     }
@@ -776,25 +776,37 @@
 
 Status CameraService::parseCameraIdRemapping(
         const hardware::CameraIdRemapping& cameraIdRemapping,
-        TCameraIdRemapping cameraIdRemappingMap) {
+        /* out */ TCameraIdRemapping* cameraIdRemappingMap) {
     String16 packageName;
     String8 cameraIdToReplace, updatedCameraId;
-    for(const auto& packageIdRemapping: cameraIdRemapping.packageIdRemapping) {
+    for(const auto& packageIdRemapping: cameraIdRemapping.packageIdRemappings) {
         packageName = packageIdRemapping.packageName;
         if (packageName == String16("")) {
             return STATUS_ERROR(ERROR_ILLEGAL_ARGUMENT,
                     "CameraIdRemapping: Package name cannot be empty");
         }
-        if (packageIdRemapping.cameraIdToReplace.size()
-            != packageIdRemapping.updatedCameraId.size()) {
+
+        if (packageIdRemapping.cameraIdsToReplace.size()
+            != packageIdRemapping.updatedCameraIds.size()) {
             return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT,
                     "CameraIdRemapping: Mismatch in CameraId Remapping lists sizes for package %s",
                      String8(packageName).c_str());
         }
-        for(size_t i = 0; i < packageIdRemapping.cameraIdToReplace.size(); i++) {
-            cameraIdToReplace = String8(packageIdRemapping.cameraIdToReplace[i]);
-            updatedCameraId = String8(packageIdRemapping.updatedCameraId[i]);
-            cameraIdRemappingMap[packageName][cameraIdToReplace] = updatedCameraId;
+        for(size_t i = 0; i < packageIdRemapping.cameraIdsToReplace.size(); i++) {
+            cameraIdToReplace = String8(packageIdRemapping.cameraIdsToReplace[i]);
+            updatedCameraId = String8(packageIdRemapping.updatedCameraIds[i]);
+            if (cameraIdToReplace == String8("") || updatedCameraId == String8("")) {
+                return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT,
+                        "CameraIdRemapping: Camera Id cannot be empty for package %s",
+                        String8(packageName).c_str());
+            }
+            if (cameraIdToReplace == updatedCameraId) {
+                return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT,
+                        "CameraIdRemapping: CameraIdToReplace cannot be the same"
+                        " as updatedCameraId for %s",
+                        String8(packageName).c_str());
+            }
+            (*cameraIdRemappingMap)[packageName][cameraIdToReplace] = updatedCameraId;
         }
     }
     return Status::ok();