Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0894f5b1 authored by malikakash's avatar malikakash
Browse files

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
parent 73125c6c

camera/aidl/android/hardware/CameraIdRemapping.aidl

+5 −5
Original line number Diff line number Diff line
@@ -35,16 +35,16 @@ parcelable CameraIdRemapping { 
         * Ordered list of Camera Ids to replace. Only Camera Ids present in this list will be

         * affected.

         */

        @utf8InCpp String[] cameraIdToReplace;

        @utf8InCpp 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].

         */

        @utf8InCpp String[] updatedCameraId;

        @utf8InCpp List<String> updatedCameraIds;

    }



    /**

     * List of Camera Id remappings to perform.

     */

    List<PackageIdRemapping> packageIdRemapping;

    List<PackageIdRemapping> packageIdRemappings;

}

services/camera/libcameraservice/CameraService.cpp

+21 −10
Original line number Diff line number Diff line
@@ -833,7 +833,7 @@ Status CameraService::remapCameraIds(const hardware::CameraIdRemapping& cameraId 
                "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::remapCameraIds(const hardware::CameraIdRemapping& cameraId 


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();

services/camera/libcameraservice/CameraService.h

+2 −2
Original line number Diff line number Diff line
@@ -989,7 +989,7 @@ private: 
    /** Parses cameraIdRemapping parcelable into the native cameraIdRemappingMap. */

    binder::Status parseCameraIdRemapping(

            const hardware::CameraIdRemapping& cameraIdRemapping,

        TCameraIdRemapping cameraIdRemappingMap);

            /* out */ TCameraIdRemapping* cameraIdRemappingMap);



    /**

     * Resolve the (potentially remapped) camera Id to use for packageName.