!249 read remote object bugfix

Merge pull request !249 from pilipala195/bugfix_remote_object

ipc/native/src/mock/include/dbinder_base_invoker.h

@@ -103,16 +103,16 @@ public:
     virtual void SetCallerTokenID(const uint32_t tokenId) = 0;
     virtual int CheckAndSetCallerInfo(uint32_t listenFd, uint64_t stubIndex) = 0;
     virtual int OnSendRawData(std::shared_ptr<T> session, const void *data, size_t size) = 0;
-    virtual bool SetTokenId(const dbinder_transaction_data *tr, uint32_t listenFd) = 0;
+    virtual bool SetTokenId(const dbinder_transaction_data *tr, std::shared_ptr<T> sessionObject) = 0;
     bool CheckTransactionData(const dbinder_transaction_data *tr) const;
 
 private:
     uint32_t TranslateBinderType(flat_binder_object *binderObject, char *sessionOffset, std::shared_ptr<T> session);
     uint32_t TranslateHandleType(flat_binder_object *binderObject, char *sessionOffset, std::shared_ptr<T> session);
-    bool TranslateRemoteHandleType(flat_binder_object *binderObject, char *sessionOffset);
-    int HandleReply(uint64_t seqNumber, MessageParcel *reply);
+    bool TranslateRemoteHandleType(flat_binder_object *binderObject, char *sessionOffset, std::shared_ptr<T> session);
+    int HandleReply(uint64_t seqNumber, MessageParcel *reply, std::shared_ptr<T> sessionObject);
     bool SetSenderStubIndex(std::shared_ptr<dbinder_transaction_data> transData, uint32_t handle);
-    int WaitForReply(uint64_t seqNumber, MessageParcel *reply, uint32_t handle, int userWaitTime);
+    int WaitForReply(uint64_t seqNumber, MessageParcel *reply, std::shared_ptr<T> sessionObject, int userWaitTime);
     void ProcessTransaction(dbinder_transaction_data *tr, uint32_t listenFd);
     void ProcessReply(dbinder_transaction_data *tr, uint32_t listenFd);
     bool IRemoteObjectTranslate(char *dataBuffer, binder_size_t buffer_size, MessageParcel &data, uint32_t socketId,
@@ -201,7 +201,8 @@ template <class T> uint32_t DBinderBaseInvoker<T>::MakeRemoteHandle(std::shared_
 }
 
 template <class T>
-bool DBinderBaseInvoker<T>::TranslateRemoteHandleType(flat_binder_object *binderObject, char *sessionOffset)
+bool DBinderBaseInvoker<T>::TranslateRemoteHandleType(flat_binder_object *binderObject, char *sessionOffset,
+    std::shared_ptr<T> session)
 {
     std::shared_ptr<T> sessionOfPeer = nullptr;
     uint64_t stubIndex = 0;
@@ -214,6 +215,7 @@ bool DBinderBaseInvoker<T>::TranslateRemoteHandleType(flat_binder_object *binder
         DBINDER_BASE_LOGE("send a wrong dbinder object");
         return false;
     }
+    sessionOfPeer->SetFeatureSet(session->GetFeatureSet());
 
     uint32_t handle = QueryHandleBySession(sessionOfPeer, stubIndex);
     if (handle == 0) {
@@ -267,7 +269,8 @@ bool DBinderBaseInvoker<T>::IRemoteObjectTranslate(char *dataBuffer, binder_size
             }
 
             case BINDER_TYPE_REMOTE_HANDLE: {
-                if (TranslateRemoteHandleType(binderObject, flatOffset + i * T::GetFlatSessionLen()) != true) {
+                if (TranslateRemoteHandleType(binderObject, flatOffset + i * T::GetFlatSessionLen(),
+                    sessionObject) != true) {
                     DBINDER_BASE_LOGE("send a wrong dbinder object");
                     return false;
                 }
@@ -561,7 +564,8 @@ std::shared_ptr<T> DBinderBaseInvoker<T>::WriteTransaction(int cmd, uint32_t fla
     return sessionObject;
 }
 
-template <class T> int DBinderBaseInvoker<T>::HandleReply(uint64_t seqNumber, MessageParcel *reply)
+template <class T> int DBinderBaseInvoker<T>::HandleReply(uint64_t seqNumber, MessageParcel *reply,
+    std::shared_ptr<T> sessionObject)
 {
     if (reply == nullptr) {
         DBINDER_BASE_LOGE("no need reply, free the buffer");
@@ -606,7 +610,7 @@ template <class T> int DBinderBaseInvoker<T>::HandleReply(uint64_t seqNumber, Me
     }
 
     if (!IRemoteObjectTranslate(reinterpret_cast<char *>(messageInfo->buffer), messageInfo->bufferSize, *reply,
-        messageInfo->socketId, nullptr)) {
+        messageInfo->socketId, sessionObject)) {
         DBINDER_BASE_LOGE("translate object failed");
         return RPC_BASE_INVOKER_INVALID_REPLY_ERR;
     }
@@ -634,14 +638,15 @@ template <class T> void DBinderBaseInvoker<T>::DBinderRecvAllocator::Dealloc(voi
 }
 
 template <class T>
-int DBinderBaseInvoker<T>::WaitForReply(uint64_t seqNumber, MessageParcel *reply, uint32_t handle, int userWaitTime)
+int DBinderBaseInvoker<T>::WaitForReply(uint64_t seqNumber, MessageParcel *reply,
+    std::shared_ptr<T> sessionObject, int userWaitTime)
 {
     /* if reply == nullptr, this is a one way message */
     if (reply == nullptr) {
         return NO_ERROR;
     }
 
-    std::shared_ptr<ThreadMessageInfo> messageInfo = MakeThreadMessageInfo(handle);
+    std::shared_ptr<ThreadMessageInfo> messageInfo = MakeThreadMessageInfo(sessionObject->GetSessionHandle());
     if (messageInfo == nullptr) {
         DBINDER_BASE_LOGE("make thread message info failed, no memory");
         return RPC_BASE_INVOKER_WAIT_REPLY_ERR;
@@ -658,7 +663,7 @@ int DBinderBaseInvoker<T>::WaitForReply(uint64_t seqNumber, MessageParcel *reply
         return RPC_BASE_INVOKER_WAIT_REPLY_ERR;
     }
 
-    int32_t err = HandleReply(seqNumber, reply);
+    int32_t err = HandleReply(seqNumber, reply, sessionObject);
     current->EraseThreadBySeqNumber(seqNumber);
     messageInfo->buffer = nullptr;
     return err;
@@ -681,7 +686,7 @@ int DBinderBaseInvoker<T>::SendOrWaitForCompletion(int userWaitTime, uint64_t se
         DBINDER_BASE_LOGE("fail to send to remote session with error = %{public}d", returnLen);
         // no return, for msg send failed maybe not mine
     }
-    return WaitForReply(seqNumber, reply, sessionOfPeer->GetSessionHandle(), userWaitTime);
+    return WaitForReply(seqNumber, reply, sessionOfPeer, userWaitTime);
 }
 
 template <class T>
@@ -865,7 +870,12 @@ template <class T> void DBinderBaseInvoker<T>::ProcessTransaction(dbinder_transa
         DBINDER_BASE_LOGE("set user info error, maybe cookie is NOT belong to current caller");
         return;
     }
-    if (SetTokenId(tr, listenFd) != true) {
+    std::shared_ptr<T> sessionObject = QueryClientSessionObject(listenFd);
+    if (sessionObject == nullptr) {
+        DBINDER_BASE_LOGE("session is not exist for listenFd = %u", listenFd);
+        return;
+    }
+    if (SetTokenId(tr, sessionObject) != true) {
         DBINDER_BASE_LOGE("set tokenid failed");
         return;
     }
@@ -883,7 +893,8 @@ template <class T> void DBinderBaseInvoker<T>::ProcessTransaction(dbinder_transa
         DBINDER_BASE_LOGE("stubIndex is invalid");
         return;
     }
-    if (!IRemoteObjectTranslate(reinterpret_cast<char *>(tr->buffer), tr->buffer_size, data, listenFd, nullptr)) {
+    if (!IRemoteObjectTranslate(reinterpret_cast<char *>(tr->buffer), tr->buffer_size, data,
+        listenFd, sessionObject)) {
         DBINDER_BASE_LOGE("translate object failed");
         return;
     }

ipc/native/src/mock/include/dbinder_databus_invoker.h

@@ -98,7 +98,7 @@ private:
     bool ConnectRemoteObject2Session(IRemoteObject *stubObject, uint64_t stubIndex,
         const std::shared_ptr<DBinderSessionObject> sessionObject);
     bool AuthSession2Proxy(uint32_t handle, const std::shared_ptr<DBinderSessionObject> Session);
-    bool SetTokenId(const dbinder_transaction_data *tr, uint32_t listenFd) override;
+    bool SetTokenId(const dbinder_transaction_data *tr, std::shared_ptr<DBinderSessionObject> sessionObject) override;
 
 private:
     DISALLOW_COPY_AND_MOVE(DBinderDatabusInvoker);

ipc/native/src/mock/source/dbinder_databus_invoker.cpp

@@ -683,15 +683,11 @@ bool DBinderDatabusInvoker::IsLocalCalling()
     return false;
 }
 
-bool DBinderDatabusInvoker::SetTokenId(const dbinder_transaction_data *tr, uint32_t listenFd)
+bool DBinderDatabusInvoker::SetTokenId(const dbinder_transaction_data *tr,
+    std::shared_ptr<DBinderSessionObject> sessionObject)
 {
-    if (tr == nullptr) {
-        DBINDER_LOGE("set tokenid tr is null");
-        return false;
-    }
-    std::shared_ptr<DBinderSessionObject> sessionObject = QueryClientSessionObject(listenFd);
     if (sessionObject == nullptr) {
-        DBINDER_LOGE("session is not exist for listenFd = %u", listenFd);
+        DBINDER_LOGE("sessionObject is null");
         return false;
     }
     std::shared_ptr<FeatureSetData> feature = sessionObject->GetFeatureSet();

services/dbinder/test/moduletest/include/rpc_test.h

@@ -31,10 +31,14 @@ public:
         SEND_ASYNC_REPLY = 1,
         SEND_WRONG_REPLY = 2,
         GET_TOKENID = 3,
+        TEST_REMOTE_OBJECT = 4,
+        TEST_ADD = 5,
     };
     std::string GetFooName(void);
     virtual std::string TestGetFooName(void) = 0;
     virtual int32_t TestAccessToken(MessageParcel &data, MessageParcel &reply) = 0;
+    virtual int32_t TestRemoteObject(MessageParcel &data, MessageParcel &reply) = 0;
+    virtual int32_t TestAdd(MessageParcel &data, MessageParcel &reply) = 0;
 public:
     DECLARE_INTERFACE_DESCRIPTOR(u"test.rpc.IRpcFooTest");
 private:
@@ -47,6 +51,8 @@ public:
         MessageParcel &data, MessageParcel &reply, MessageOption &option) override;
     std::string TestGetFooName(void) override;
     int32_t TestAccessToken(MessageParcel &data, MessageParcel &reply) override;
+    int32_t TestRemoteObject(MessageParcel &data, MessageParcel &reply) override;
+    int32_t TestAdd(MessageParcel &data, MessageParcel &reply) override;
 };
 
 class RpcFooProxy : public IRemoteProxy<IRpcFooTest> {
@@ -55,6 +61,8 @@ public:
     ~RpcFooProxy() = default;
     std::string TestGetFooName(void) override;
     int32_t TestAccessToken(MessageParcel &data, MessageParcel &reply) override;
+    int32_t TestRemoteObject(MessageParcel &data, MessageParcel &reply) override;
+    int32_t TestAdd(MessageParcel &data, MessageParcel &reply) override;
 private:
     static inline BrokerDelegator<RpcFooProxy> delegator_;
 };

services/dbinder/test/moduletest/src/rpc_client_test.cpp

@@ -30,6 +30,7 @@ using namespace OHOS;
 #define TEST_NUMS 1000
 static std::string g_deviceId;
 static sptr<IRpcFooTest> g_rpcTestProxy;
+static sptr<IRpcFooTest> g_ipcTestProxy;
 class RpcClientTest : public testing::Test {
 public:
     static constexpr char DBINDER_PKG_NAME[] = "DBinderService";
@@ -105,4 +106,25 @@ HWTEST_F(RpcClientTest, function_test_004, TestSize.Level1)
         }
     }
 }
+
+HWTEST_F(RpcClientTest, function_test_005, TestSize.Level1)
+{
+    auto saMgr = SystemAbilityManagerClient::GetInstance().GetSystemAbilityManager();
+    ASSERT_FALSE(saMgr == nullptr);
+
+    sptr<IRemoteObject> object = saMgr->GetSystemAbility(RPC_TEST_SERVICE2);
+    ASSERT_TRUE(object != nullptr);
+
+    MessageParcel dataParcel, replyParcel;
+    dataParcel.WriteRemoteObject(object);
+    dataParcel.WriteInt32(1);
+    dataParcel.WriteInt32(1);
+    dataParcel.WriteInt32(1);
+    dataParcel.WriteInt32(1);
+    dataParcel.WriteInt32(1);
+    int32_t err = g_rpcTestProxy->TestRemoteObject(dataParcel, replyParcel);
+    ASSERT_EQ(err, ERR_NONE);
+    err = replyParcel.ReadInt32();
+    ASSERT_EQ(err, ERR_NONE);
+}
 }
\ No newline at end of file

services/dbinder/test/moduletest/src/rpc_server_test.cpp

@@ -32,7 +32,10 @@ public:
 
 void RpcServerTest::SetUpTestCase() {}
 
-void RpcServerTest::TearDownTestCase() {}
+void RpcServerTest::TearDownTestCase()
+{
+    IPCSkeleton::JoinWorkThread();
+}
 
 HWTEST_F(RpcServerTest, function_test_001, TestSize.Level1)
 {
@@ -43,7 +46,14 @@ HWTEST_F(RpcServerTest, function_test_001, TestSize.Level1)
     saExtra.isDistributed = true;
     int result = saMgr->AddSystemAbility(RPC_TEST_SERVICE, new RpcFooStub(), saExtra);
     ASSERT_EQ(result, 0);
+}
 
-    IPCSkeleton::JoinWorkThread();
+HWTEST_F(RpcServerTest, function_test_002, TestSize.Level1)
+{
+    auto saMgr = SystemAbilityManagerClient::GetInstance().GetSystemAbilityManager();
+    ASSERT_TRUE(saMgr != nullptr);
+
+    int result = saMgr->AddSystemAbility(RPC_TEST_SERVICE2, new RpcFooStub());
+    ASSERT_EQ(result, 0);
 }
 }
\ No newline at end of file

services/dbinder/test/moduletest/src/rpc_test.cpp

@@ -14,6 +14,7 @@
  */
 
 #include "rpc_test.h"
+#include "iremote_object.h"
 #include "ipc_skeleton.h"
 #include "ipc_types.h"
 
@@ -37,6 +38,14 @@ int RpcFooStub::OnRemoteRequest(uint32_t code,
             result = TestAccessToken(data, reply);
             break;
         }
+        case TEST_REMOTE_OBJECT: {
+            result = TestRemoteObject(data, reply);
+            break;
+        }
+        case TEST_ADD: {
+            result = TestAdd(data, reply);
+            break;
+        }
         default:
             return IPCObjectStub::OnRemoteRequest(code, data, reply, option);
     }
@@ -61,6 +70,27 @@ int32_t RpcFooStub::TestAccessToken(MessageParcel &data, MessageParcel &reply)
     return ERR_NONE;
 }
 
+int32_t RpcFooStub::TestRemoteObject(MessageParcel &data, MessageParcel &reply)
+{
+    sptr<RpcFooProxy> rpcProxy = iface_cast<RpcFooProxy>(data.ReadRemoteObject());
+    if (rpcProxy == nullptr) {
+        reply.WriteInt32(ERR_NULL_OBJECT);
+        return ERR_NULL_OBJECT;
+    }
+    MessageParcel dataParcel, replyParcel;
+    int32_t err = rpcProxy->TestAdd(dataParcel, replyParcel);
+    reply.WriteInt32(err);
+    return err;
+}
+
+int32_t RpcFooStub::TestAdd(MessageParcel &data, MessageParcel &reply)
+{
+    int32_t a = data.ReadInt32();
+    int32_t b = data.ReadInt32();
+    reply.WriteInt32(a + b);
+    return ERR_NONE;
+}
+
 RpcFooProxy::RpcFooProxy(const sptr<IRemoteObject> &impl)
     : IRemoteProxy<IRpcFooTest>(impl)
 {
@@ -83,4 +113,25 @@ int32_t RpcFooProxy::TestAccessToken(MessageParcel &data, MessageParcel &reply)
     int32_t err = Remote()->SendRequest(GET_TOKENID, data, reply, option);
     return err;
 }
+
+int32_t RpcFooProxy::TestRemoteObject(MessageParcel &data, MessageParcel &reply)
+{
+    MessageOption option;
+    int32_t err = Remote()->SendRequest(TEST_REMOTE_OBJECT, data, reply, option);
+    return err;
+}
+
+int32_t RpcFooProxy::TestAdd(MessageParcel &data, MessageParcel &reply)
+{
+    int32_t a = 223;
+    int32_t b = 513;
+    data.WriteInt32(a);
+    data.WriteInt32(b);
+    MessageOption option;
+    int32_t err = Remote()->SendRequest(TEST_ADD, data, reply, option);
+    if (err != ERR_NONE) {
+        return err;
+    }
+    return reply.ReadInt32() == (a + b) ? ERR_NONE : ERR_INVALID_DATA;
+}
 } // namespace OHOS
\ No newline at end of file