diff --git a/interfaces/inner_api/dataobs_manager/include/dataobs_mgr_changeinfo.h b/interfaces/inner_api/dataobs_manager/include/dataobs_mgr_changeinfo.h index 70404e5a31..aa533b15e3 100644 --- a/interfaces/inner_api/dataobs_manager/include/dataobs_mgr_changeinfo.h +++ b/interfaces/inner_api/dataobs_manager/include/dataobs_mgr_changeinfo.h @@ -43,6 +43,7 @@ struct ChangeInfo { uint32_t size_ = 0; VBuckets valueBuckets_ = {}; static constexpr int LIST_MAX_COUNT = 3000; + static constexpr uint32_t MAX_DATA_SIZE = 200 * 1024; }; } // namespace AAFwk } // namespace OHOS diff --git a/services/dataobsmgr/include/dataobs_mgr_service.h b/services/dataobsmgr/include/dataobs_mgr_service.h index 32cada984a..03fc388dde 100644 --- a/services/dataobsmgr/include/dataobs_mgr_service.h +++ b/services/dataobsmgr/include/dataobs_mgr_service.h @@ -125,6 +125,7 @@ private: uint32_t tokenId, int32_t userId); void SubmitNotifyChangeTask(Uri &uri, int32_t userId, std::string readPermission, ObserverInfo &info); + void ReduceTaskCount(); private: static constexpr std::uint32_t TASK_COUNT_MAX = 50; ffrt::mutex taskCountMutex_; diff --git a/services/dataobsmgr/src/dataobs_mgr_changeinfo.cpp b/services/dataobsmgr/src/dataobs_mgr_changeinfo.cpp index 457f0d779a..1befb41cd1 100644 --- a/services/dataobsmgr/src/dataobs_mgr_changeinfo.cpp +++ b/services/dataobsmgr/src/dataobs_mgr_changeinfo.cpp @@ -89,9 +89,9 @@ bool ChangeInfo::Unmarshalling(ChangeInfo &output, MessageParcel &parcel) return false; } - const uint8_t *data = size > 0 ? parcel.ReadBuffer(size) : nullptr; + const uint8_t *data = (size > 0 && size <= ChangeInfo::MAX_DATA_SIZE) ? parcel.ReadBuffer(size) : nullptr; if (size > 0 && data == nullptr) { - LOG_ERROR("Failed to read buffer from parcel."); + LOG_ERROR("Failed to read buffer from parcel. size: %{public}u", size); return false; } VBuckets buckets; diff --git a/services/dataobsmgr/src/dataobs_mgr_service.cpp b/services/dataobsmgr/src/dataobs_mgr_service.cpp index c63570b8ba..49c7ac540b 100644 --- a/services/dataobsmgr/src/dataobs_mgr_service.cpp +++ b/services/dataobsmgr/src/dataobs_mgr_service.cpp @@ -517,6 +517,12 @@ bool DataObsMgrService::IsTaskOverLimit() return false; } +void DataObsMgrService::ReduceTaskCount() +{ + std::lock_guard lck(taskCountMutex_); + --taskCount_; +} + void DataObsMgrService::SubmitNotifyChangeTask(Uri &uri, int32_t userId, std::string readPermission, ObserverInfo &info) { ChangeInfo changeInfo = { ChangeInfo::ChangeType::OTHER, { uri } }; @@ -744,13 +750,10 @@ std::pair> DataObsMgrService::MakeNotifyInfos(Ch Status DataObsMgrService::NotifyChangeExt(const ChangeInfo &changeInfo, DataObsOption opt) { - if (handler_ == nullptr) { - TAG_LOGE(AAFwkTag::DBOBSMGR, "null handler"); - return DATAOBS_SERVICE_HANDLER_IS_NULL; - } - if (dataObsMgrInner_ == nullptr || dataObsMgrInnerExt_ == nullptr) { - LOG_ERROR("dataObsMgrInner_:%{public}d or null dataObsMgrInnerExt", dataObsMgrInner_ == nullptr); - return DATAOBS_SERVICE_INNER_IS_NULL; + if (handler_ == nullptr || dataObsMgrInner_ == nullptr || dataObsMgrInnerExt_ == nullptr) { + LOG_ERROR("handler_: %{public}d, dataObsMgrInner: %{public}d, dataObsMgrInnerExt: %{public}d", + handler_ == nullptr, dataObsMgrInner_ == nullptr, dataObsMgrInnerExt_ == nullptr); + return handler_ == nullptr ? DATAOBS_SERVICE_HANDLER_IS_NULL : DATAOBS_SERVICE_INNER_IS_NULL; } if (!IsCallingPermissionValid(opt)) { return DATAOBS_NOT_SYSTEM_APP; @@ -761,20 +764,23 @@ Status DataObsMgrService::NotifyChangeExt(const ChangeInfo &changeInfo, DataObsO LOG_ERROR("GetCallingUserId fail, type:%{public}d, userId:%{public}d", changeInfo.changeType_, userId); return DATAOBS_INVALID_USERID; } + if (IsTaskOverLimit()) { + return DATAOBS_SERVICE_TASK_LIMMIT; + } ChangeInfo changes; Status result = DeepCopyChangeInfo(changeInfo, changes); if (result != SUCCESS) { LOG_ERROR("copy data failed,changeType:%{public}ud,uris num:%{public}zu,null data:%{public}d,size:%{public}ud", changeInfo.changeType_, changeInfo.uris_.size(), changeInfo.data_ == nullptr, changeInfo.size_); + ReduceTaskCount(); return result; } - if (IsTaskOverLimit()) { - return DATAOBS_SERVICE_TASK_LIMMIT; - } std::vector notifyInfo; std::tie (result, notifyInfo) = MakeNotifyInfos(changes, opt, tokenId, userId); if (changes.uris_.empty()) { TAG_LOGE(AAFwkTag::DBOBSMGR, "uris_ is empty"); + delete [] static_cast(changes.data_); + ReduceTaskCount(); return result; } handler_->SubmitTask([this, changes, userId, tokenId, notifyInfo]() { @@ -787,8 +793,7 @@ Status DataObsMgrService::NotifyChangeExt(const ChangeInfo &changeInfo, DataObsO count++; } delete [] static_cast(changes.data_); - std::lock_guard lck(taskCountMutex_); - --taskCount_; + ReduceTaskCount(); }); return SUCCESS; } diff --git a/test/unittest/dataobs_mgr_service_second_test/dataobs_mgr_service_second_test.cpp b/test/unittest/dataobs_mgr_service_second_test/dataobs_mgr_service_second_test.cpp index c3df1f1b26..34133e8655 100644 --- a/test/unittest/dataobs_mgr_service_second_test/dataobs_mgr_service_second_test.cpp +++ b/test/unittest/dataobs_mgr_service_second_test/dataobs_mgr_service_second_test.cpp @@ -272,6 +272,49 @@ HWTEST_F(DataObsMgrServiceSecondTest, DataObsMgrServiceSecondTest_NotifyChangeEx TAG_LOGI(AAFwkTag::TEST, "DataObsMgrServiceSecondTest_NotifyChangeExt_0400 end"); } +/* + * Feature: DataObsMgrService + * Function: NotifyChangeExt + * SubFunction: NA + * FunctionPoints: DataObsMgrService NotifyChangeExt + * EnvConditions: NA + * CaseDescription: Verify that the DataObsMgrService NotifyChangeExt is abnormal. + */ +HWTEST_F(DataObsMgrServiceSecondTest, DataObsMgrServiceSecondTest_NotifyChangeExt_0500, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "DataObsMgrServiceSecondTest_NotifyChangeExt_0500 start"); + const int testVal = static_cast(DATAOBS_SERVICE_HANDLER_IS_NULL); + Uri uri("dataobs://authority/com.domainname.dataability.persondata/ person/10"); + auto dataObsMgrServer = std::make_shared(); + dataObsMgrServer->handler_ = nullptr; + + EXPECT_EQ(testVal, dataObsMgrServer->NotifyChangeExt({ ChangeInfo::ChangeType::UPDATE, { uri } })); + TAG_LOGI(AAFwkTag::TEST, "DataObsMgrServiceSecondTest_NotifyChangeExt_0500 end"); +} + +/* + * Feature: DataObsMgrService + * Function: NotifyChangeExt + * SubFunction: NA + * FunctionPoints: DataObsMgrService NotifyChangeExt + * EnvConditions: NA + * CaseDescription: Verify that the DataObsMgrService NotifyChangeExt is abnormal. + */ +HWTEST_F(DataObsMgrServiceSecondTest, DataObsMgrServiceSecondTest_NotifyChangeExt_0600, TestSize.Level1) +{ + TAG_LOGI(AAFwkTag::TEST, "DataObsMgrServiceSecondTest_NotifyChangeExt_0600 start"); + const int testVal = static_cast(DATAOBS_SERVICE_INNER_IS_NULL); + Uri uri("dataobs://authority/com.domainname.dataability.persondata/ person/10"); + auto dataObsMgrServer = std::make_shared(); + dataObsMgrServer->Init(); + auto tmp = dataObsMgrServer->dataObsMgrInner_; + dataObsMgrServer->dataObsMgrInner_ = nullptr; + + EXPECT_EQ(testVal, dataObsMgrServer->NotifyChangeExt({ ChangeInfo::ChangeType::UPDATE, { uri } })); + dataObsMgrServer->dataObsMgrInner_ = tmp; + TAG_LOGI(AAFwkTag::TEST, "DataObsMgrServiceSecondTest_NotifyChangeExt_0600 end"); +} + /* * Feature: DataObsMgrService * Function: NotifyProcessObserver