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
diff --git a/camera/aidl/android/hardware/CameraIdRemapping.aidl b/camera/aidl/android/hardware/CameraIdRemapping.aidl
index ab0a39c..e875c53 100644
--- a/camera/aidl/android/hardware/CameraIdRemapping.aidl
+++ b/camera/aidl/android/hardware/CameraIdRemapping.aidl
@@ -35,16 +35,16 @@
* Ordered list of Camera Ids to replace. Only Camera Ids present in this list will be
* affected.
*/
- String[] cameraIdToReplace;
+ List<String> cameraIdsToReplace;
/**
- * Ordered list of updated Camera Ids, where updatedCameraId[i] corresponds to
- * the updated camera id for cameraIdToReplace[i].
+ * Ordered list of updated Camera Ids, where updatedCameraIds[i] corresponds to
+ * the updated camera id for cameraIdsToReplace[i].
*/
- String[] updatedCameraId;
+ List<String> updatedCameraIds;
}
/**
* List of Camera Id remappings to perform.
*/
- List<PackageIdRemapping> packageIdRemapping;
+ List<PackageIdRemapping> packageIdRemappings;
}
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();
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index b9966b3..65b11e7 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -964,8 +964,8 @@
/** Parses cameraIdRemapping parcelable into the native cameraIdRemappingMap. */
binder::Status parseCameraIdRemapping(
- const hardware::CameraIdRemapping& cameraIdRemapping,
- TCameraIdRemapping cameraIdRemappingMap);
+ const hardware::CameraIdRemapping& cameraIdRemapping,
+ /* out */ TCameraIdRemapping* cameraIdRemappingMap);
/**
* Resolve the (potentially remapped) camera Id to use for packageName.