commit 0894f5b1119ac1d6d83672aa0b08af559fc8e83f
author malikakash <malikakash@google.com> Thu Aug 10 22:46:47 2023 +0000
committer malikakash <malikakash@google.com> Thu Aug 10 23:54:06 2023 +0000
tree eeb9d2bd0f52050e3b041ef42d05b45fac4e86cd
parent 73125c6caac97cc279279c54dbdb40bb596032e1

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: I176959e56563bc587c4fca594cfc242941b503d6

services/camera/libcameraservice/CameraService.cpp

diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index b37fe9c..ed321b6 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -833,7 +833,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;
     }
@@ -843,25 +843,36 @@
 
 Status CameraService::parseCameraIdRemapping(
         const hardware::CameraIdRemapping& cameraIdRemapping,
-        TCameraIdRemapping cameraIdRemappingMap) {
+        /* out */ TCameraIdRemapping* cameraIdRemappingMap) {
     std::string packageName;
     std::string cameraIdToReplace, updatedCameraId;
-    for(const auto& packageIdRemapping: cameraIdRemapping.packageIdRemapping) {
+    for(const auto& packageIdRemapping: cameraIdRemapping.packageIdRemappings) {
         packageName = packageIdRemapping.packageName;
-        if (packageName == "") {
+        if (packageName.empty()) {
             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",
                     packageName.c_str());
         }
-        for(size_t i = 0; i < packageIdRemapping.cameraIdToReplace.size(); i++) {
-            cameraIdToReplace = std::string(packageIdRemapping.cameraIdToReplace[i]);
-            updatedCameraId = std::string(packageIdRemapping.updatedCameraId[i]);
-            cameraIdRemappingMap[packageName][cameraIdToReplace] = updatedCameraId;
+        for(size_t i = 0; i < packageIdRemapping.cameraIdsToReplace.size(); i++) {
+            cameraIdToReplace = packageIdRemapping.cameraIdsToReplace[i];
+            updatedCameraId = packageIdRemapping.updatedCameraIds[i];
+            if (cameraIdToReplace.empty() || updatedCameraId.empty()) {
+                return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT,
+                        "CameraIdRemapping: Camera Id cannot be empty for package %s",
+                        packageName.c_str());
+            }
+            if (cameraIdToReplace == updatedCameraId) {
+                return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT,
+                        "CameraIdRemapping: CameraIdToReplace cannot be the same"
+                        " as updatedCameraId for %s",
+                        packageName.c_str());
+            }
+            (*cameraIdRemappingMap)[packageName][cameraIdToReplace] = updatedCameraId;
         }
     }
     return Status::ok();