Allow null featureIds in AppOpsService binder calls

Bug: 136595429
Test: atest CtsAppOpsTestCases
Change-Id: I48713e962e3c3b60bd31ef134712a1d2f3da7cf5

4 files changed, 65 insertions, 67 deletions

diff --git a/libs/binder/AppOpsManager.cpp b/libs/binder/AppOpsManager.cpp
index 60f047fd90..4f0b7d31a2 100644
--- a/libs/binder/AppOpsManager.cpp
+++ b/libs/binder/AppOpsManager.cpp
@@ -115,23 +115,19 @@ int32_t AppOpsManager::checkAudioOpNoThrow(int32_t op, int32_t usage, int32_t ui
 }
 
 int32_t AppOpsManager::noteOp(int32_t op, int32_t uid, const String16& callingPackage) {
-    return noteOp(op, uid, callingPackage, String16(), String16());
+    return noteOp(op, uid, callingPackage, std::unique_ptr<String16>(),
+            String16("Legacy AppOpsManager.noteOp call"));
 }
 
 int32_t AppOpsManager::noteOp(int32_t op, int32_t uid, const String16& callingPackage,
-        const String16& featureId, const String16& message) {
+        const std::unique_ptr<String16>& featureId, const String16& message) {
     sp<IAppOpsService> service = getService();
     int32_t mode = service != nullptr
-            ? service->noteOperation(op, uid, callingPackage)
+            ? service->noteOperation(op, uid, callingPackage, featureId)
             : APP_OPS_MANAGER_UNAVAILABLE_MODE;
 
     if (mode == AppOpsManager::MODE_ALLOWED) {
-        if (message.size() == 0) {
-            markAppOpNoted(uid, callingPackage, op, featureId,
-                    String16("noteOp from native code"));
-        } else {
-            markAppOpNoted(uid, callingPackage, op, featureId, message);
-        }
+        markAppOpNoted(uid, callingPackage, op, featureId, message);
     }
 
     return mode;
@@ -139,32 +135,34 @@ int32_t AppOpsManager::noteOp(int32_t op, int32_t uid, const String16& callingPa
 
 int32_t AppOpsManager::startOpNoThrow(int32_t op, int32_t uid, const String16& callingPackage,
         bool startIfModeDefault) {
-    return startOpNoThrow(op, uid, callingPackage, startIfModeDefault, String16(), String16());
+    return startOpNoThrow(op, uid, callingPackage, startIfModeDefault, std::unique_ptr<String16>(),
+            String16("Legacy AppOpsManager.startOpNoThrow call"));
 }
 
 int32_t AppOpsManager::startOpNoThrow(int32_t op, int32_t uid, const String16& callingPackage,
-        bool startIfModeDefault, const String16& featureId, const String16& message) {
+        bool startIfModeDefault, const std::unique_ptr<String16>& featureId,
+        const String16& message) {
     sp<IAppOpsService> service = getService();
     int32_t mode = service != nullptr
             ? service->startOperation(getToken(service), op, uid, callingPackage,
-                    startIfModeDefault) : APP_OPS_MANAGER_UNAVAILABLE_MODE;
+                    featureId, startIfModeDefault) : APP_OPS_MANAGER_UNAVAILABLE_MODE;
 
     if (mode == AppOpsManager::MODE_ALLOWED) {
-        if (message.size() == 0) {
-            markAppOpNoted(uid, callingPackage, op, featureId,
-                    String16("startOp from native code"));
-        } else {
-            markAppOpNoted(uid, callingPackage, op, featureId, message);
-        }
+        markAppOpNoted(uid, callingPackage, op, featureId, message);
     }
 
     return mode;
 }
 
 void AppOpsManager::finishOp(int32_t op, int32_t uid, const String16& callingPackage) {
+    finishOp(op, uid, callingPackage, std::unique_ptr<String16>());
+}
+
+void AppOpsManager::finishOp(int32_t op, int32_t uid, const String16& callingPackage,
+        const std::unique_ptr<String16>& callingFeatureId) {
     sp<IAppOpsService> service = getService();
     if (service != nullptr) {
-        service->finishOperation(getToken(service), op, uid, callingPackage);
+        service->finishOperation(getToken(service), op, uid, callingPackage, callingFeatureId);
     }
 }
 
@@ -207,7 +205,7 @@ bool AppOpsManager::shouldCollectNotes(int32_t opcode) {
 }
 
 void AppOpsManager::markAppOpNoted(int32_t uid, const String16& packageName, int32_t opCode,
-         const String16& featureId, const String16& message) {
+         const std::unique_ptr<String16>& featureId, const String16& message) {
     // check it the appops needs to be collected and cache result
     if (appOpsToNote[opCode] == 0) {
         if (shouldCollectNotes(opCode)) {
@@ -221,11 +219,11 @@ void AppOpsManager::markAppOpNoted(int32_t uid, const String16& packageName, int
         return;
     }
 
-    noteAsyncOp(String16(), uid, packageName, opCode, featureId, message);
+    noteAsyncOp(std::unique_ptr<String16>(), uid, packageName, opCode, featureId, message);
 }
 
-void AppOpsManager::noteAsyncOp(const String16& callingPackageName, int32_t uid,
-         const String16& packageName, int32_t opCode, const String16& featureId,
+void AppOpsManager::noteAsyncOp(const std::unique_ptr<String16>& callingPackageName, int32_t uid,
+         const String16& packageName, int32_t opCode, const std::unique_ptr<String16>& featureId,
          const String16& message) {
     sp<IAppOpsService> service = getService();
     if (service != nullptr) {
diff --git a/libs/binder/IAppOpsService.cpp b/libs/binder/IAppOpsService.cpp
index 9760e135a1..b85a5f298e 100644
--- a/libs/binder/IAppOpsService.cpp
+++ b/libs/binder/IAppOpsService.cpp
@@ -46,12 +46,14 @@ public:
         return reply.readInt32();
     }
 
-    virtual int32_t noteOperation(int32_t code, int32_t uid, const String16& packageName) {
+    virtual int32_t noteOperation(int32_t code, int32_t uid, const String16& packageName,
+                const std::unique_ptr<String16>& featureId) {
         Parcel data, reply;
         data.writeInterfaceToken(IAppOpsService::getInterfaceDescriptor());
         data.writeInt32(code);
         data.writeInt32(uid);
         data.writeString16(packageName);
+        data.writeString16(featureId);
         remote()->transact(NOTE_OPERATION_TRANSACTION, data, &reply);
         // fail on exception
         if (reply.readExceptionCode() != 0) return MODE_ERRORED;
@@ -59,13 +61,15 @@ public:
     }
 
     virtual int32_t startOperation(const sp<IBinder>& token, int32_t code, int32_t uid,
-                const String16& packageName, bool startIfModeDefault) {
+                const String16& packageName, const std::unique_ptr<String16>& featureId,
+                bool startIfModeDefault) {
         Parcel data, reply;
         data.writeInterfaceToken(IAppOpsService::getInterfaceDescriptor());
         data.writeStrongBinder(token);
         data.writeInt32(code);
         data.writeInt32(uid);
         data.writeString16(packageName);
+        data.writeString16(featureId);
         data.writeInt32(startIfModeDefault ? 1 : 0);
         remote()->transact(START_OPERATION_TRANSACTION, data, &reply);
         // fail on exception
@@ -74,13 +78,14 @@ public:
     }
 
     virtual void finishOperation(const sp<IBinder>& token, int32_t code, int32_t uid,
-            const String16& packageName) {
+            const String16& packageName, const std::unique_ptr<String16>& featureId) {
         Parcel data, reply;
         data.writeInterfaceToken(IAppOpsService::getInterfaceDescriptor());
         data.writeStrongBinder(token);
         data.writeInt32(code);
         data.writeInt32(uid);
         data.writeString16(packageName);
+        data.writeString16(featureId);
         remote()->transact(FINISH_OPERATION_TRANSACTION, data, &reply);
     }
 
@@ -144,37 +149,16 @@ public:
         remote()->transact(SET_CAMERA_AUDIO_RESTRICTION_TRANSACTION, data, &reply);
     }
 
-    virtual void noteAsyncOp(const String16& callingPackageName, int32_t uid,
-            const String16& packageName, int32_t opCode, const String16& featureId,
+    virtual void noteAsyncOp(const std::unique_ptr<String16>& callingPackageName, int32_t uid,
+            const String16& packageName, int32_t opCode, const std::unique_ptr<String16>& featureId,
             const String16& message) {
         Parcel data, reply;
         data.writeInterfaceToken(IAppOpsService::getInterfaceDescriptor());
-
-        // Convert empty callingPackage into null string
-        if (callingPackageName.size() != 0) {
-            data.writeString16(callingPackageName);
-        } else {
-            data.writeString16(nullptr, 0);
-        }
-
+        data.writeString16(callingPackageName);
         data.writeInt32(uid);
-
-        // Convert empty packageName into null string
-        if (packageName.size() != 0) {
-            data.writeString16(packageName);
-        } else {
-            data.writeString16(nullptr, 0);
-        }
-
+        data.writeString16(packageName);
         data.writeInt32(opCode);
-
-        // Convert empty featureId into null string
-        if (featureId.size() != 0) {
-            data.writeString16(featureId);
-        } else {
-            data.writeString16(nullptr, 0);
-        }
-
+        data.writeString16(featureId);
         data.writeString16(message);
         remote()->transact(NOTE_ASYNC_OP_TRANSACTION, data, &reply);
     }
@@ -217,7 +201,9 @@ status_t BnAppOpsService::onTransact(
             int32_t code = data.readInt32();
             int32_t uid = data.readInt32();
             String16 packageName = data.readString16();
-            int32_t res = noteOperation(code, uid, packageName);
+            std::unique_ptr<String16> featureId;
+            data.readString16(&featureId);
+            int32_t res = noteOperation(code, uid, packageName, featureId);
             reply->writeNoException();
             reply->writeInt32(res);
             return NO_ERROR;
@@ -228,8 +214,11 @@ status_t BnAppOpsService::onTransact(
             int32_t code = data.readInt32();
             int32_t uid = data.readInt32();
             String16 packageName = data.readString16();
+            std::unique_ptr<String16> featureId;
+            data.readString16(&featureId);
             bool startIfModeDefault = data.readInt32() == 1;
-            int32_t res = startOperation(token, code, uid, packageName, startIfModeDefault);
+            int32_t res = startOperation(token, code, uid, packageName, featureId,
+                    startIfModeDefault);
             reply->writeNoException();
             reply->writeInt32(res);
             return NO_ERROR;
@@ -240,7 +229,9 @@ status_t BnAppOpsService::onTransact(
             int32_t code = data.readInt32();
             int32_t uid = data.readInt32();
             String16 packageName = data.readString16();
-            finishOperation(token, code, uid, packageName);
+            std::unique_ptr<String16> featureId;
+            data.readString16(&featureId);
+            finishOperation(token, code, uid, packageName, featureId);
             reply->writeNoException();
             return NO_ERROR;
         } break;
@@ -296,11 +287,13 @@ status_t BnAppOpsService::onTransact(
         } break;
         case NOTE_ASYNC_OP_TRANSACTION: {
             CHECK_INTERFACE(IAppOpsService, data, reply);
-            String16 callingPackageName = data.readString16();
+            std::unique_ptr<String16> callingPackageName;
+            data.readString16(&callingPackageName);
             int32_t uid = data.readInt32();
             String16 packageName = data.readString16();
             int32_t opCode = data.readInt32();
-            String16 featureId = data.readString16();
+            std::unique_ptr<String16> featureId;
+            data.readString16(&featureId);
             String16 message = data.readString16();
             noteAsyncOp(callingPackageName, uid, packageName, opCode, featureId, message);
             reply->writeNoException();
diff --git a/libs/binder/include/binder/AppOpsManager.h b/libs/binder/include/binder/AppOpsManager.h
index 2744ce126d..22a017941e 100644
--- a/libs/binder/include/binder/AppOpsManager.h
+++ b/libs/binder/include/binder/AppOpsManager.h
@@ -134,21 +134,26 @@ public:
     //              const String16&) instead
     int32_t noteOp(int32_t op, int32_t uid, const String16& callingPackage);
     int32_t noteOp(int32_t op, int32_t uid, const String16& callingPackage,
-            const String16& featureId, const String16& message);
+            const std::unique_ptr<String16>& featureId, const String16& message);
     // @Deprecated, use startOpNoThrow(int32_t, int32_t, const String16&, bool, const String16&,
     //              const String16&) instead
     int32_t startOpNoThrow(int32_t op, int32_t uid, const String16& callingPackage,
             bool startIfModeDefault);
     int32_t startOpNoThrow(int32_t op, int32_t uid, const String16& callingPackage,
-            bool startIfModeDefault, const String16& featureId, const String16& message);
+            bool startIfModeDefault, const std::unique_ptr<String16>& featureId,
+            const String16& message);
+    // @Deprecated, use finishOp(int32_t, int32_t, const String16&, bool, const String16&) instead
     void finishOp(int32_t op, int32_t uid, const String16& callingPackage);
+    void finishOp(int32_t op, int32_t uid, const String16& callingPackage,
+            const std::unique_ptr<String16>& featureId);
     void startWatchingMode(int32_t op, const String16& packageName,
             const sp<IAppOpsCallback>& callback);
     void stopWatchingMode(const sp<IAppOpsCallback>& callback);
     int32_t permissionToOpCode(const String16& permission);
     void setCameraAudioRestriction(int32_t mode);
-    void noteAsyncOp(const String16& callingPackageName, int32_t uid, const String16& packageName,
-            int32_t opCode, const String16& featureId, const String16& message);
+    void noteAsyncOp(const std::unique_ptr<String16>& callingPackageName, int32_t uid,
+            const String16& packageName, int32_t opCode, const std::unique_ptr<String16>& featureId,
+            const String16& message);
 
 private:
     Mutex mLock;
@@ -156,7 +161,7 @@ private:
 
     sp<IAppOpsService> getService();
     void markAppOpNoted(int32_t uid, const String16& packageName, int32_t opCode,
-            const String16& featureId, const String16& message);
+            const std::unique_ptr<String16>& featureId, const String16& message);
     bool shouldCollectNotes(int32_t opCode);
 };
 
diff --git a/libs/binder/include/binder/IAppOpsService.h b/libs/binder/include/binder/IAppOpsService.h
index ad34bc5692..15ba005cec 100644
--- a/libs/binder/include/binder/IAppOpsService.h
+++ b/libs/binder/include/binder/IAppOpsService.h
@@ -35,11 +35,13 @@ public:
     DECLARE_META_INTERFACE(AppOpsService)
 
     virtual int32_t checkOperation(int32_t code, int32_t uid, const String16& packageName) = 0;
-    virtual int32_t noteOperation(int32_t code, int32_t uid, const String16& packageName) = 0;
+    virtual int32_t noteOperation(int32_t code, int32_t uid, const String16& packageName,
+            const std::unique_ptr<String16>& featureId) = 0;
     virtual int32_t startOperation(const sp<IBinder>& token, int32_t code, int32_t uid,
-            const String16& packageName, bool startIfModeDefault) = 0;
+            const String16& packageName, const std::unique_ptr<String16>& featureId,
+            bool startIfModeDefault) = 0;
     virtual void finishOperation(const sp<IBinder>& token, int32_t code, int32_t uid,
-            const String16& packageName) = 0;
+            const String16& packageName, const std::unique_ptr<String16>& featureId) = 0;
     virtual void startWatchingMode(int32_t op, const String16& packageName,
             const sp<IAppOpsCallback>& callback) = 0;
     virtual void stopWatchingMode(const sp<IAppOpsCallback>& callback) = 0;
@@ -48,8 +50,8 @@ public:
     virtual int32_t checkAudioOperation(int32_t code, int32_t usage,int32_t uid,
             const String16& packageName) = 0;
     virtual void setCameraAudioRestriction(int32_t mode) = 0;
-    virtual void noteAsyncOp(const String16& callingPackageName, int32_t uid,
-            const String16& packageName, int32_t opCode, const String16& featureId,
+    virtual void noteAsyncOp(const std::unique_ptr<String16>& callingPackageName, int32_t uid,
+            const String16& packageName, int32_t opCode, const std::unique_ptr<String16>& featureId,
             const String16& message) = 0;
     virtual bool shouldCollectNotes(int32_t opCode) = 0;