From: Pawel Wasowski Date: Wed, 21 Oct 2020 23:13:27 +0000 (+0200) Subject: [Bluetooth] Prevent accessing inappropriate memory areas X-Git-Tag: submit/tizen/20201209.085808~10^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=094dde72ea2bc9e0cb00800e4d2f2d3f38e66a98;p=platform%2Fcore%2Fapi%2Fwebapi-plugins.git [Bluetooth] Prevent accessing inappropriate memory areas This commit prevents from accessing inappropriate memory areas, leading to crashes. The first crash scenario: 1. connect to a remote GATT server: device.connect() // "device" was passed as an argument to BluetoothLEAdapter::startScan() callback 2. save one of the services to a variable: var service = device.getService(); 3. disconnect from the server: device.disconnect() 4. connect to the same GATT server again: device.connect(); // "device" represents the same device as in the 1st step 5. get the same service as before: var service_new = device.getService() 6. access one of the "service"'s members: service.services 7. The app has crashed The second crash scenario has not been observed, but we anticipate that crashes could happen if the old code were run on 64-bit CPUs (XWALK-2106). The old code has stored bt_gatt_h (typedef for just void*) values from C++ in the JS layer, as "double" variables. On 32-bit architectures, a conversion of a pointer to double (from C++ to JS) and from double to a pointer (from JS back to C++) gives us the same pointer. On 64-bit systems, a pointer may be greater than the greatest integer, guaranteed to have a double representation (Number.MAX_SAFE_INTEGER in JS) and using a memory address, casted back from double may lead to a crash or another unpredictable behavior. [Verification] tct-bluetooth-tizen-tests: auto: 100% pass rate manual Bluetooth04_BLE_wearable: 100% pass rate When the first crash scenario has been followed in ChromeDev Tools, no crash has happened. Change-Id: I52dead1398b2d0aaf21c34c68f6bbcb04ebd3dba Signed-off-by: Pawel Wasowski --- diff --git a/src/bluetooth/bluetooth_api.js b/src/bluetooth/bluetooth_api.js index 826cbb9e..c93da64d 100755 --- a/src/bluetooth/bluetooth_api.js +++ b/src/bluetooth/bluetooth_api.js @@ -1769,7 +1769,7 @@ BluetoothLEAdapter.prototype.removeConnectStateChangeListener = function() { //class BluetoothGATTService /////////////////////////// var BluetoothGATTService = function(data, address) { - var handle_ = data.handle; + var handleId_ = data.handleId; var uuid_ = data.uuid; var serviceUuid_ = data.serviceUuid; //address_ is needed to control if device is still connected @@ -1778,7 +1778,7 @@ var BluetoothGATTService = function(data, address) { function servicesGetter() { var services = []; var result = native.callSync('BluetoothGATTClientServiceGetServices', { - handle: handle_, + handleId: handleId_, address: address_ }); if (native.isSuccess(result)) { @@ -1792,7 +1792,7 @@ var BluetoothGATTService = function(data, address) { function characteristicsGetter() { var characteristics = []; var result = native.callSync('BluetoothGATTClientServiceGetCharacteristics', { - handle: handle_, + handleId: handleId_, uuid: serviceUuid_, address: address_ }); @@ -2178,8 +2178,8 @@ function _registerReadWriteValueRequestCallbacksInGATTService( var count = _countCallbackToRegisterInService(service); if (count === 0) { - successCallback(); - return; + successCallback(); + return; } var callbacksAggregator = new ResultCallbacksAggregator( @@ -2213,7 +2213,7 @@ var BluetoothGATTCharacteristic = function(data, address) { return null; } var address_ = address; - var handle_ = data.handle; + var handleId_ = data.handleId; var descriptors_ = data.descriptors.map(function(descriptor_data) { return new BluetoothGATTDescriptor(descriptor_data, address_); }); @@ -2336,7 +2336,7 @@ var BluetoothGATTCharacteristic = function(data, address) { } }; - var callArgs = { handle: handle_, address: address_ }; + var callArgs = { handleId: handleId_, address: address_ }; var result = native.call( 'BluetoothGATTClientServiceReadValue', @@ -2375,7 +2375,7 @@ var BluetoothGATTCharacteristic = function(data, address) { }; var callArgs = { - handle: handle_, + handleId: handleId_, value: BluetoothManager_toByteArray(value), address: address_ }; @@ -2405,10 +2405,10 @@ var BluetoothGATTCharacteristic = function(data, address) { } ]); - var callArgs = { handle: handle_, address: address_ }; + var callArgs = { handleId: handleId_, address: address_ }; var callback = function(event) { - if (event.handle === handle_) { + if (event.handleId === handleId_) { args.callback(numberArrayToByteArray(native.getResultObject(event))); } }; @@ -2430,7 +2430,7 @@ var BluetoothGATTCharacteristic = function(data, address) { } ]); - var callArgs = { handle: handle_, address: address_ }; + var callArgs = { handleId: handleId_, address: address_ }; return _bluetoothGATTCharacteristicListener.removeListener( args.watchID, @@ -3220,7 +3220,7 @@ var _bleAttMtuChangeListener = _multipleListenerBuilder( //class BluetoothGATTDescriptor /////////////////////////// var BluetoothGATTDescriptor = function(data, address) { - var handle_ = data.handle; + var handleId_ = data.handleId; //address_ is needed to control if device is still connected var address_ = address; var uuid_ = data.uuid; @@ -3249,7 +3249,7 @@ var BluetoothGATTDescriptor = function(data, address) { } }; - var callArgs = { handle: handle_, address: address_ }; + var callArgs = { handleId: handleId_, address: address_ }; var result = native.call( 'BluetoothGATTClientServiceReadValue', @@ -3288,7 +3288,7 @@ var BluetoothGATTDescriptor = function(data, address) { }; var callArgs = { - handle: handle_, + handleId: handleId_, value: BluetoothManager_toByteArray(value), address: address_ }; diff --git a/src/bluetooth/bluetooth_gatt_client_service.cc b/src/bluetooth/bluetooth_gatt_client_service.cc index fd7a5319..175ebe58 100644 --- a/src/bluetooth/bluetooth_gatt_client_service.cc +++ b/src/bluetooth/bluetooth_gatt_client_service.cc @@ -39,8 +39,9 @@ using namespace common::tools; namespace { const std::string kUuid = "uuid"; const std::string kServiceUuid = "serviceUuid"; -const std::string kHandle = "handle"; +const std::string kHandleId = "handleId"; const std::string kAddress = "address"; +const std::string kValue = "value"; const std::string kDescriptors = "descriptors"; const std::string kBroadcast = "isBroadcast"; @@ -54,11 +55,18 @@ const std::string kWriteNoResponse = "isWriteNoResponse"; const std::string kOnValueChanged = "BluetoothGATTCharacteristicValueChangeListener"; +const std::string kGATTService = "GATT service"; +const std::string kGATTCharacteristic = "GATT characteristic"; +const std::string kGATTDescriptor = "GATT descriptor"; +const std::string kGATTObject = "GATT object"; + bool IsProperty(int propertyBits, bt_gatt_property_e property) { return (propertyBits & property) != 0; } } +int BluetoothGATTClientService::GATTClient::next_id_ = 0; + BluetoothGATTClientService::BluetoothGATTClientService(BluetoothInstance& instance) : instance_(instance) { ScopeLogger(); @@ -67,14 +75,14 @@ BluetoothGATTClientService::BluetoothGATTClientService(BluetoothInstance& instan BluetoothGATTClientService::~BluetoothGATTClientService() { ScopeLogger(); - for (auto it : gatt_characteristic_) { + for (const auto& handle_and_id : listened_gatt_handle_to_id) { // unregister callback, ignore errors - bt_gatt_client_unset_characteristic_value_changed_cb(it); + bt_gatt_client_unset_characteristic_value_changed_cb(handle_and_id.first); } - for (auto it : gatt_clients_) { - LoggerD("destroying client for address: %s", it.first.c_str()); - bt_gatt_client_destroy(it.second); + for (const auto& address_and_gatt_client_handle : gatt_clients_) { + LoggerD("destroying client for address: %s", address_and_gatt_client_handle.first.c_str()); + bt_gatt_client_destroy(address_and_gatt_client_handle.second.handle); } } @@ -83,54 +91,88 @@ bool BluetoothGATTClientService::IsStillConnected(const std::string& address) { return gatt_clients_.end() != it; } -bt_gatt_client_h BluetoothGATTClientService::GetGattClient(const std::string& address) { - ScopeLogger(); +BluetoothGATTClientService::GATTClientsMap::iterator BluetoothGATTClientService::CreateGATTClient( + const std::string& address) { + ScopeLogger("address: %s", address.c_str()); bt_gatt_client_h client = nullptr; - const auto it = gatt_clients_.find(address); + const auto client_it = gatt_clients_.find(address); - if (gatt_clients_.end() == it) { + if (client_it == gatt_clients_.end()) { + LoggerD("Client not found"); int ret = bt_gatt_client_create(address.c_str(), &client); if (BT_ERROR_NONE != ret) { LoggerE("Failed to create GATT client, error: %d", ret); } else { - gatt_clients_.insert(std::make_pair(address, client)); + return gatt_clients_.insert(std::make_pair(address, GATTClient{client})).first; } } else { - LoggerD("Client already created"); - client = it->second; + LoggerD("Returning existing client"); } - return client; + return client_it; +} + +PlatformResult BluetoothGATTClientService::GATTHandleFromJSON(const picojson::value& json, + const std::string& gatt_object_type, + bt_gatt_h* result) { + ScopeLogger(); + + const auto& address = json.get(kAddress).get(); + auto gatt_handle_id = static_cast(json.get(kHandleId).get()); + LoggerD("Getting bt_gatt_h for address: %s, handle_id: %d", address.c_str(), gatt_handle_id); + auto& gatt_client = gatt_clients_.at(address); + auto gatt_handle_it = gatt_client.id_to_handle.find(gatt_handle_id); + if (gatt_client.id_to_handle.end() == gatt_handle_it) { + return LogAndCreateResult( + ErrorCode::INVALID_STATE_ERR, "This " + gatt_object_type + " has been invalidated", + ("No handle with %d gatt_handle_id found for address %s", gatt_handle_id, address.c_str())); + } + + *result = gatt_handle_it->second; + return PlatformResult{ErrorCode::NO_ERROR}; +} + +bt_gatt_client_h BluetoothGATTClientService::GetGattClientHandle(const std::string& address) { + ScopeLogger("address: %s", address.c_str()); + + auto client_it = CreateGATTClient(address); + return (gatt_clients_.end() == client_it) ? nullptr : client_it->second.handle; } // this method should be used to inform this object that some device was disconnected void BluetoothGATTClientService::TryDestroyClient(const std::string& address) { - ScopeLogger(); - auto it = gatt_clients_.find(address); - if (gatt_clients_.end() != it) { - LoggerD("destroying client for address: %s", it->first.c_str()); - bt_gatt_client_destroy(it->second); - gatt_clients_.erase(it); + ScopeLogger("address: %s", address.c_str()); + + auto client_it = gatt_clients_.find(address); + if (gatt_clients_.end() != client_it) { + auto ret = bt_gatt_client_destroy(client_it->second.handle); + LoggerD("bt_gatt_client_destroy(): %d (%s)", ret, get_error_message(ret)); + + for (const auto& id_and_handle : client_it->second.id_to_handle) { + listened_gatt_handle_to_id.erase(id_and_handle.second); + } + + gatt_clients_.erase(client_it); } else { LoggerD("Client for address: %s does not exist, no need for deletion", address.c_str()); } } -PlatformResult BluetoothGATTClientService::GetSpecifiedGATTClient(const std::string& address, - const UUID& uuid, - picojson::object* result) { - ScopeLogger(); +PlatformResult BluetoothGATTClientService::GetSpecifiedGATTService(const std::string& address, + const UUID& uuid, + picojson::object* result) { + ScopeLogger("address: %s, UUID: %s", address.c_str(), uuid.uuid_in_source_format.c_str()); - bt_gatt_client_h client = GetGattClient(address); - - if (nullptr == client) { + auto client_it = CreateGATTClient(address); + if (gatt_clients_.end() == client_it) { return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Failed to create the GATT client's handle"); } bt_gatt_h service = nullptr; - int ret = bt_gatt_client_get_service(client, uuid.uuid_128_bit.c_str(), &service); + int ret = + bt_gatt_client_get_service(client_it->second.handle, uuid.uuid_128_bit.c_str(), &service); if (BT_ERROR_NONE != ret) { LoggerE("bt_gatt_client_get_service() error: %d", ret); switch (ret) { @@ -151,6 +193,8 @@ PlatformResult BluetoothGATTClientService::GetSpecifiedGATTClient(const std::str } } + auto gatt_handle_id = client_it->second.AddGATTHandle(service); + /* * BACKWARD COMPATIBILITY * @@ -159,9 +203,8 @@ PlatformResult BluetoothGATTClientService::GetSpecifiedGATTClient(const std::str */ result->insert(std::make_pair(kUuid, picojson::value(uuid.uuid_in_source_format))); result->insert(std::make_pair(kServiceUuid, picojson::value(uuid.uuid_in_source_format))); - // handle is passed to upper layer because there is no need to delete it - result->insert(std::make_pair(kHandle, picojson::value((double)(long)service))); - // address is necessary to later check if device is still connected + result->insert(std::make_pair(kHandleId, picojson::value(static_cast(gatt_handle_id)))); + // address is necessary to later check if device is still connected and retrieve handle result->insert(std::make_pair(kAddress, picojson::value(address))); return PlatformResult(ErrorCode::NO_ERROR); } @@ -169,11 +212,23 @@ PlatformResult BluetoothGATTClientService::GetSpecifiedGATTClient(const std::str void BluetoothGATTClientService::GetServices(const picojson::value& args, picojson::object& out) { ScopeLogger(); - bt_gatt_h handle = (bt_gatt_h) static_cast(args.get("handle").get()); - const std::string& address = args.get("address").get(); + const std::string& address = args.get(kAddress).get(); + + if (!IsStillConnected(address)) { + LogAndReportError(PlatformResult(ErrorCode::INVALID_STATE_ERR, "Device is not connected"), &out, + ("Device with address %s is no longer connected", address.c_str())); + return; + } + + bt_gatt_h gatt_handle = nullptr; + auto result = GATTHandleFromJSON(args, kGATTService, &gatt_handle); + if (!result) { + ReportError(result, &out); + return; + } picojson::array array; - PlatformResult ret = GetServicesHelper(handle, address, &array); + PlatformResult ret = GetServicesHelper(gatt_handle, address, &array); if (ret.IsError()) { LogAndReportError(ret, &out, ("Error while getting services")); } else { @@ -184,12 +239,20 @@ void BluetoothGATTClientService::GetServices(const picojson::value& args, picojs PlatformResult BluetoothGATTClientService::GetServicesHelper(bt_gatt_h handle, const std::string& address, picojson::array* array) { - ScopeLogger(); + /* + * This function expects valid input, i.e. "handle" pointing to an existing + * object, "address" to a connected client and a "array" pointing to an + * existing picojson::array. + */ + ScopeLogger("address: %s", address.c_str()); - if (!IsStillConnected(address)) { - return LogAndCreateResult(ErrorCode::INVALID_STATE_ERR, "Device is not connected", - ("Device with address %s is no longer connected", address.c_str())); - } + auto& gatt_client = gatt_clients_.at(address); + std::vector gatt_handle_ids_to_remove_in_case_of_error; + struct UserData { + picojson::array* array; + GATTClient* gatt_client; + std::vector* gatt_handle_ids_to_remove_in_case_of_error; + } user_data = {array, &gatt_client, &gatt_handle_ids_to_remove_in_case_of_error}; int ret = bt_gatt_service_foreach_included_services( handle, @@ -216,15 +279,24 @@ PlatformResult BluetoothGATTClientService::GetServicesHelper(bt_gatt_h handle, result_obj.insert(std::make_pair(kServiceUuid, picojson::value{})); } - // handle is passed to upper layer because there is no need of deletion - result_obj.insert(std::make_pair(kHandle, picojson::value{(double)(long)gatt_handle})); - static_cast(data)->push_back(result); + UserData* user_data = static_cast(data); + auto gatt_handle_id = user_data->gatt_client->AddGATTHandle(gatt_handle); + result_obj.insert(std::make_pair(kHandleId, static_cast(gatt_handle_id))); + user_data->gatt_handle_ids_to_remove_in_case_of_error->push_back(gatt_handle_id); + + user_data->array->push_back(result); return true; }, - array); + static_cast(&user_data)); + if (BT_ERROR_NONE != ret) { LoggerE("Failed bt_gatt_service_foreach_included_services() (%d)", ret); - return util::GetBluetoothError(ret, "Failed to set a service's GATT callback"); + + for (auto gatt_handle_id : gatt_handle_ids_to_remove_in_case_of_error) { + gatt_client.id_to_handle.erase(gatt_handle_id); + } + + return util::GetBluetoothError(ret, "Failed to get included GATT services"); } return PlatformResult(ErrorCode::NO_ERROR); @@ -234,12 +306,23 @@ void BluetoothGATTClientService::GetCharacteristics(const picojson::value& args, picojson::object& out) { ScopeLogger(); - bt_gatt_h handle = (bt_gatt_h) static_cast(args.get("handle").get()); + const std::string& address = args.get(kAddress).get(); - const std::string& address = args.get("address").get(); + if (!IsStillConnected(address)) { + LogAndReportError(PlatformResult(ErrorCode::INVALID_STATE_ERR, "Device is not connected"), &out, + ("Device with address %s is no longer connected", address.c_str())); + return; + } + + bt_gatt_h gatt_handle = nullptr; + auto result = GATTHandleFromJSON(args, kGATTService, &gatt_handle); + if (!result) { + ReportError(result, &out); + return; + } picojson::array array; - PlatformResult ret = GetCharacteristicsHelper(handle, address, &array); + PlatformResult ret = GetCharacteristicsHelper(gatt_handle, address, &array); if (ret.IsError()) { LogAndReportError(ret, &out, ("Error while getting characteristics")); } else { @@ -250,20 +333,25 @@ void BluetoothGATTClientService::GetCharacteristics(const picojson::value& args, PlatformResult BluetoothGATTClientService::GetCharacteristicsHelper(bt_gatt_h handle, const std::string& address, picojson::array* array) { - ScopeLogger(); + /* + * This function expects valid input, i.e. "handle" pointing to an existing + * object, "address" to a connected client and a "array" pointing to an + * existing picojson::array. + */ - if (!IsStillConnected(address)) { - return LogAndCreateResult(ErrorCode::INVALID_STATE_ERR, "Device is not connected", - ("Device with address %s is no longer connected", address.c_str())); - } + ScopeLogger("address: %s", address.c_str()); + PlatformResult platform_result = PlatformResult(ErrorCode::NO_ERROR); + std::vector gatt_handle_ids_to_remove_in_case_of_error; + + auto& gatt_client = gatt_clients_.at(address); struct Data { + GATTClient* gatt_client; + std::vector* gatt_handle_ids_to_remove_in_case_of_error; picojson::array* array; PlatformResult* platform_res; - }; - - PlatformResult platform_result = PlatformResult(ErrorCode::NO_ERROR); - Data user_data = {array, &platform_result}; + } user_data = {&gatt_client, &gatt_handle_ids_to_remove_in_case_of_error, array, + &platform_result}; int ret = bt_gatt_service_foreach_characteristics( handle, @@ -275,17 +363,29 @@ PlatformResult BluetoothGATTClientService::GetCharacteristicsHelper(bt_gatt_h ha Data* user_data = static_cast(data); picojson::array* array = user_data->array; PlatformResult* platform_result = user_data->platform_res; + GATTClient* gatt_client = user_data->gatt_client; + std::vector* gatt_handle_ids_to_remove_in_case_of_error = + user_data->gatt_handle_ids_to_remove_in_case_of_error; picojson::value result = picojson::value(picojson::object()); picojson::object& result_obj = result.get(); - // handle is passed to upper layer because there is no need of deletion - result_obj.insert(std::make_pair(kHandle, picojson::value((double)(long)gatt_handle))); + auto gatt_handle_id = gatt_client->AddGATTHandle(gatt_handle); + result_obj.insert( + std::make_pair(kHandleId, picojson::value(static_cast(gatt_handle_id)))); + gatt_handle_ids_to_remove_in_case_of_error->push_back(gatt_handle_id); // descriptors picojson::array& desc_array = - result_obj.insert(std::make_pair("descriptors", picojson::value(picojson::array()))) + result_obj.insert(std::make_pair(kDescriptors, picojson::value(picojson::array()))) .first->second.get(); + struct DescriptorsData { + GATTClient* gatt_client; + std::vector* gatt_handle_ids_to_remove_in_case_of_error; + picojson::array* desc_array; + } descriptors_user_data = {gatt_client, gatt_handle_ids_to_remove_in_case_of_error, + &desc_array}; + int ret = bt_gatt_characteristic_foreach_descriptors( gatt_handle, [](int total, int index, bt_gatt_h desc_handle, void* data) { @@ -293,13 +393,18 @@ PlatformResult BluetoothGATTClientService::GetCharacteristicsHelper(bt_gatt_h ha "Entered into asynchronous function, bt_gatt_characteristic_foreach_descriptors;" " index: %d", index); - picojson::array& desc_array = *(static_cast(data)); - + DescriptorsData* descriptors_data = static_cast(data); + picojson::array* desc_array = descriptors_data->desc_array; + GATTClient* gatt_client = descriptors_data->gatt_client; + std::vector* gatt_handle_ids_to_remove_in_case_of_error = + descriptors_data->gatt_handle_ids_to_remove_in_case_of_error; picojson::value desc = picojson::value(picojson::object()); picojson::object& desc_obj = desc.get(); - // handle is passed to upper layer because there is no need of deletion - desc_obj.insert(std::make_pair(kHandle, picojson::value((double)(long)desc_handle))); + auto gatt_handle_id = gatt_client->AddGATTHandle(desc_handle); + desc_obj.insert( + std::make_pair(kHandleId, picojson::value(static_cast(gatt_handle_id)))); + gatt_handle_ids_to_remove_in_case_of_error->push_back(gatt_handle_id); auto uuid = UUID::createFromGatt(desc_handle); if (uuid) { @@ -309,10 +414,10 @@ PlatformResult BluetoothGATTClientService::GetCharacteristicsHelper(bt_gatt_h ha desc_obj.insert(std::make_pair(kUuid, picojson::value{})); } - desc_array.push_back(desc); + desc_array->push_back(desc); return true; }, - static_cast(&desc_array)); + static_cast(&descriptors_user_data)); if (BT_ERROR_NONE != ret) { *platform_result = util::GetBluetoothError(ret, "Failed to get descriptors"); LoggerE("Failed bt_gatt_characteristic_foreach_descriptors() (%d)", ret); @@ -361,7 +466,12 @@ PlatformResult BluetoothGATTClientService::GetCharacteristicsHelper(bt_gatt_h ha return platform_result; } if (BT_ERROR_NONE != ret) { - LoggerE("Failed (%d)", ret); + LoggerE("bt_gatt_service_foreach_characteristics failed: %d (%s)", ret, get_error_message(ret)); + + for (auto& gatt_handle_id : gatt_handle_ids_to_remove_in_case_of_error) { + gatt_client.id_to_handle.erase(gatt_handle_id); + } + return util::GetBluetoothError(ret, "Failed while getting characteristic"); } @@ -373,7 +483,7 @@ void BluetoothGATTClientService::ReadValue(const picojson::value& args, picojson CHECK_BACKWARD_COMPABILITY_PRIVILEGE_ACCESS(Privilege::kBluetooth, Privilege::kBluetoothAdmin, &out); - const std::string& address = args.get("address").get(); + const std::string& address = args.get(kAddress).get(); if (!IsStillConnected(address)) { LogAndReportError(PlatformResult(ErrorCode::INVALID_STATE_ERR, "Device is not connected"), &out, ("Device with address %s is no longer connected", address.c_str())); @@ -387,7 +497,13 @@ void BluetoothGATTClientService::ReadValue(const picojson::value& args, picojson }; Data* user_data = new Data{callback_handle, this}; - bt_gatt_h handle = (bt_gatt_h) static_cast(args.get("handle").get()); + + bt_gatt_h gatt_handle = nullptr; + auto result = GATTHandleFromJSON(args, kGATTObject, &gatt_handle); + if (!result) { + ReportError(result, &out); + return; + } auto read_value = [](int result, bt_gatt_h handle, void* user_data) -> void { ScopeLogger("Entered into asynchronous function, read_value"); @@ -433,7 +549,7 @@ void BluetoothGATTClientService::ReadValue(const picojson::value& args, picojson }, response); }; - int ret = bt_gatt_client_read_value(handle, read_value, (void*)user_data); + int ret = bt_gatt_client_read_value(gatt_handle, read_value, (void*)user_data); if (BT_ERROR_NONE != ret) { delete user_data; user_data = nullptr; @@ -447,7 +563,7 @@ void BluetoothGATTClientService::WriteValue(const picojson::value& args, picojso CHECK_BACKWARD_COMPABILITY_PRIVILEGE_ACCESS(Privilege::kBluetooth, Privilege::kBluetoothAdmin, &out); - const std::string& address = args.get("address").get(); + const std::string& address = args.get(kAddress).get(); if (!IsStillConnected(address)) { LogAndReportError(PlatformResult(ErrorCode::INVALID_STATE_ERR, "Device is not connected"), &out, ("Device with address %s is no longer connected", address.c_str())); @@ -455,7 +571,7 @@ void BluetoothGATTClientService::WriteValue(const picojson::value& args, picojso } const double callback_handle = util::GetAsyncCallbackHandle(args); - const picojson::array& value_array = args.get("value").get(); + const picojson::array& value_array = args.get(kValue).get(); int value_size = value_array.size(); std::unique_ptr value_data(new char[value_size]); @@ -463,13 +579,18 @@ void BluetoothGATTClientService::WriteValue(const picojson::value& args, picojso value_data[i] = (int)value_array[i].get(); } + bt_gatt_h gatt_handle = nullptr; + auto result = GATTHandleFromJSON(args, kGATTObject, &gatt_handle); + if (!result) { + ReportError(result, &out); + return; + } + struct Data { double callback_handle; BluetoothGATTClientService* service; }; - bt_gatt_h handle = (bt_gatt_h) static_cast(args.get("handle").get()); - auto write_value = [](int result, bt_gatt_h handle, void* user_data) -> void { ScopeLogger("Entered into asynchronous function, write_value"); Data* data = static_cast(user_data); @@ -496,8 +617,7 @@ void BluetoothGATTClientService::WriteValue(const picojson::value& args, picojso response); }; - int ret = bt_gatt_set_value(handle, value_data.get(), value_size); - + int ret = bt_gatt_set_value(gatt_handle, value_data.get(), value_size); if (BT_ERROR_NONE != ret) { std::shared_ptr response = std::shared_ptr(new picojson::value(picojson::object())); @@ -511,7 +631,7 @@ void BluetoothGATTClientService::WriteValue(const picojson::value& args, picojso response); } else { Data* user_data = new Data{callback_handle, this}; - ret = bt_gatt_client_write_value(handle, write_value, user_data); + ret = bt_gatt_client_write_value(gatt_handle, write_value, user_data); if (BT_ERROR_NONE != ret) { delete user_data; LoggerE("Couldn't register callback for write value %d (%s)", ret, get_error_message(ret)); @@ -523,23 +643,33 @@ void BluetoothGATTClientService::WriteValue(const picojson::value& args, picojso void BluetoothGATTClientService::AddValueChangeListener(const picojson::value& args, picojson::object& out) { ScopeLogger(); - const auto& address = args.get("address").get(); + + const auto& address = args.get(kAddress).get(); if (!IsStillConnected(address)) { LogAndReportError(PlatformResult(ErrorCode::INVALID_STATE_ERR, "Device is not connected"), &out, ("Device with address %s is no longer connected", address.c_str())); return; } - bt_gatt_h handle = (bt_gatt_h) static_cast(args.get(kHandle).get()); + bt_gatt_h gatt_handle = nullptr; + auto result = GATTHandleFromJSON(args, kGATTObject, &gatt_handle); + if (!result) { + ReportError(result, &out); + return; + } - int ret = bt_gatt_client_set_characteristic_value_changed_cb(handle, OnCharacteristicValueChanged, - this); + auto gatt_handle_id = static_cast(args.get(kHandleId).get()); + LoggerD("Adding change listener for address: %s, gatt_handle_id: %d", address.c_str(), + gatt_handle_id); + + int ret = bt_gatt_client_set_characteristic_value_changed_cb(gatt_handle, + OnCharacteristicValueChanged, this); if (BT_ERROR_NONE != ret) { LogAndReportError(util::GetBluetoothError(ret, "Failed to register listener"), &out, ("bt_gatt_client_set_characteristic_value_changed_cb() failed with: %d (%s)", ret, get_error_message(ret))); } else { - gatt_characteristic_.push_back(handle); + listened_gatt_handle_to_id.insert(std::make_pair(gatt_handle, gatt_handle_id)); ReportSuccess(out); } } @@ -547,21 +677,28 @@ void BluetoothGATTClientService::AddValueChangeListener(const picojson::value& a void BluetoothGATTClientService::RemoveValueChangeListener(const picojson::value& args, picojson::object& out) { ScopeLogger(); - const auto& address = args.get("address").get(); + + const auto& address = args.get(kAddress).get(); if (!IsStillConnected(address)) { LoggerW("Device with address %s is no longer connected", address.c_str()); } else { - bt_gatt_h handle = (bt_gatt_h) static_cast(args.get(kHandle).get()); + bt_gatt_h gatt_handle = nullptr; + auto result = GATTHandleFromJSON(args, kGATTObject, &gatt_handle); + if (!result) { + ReportError(result, &out); + return; + } - int ret = bt_gatt_client_unset_characteristic_value_changed_cb(handle); + auto gatt_handle_id = static_cast(args.get(kHandleId).get()); + LoggerD("Removing change listener of address: %s, gatt_handle_id: %d", address.c_str(), + gatt_handle_id); + int ret = bt_gatt_client_unset_characteristic_value_changed_cb(gatt_handle); if (BT_ERROR_NONE != ret) { LoggerW("bt_gatt_client_unset_characteristic_value_changed_cb() failed with: %d (%s)", ret, get_error_message(ret)); } else { - gatt_characteristic_.erase( - std::remove(gatt_characteristic_.begin(), gatt_characteristic_.end(), handle), - gatt_characteristic_.end()); + listened_gatt_handle_to_id.erase(gatt_handle); } } ReportSuccess(out); @@ -569,11 +706,10 @@ void BluetoothGATTClientService::RemoveValueChangeListener(const picojson::value common::PlatformResult BluetoothGATTClientService::GetServiceAllUuids(const std::string& address, picojson::array* array) { - ScopeLogger(); - - bt_gatt_client_h client = GetGattClient(address); + ScopeLogger("address: %s", address.c_str()); - if (nullptr == client) { + auto client_it = CreateGATTClient(address); + if (gatt_clients_.end() == client_it) { return LogAndCreateResult(ErrorCode::UNKNOWN_ERR, "Unable to create client"); } @@ -603,7 +739,7 @@ common::PlatformResult BluetoothGATTClientService::GetServiceAllUuids(const std: return true; }; - int ret = bt_gatt_client_foreach_services(client, foreach_callback, array); + int ret = bt_gatt_client_foreach_services(client_it->second.handle, foreach_callback, array); if (BT_ERROR_NONE == ret) { return PlatformResult(ErrorCode::NO_ERROR); @@ -627,8 +763,14 @@ void BluetoothGATTClientService::OnCharacteristicValueChanged(bt_gatt_h characte picojson::value result = picojson::value(picojson::object()); picojson::object& result_obj = result.get(); - result_obj.insert(std::make_pair(kHandle, picojson::value((double)(long)characteristic))); + auto listened_gatt_handle_id_it = service->listened_gatt_handle_to_id.find(characteristic); + if (service->listened_gatt_handle_to_id.end() == listened_gatt_handle_id_it) { + LoggerE("The characteristic has been invalidated."); + return; + } + result_obj.insert(std::make_pair( + kHandleId, picojson::value(static_cast(listened_gatt_handle_id_it->second)))); picojson::value byte_array = picojson::value(picojson::array()); picojson::array& byte_array_obj = byte_array.get(); diff --git a/src/bluetooth/bluetooth_gatt_client_service.h b/src/bluetooth/bluetooth_gatt_client_service.h index 9f405658..e3162e00 100644 --- a/src/bluetooth/bluetooth_gatt_client_service.h +++ b/src/bluetooth/bluetooth_gatt_client_service.h @@ -35,8 +35,8 @@ class BluetoothGATTClientService { BluetoothGATTClientService(BluetoothInstance& instance); ~BluetoothGATTClientService(); - common::PlatformResult GetSpecifiedGATTClient(const std::string& address, const UUID& uuid, - picojson::object* result); + common::PlatformResult GetSpecifiedGATTService(const std::string& address, const UUID& uuid, + picojson::object* result); void TryDestroyClient(const std::string& address); void GetServices(const picojson::value& data, picojson::object& out); @@ -46,7 +46,7 @@ class BluetoothGATTClientService { void AddValueChangeListener(const picojson::value& args, picojson::object& out); void RemoveValueChangeListener(const picojson::value& args, picojson::object& out); - bt_gatt_client_h GetGattClient(const std::string& address); + bt_gatt_client_h GetGattClientHandle(const std::string& address); common::PlatformResult GetServiceAllUuids(const std::string& address, picojson::array* array); @@ -61,8 +61,34 @@ class BluetoothGATTClientService { static void OnCharacteristicValueChanged(bt_gatt_h characteristic, char* value, int len, void* user_data); - std::map gatt_clients_; - std::vector gatt_characteristic_; + struct GATTClient { + GATTClient(bt_gatt_client_h handle) : handle{handle} { + } + bt_gatt_client_h handle; + std::map id_to_handle; + + int AddGATTHandle(bt_gatt_h handle) { + id_to_handle[next_id_] = handle; + return next_id_++; + } + + private: + /* + * handleIds have to be unique across single instance, + * because we use a single map (listened_gatt_handle_to_id) to store + * handles and handleIds for all GATTClients. + * Thus, next_id_ is a static member. + */ + static int next_id_; + }; + + using GATTClientsMap = std::map; + GATTClientsMap::iterator CreateGATTClient(const std::string& address); + + std::map gatt_clients_; + common::PlatformResult GATTHandleFromJSON(const picojson::value& json, + const std::string& gatt_object_type, bt_gatt_h* result); + std::map listened_gatt_handle_to_id; BluetoothInstance& instance_; }; diff --git a/src/bluetooth/bluetooth_le_device.cc b/src/bluetooth/bluetooth_le_device.cc index 6af1201a..bb20d59d 100644 --- a/src/bluetooth/bluetooth_le_device.cc +++ b/src/bluetooth/bluetooth_le_device.cc @@ -82,7 +82,7 @@ BluetoothLEDevice::~BluetoothLEDevice() { return; } - bt_gatt_client_h client = service_.GetGattClient(mac_address_); + bt_gatt_client_h client = service_.GetGattClientHandle(mac_address_); if (nullptr == client) { LoggerW("Failed to get client handle while unsetting att mtu change listener"); @@ -417,7 +417,7 @@ void BluetoothLEDevice::GetService(const picojson::value& data, picojson::object picojson::value response = picojson::value(picojson::object()); picojson::object* data_obj = &response.get(); - PlatformResult result = service_.GetSpecifiedGATTClient(address, *uuid, data_obj); + PlatformResult result = service_.GetSpecifiedGATTService(address, *uuid, data_obj); if (result.IsError()) { LogAndReportError(result, &out); @@ -487,7 +487,7 @@ void BluetoothLEDevice::GetAttMtu(const picojson::value& data, picojson::object& mac_address_ == address; } - bt_gatt_client_h client = service_.GetGattClient(address); + bt_gatt_client_h client = service_.GetGattClientHandle(address); int ret = bt_gatt_client_get_att_mtu(client, &mtu); if (BT_ERROR_NONE != ret) { @@ -515,7 +515,7 @@ void BluetoothLEDevice::RequestAttMtuChange(const picojson::value& data, picojso mac_address_ == address; } - bt_gatt_client_h client = service_.GetGattClient(address); + bt_gatt_client_h client = service_.GetGattClientHandle(address); int ret = bt_gatt_client_request_att_mtu_change(client, mtu); if (BT_ERROR_NONE != ret) { @@ -538,7 +538,7 @@ void BluetoothLEDevice::AddAttMtuChangeListener(const picojson::value& data, mac_address_ == address; } - bt_gatt_client_h client = service_.GetGattClient(address); + bt_gatt_client_h client = service_.GetGattClientHandle(address); int ret = bt_gatt_client_set_att_mtu_changed_cb(client, OnAttMtuValueChanged, this); @@ -558,7 +558,7 @@ void BluetoothLEDevice::RemoveAttMtuChangeListener(const picojson::value& data, const auto& args = util::GetArguments(data); const auto& address = common::FromJson(args, kAddress); - bt_gatt_client_h client = service_.GetGattClient(address); + bt_gatt_client_h client = service_.GetGattClientHandle(address); int ret = bt_gatt_client_unset_att_mtu_changed_cb(client);