[Bluetooth] Prevent accessing inappropriate memory areas 88/246188/10
authorPawel Wasowski <p.wasowski2@samsung.com>
Wed, 21 Oct 2020 23:13:27 +0000 (01:13 +0200)
committerPawel Wasowski <p.wasowski2@samsung.com>
Wed, 4 Nov 2020 12:01:43 +0000 (12:01 +0000)
This commit prevents from accessing inappropriate memory areas,
leading to crashes.

The first crash scenario:
1. connect to a remote GATT server:
device.connect(<callbacks>) // "device" was passed as an argument to
BluetoothLEAdapter::startScan() callback
2. save one of the services to a variable:
var service = device.getService(<a UUID>);
3. disconnect from the server:
device.disconnect(<callbacks>)
4. connect to the same GATT server again:
device.connect(<callbacks>); // "device" represents the same
device as in the 1st step
5. get the same service as before:
var service_new = device.getService(<the same UUID as in 2.>)
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 <p.wasowski2@samsung.com>
src/bluetooth/bluetooth_api.js
src/bluetooth/bluetooth_gatt_client_service.cc
src/bluetooth/bluetooth_gatt_client_service.h
src/bluetooth/bluetooth_le_device.cc

index 826cbb9e13922334adebeea00a1161390d57b13b..c93da64d0b5f88889ce17daca3170b6e14376cf0 100755 (executable)
@@ -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_
         };
index fd7a53190d465c3c11a58f0da52a3b8a8780b7a2..175ebe580e2862b703bde272e8a19ffb81f167f6 100644 (file)
@@ -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<std::string>();
+  auto gatt_handle_id = static_cast<int>(json.get(kHandleId).get<double>());
+  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<double>(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<long>(args.get("handle").get<double>());
-  const std::string& address = args.get("address").get<std::string>();
+  const std::string& address = args.get(kAddress).get<std::string>();
+
+  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<int> gatt_handle_ids_to_remove_in_case_of_error;
+  struct UserData {
+    picojson::array* array;
+    GATTClient* gatt_client;
+    std::vector<int>* 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<picojson::array*>(data)->push_back(result);
+        UserData* user_data = static_cast<UserData*>(data);
+        auto gatt_handle_id = user_data->gatt_client->AddGATTHandle(gatt_handle);
+        result_obj.insert(std::make_pair(kHandleId, static_cast<double>(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<void*>(&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<long>(args.get("handle").get<double>());
+  const std::string& address = args.get(kAddress).get<std::string>();
 
-  const std::string& address = args.get("address").get<std::string>();
+  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<int> gatt_handle_ids_to_remove_in_case_of_error;
+
+  auto& gatt_client = gatt_clients_.at(address);
   struct Data {
+    GATTClient* gatt_client;
+    std::vector<int>* 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*>(data);
         picojson::array* array = user_data->array;
         PlatformResult* platform_result = user_data->platform_res;
+        GATTClient* gatt_client = user_data->gatt_client;
+        std::vector<int>* 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<picojson::object>();
 
-        // 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<double>(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<picojson::array>();
+        struct DescriptorsData {
+          GATTClient* gatt_client;
+          std::vector<int>* 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<picojson::array*>(data));
-
+              DescriptorsData* descriptors_data = static_cast<DescriptorsData*>(data);
+              picojson::array* desc_array = descriptors_data->desc_array;
+              GATTClient* gatt_client = descriptors_data->gatt_client;
+              std::vector<int>* 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<picojson::object>();
 
-              // 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<double>(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<void*>(&desc_array));
+            static_cast<void*>(&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<std::string>();
+  const std::string& address = args.get(kAddress).get<std::string>();
   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<long>(args.get("handle").get<double>());
+
+  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<std::string>();
+  const std::string& address = args.get(kAddress).get<std::string>();
   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<picojson::array>();
+  const picojson::array& value_array = args.get(kValue).get<picojson::array>();
 
   int value_size = value_array.size();
   std::unique_ptr<char[]> 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<double>();
   }
 
+  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<long>(args.get("handle").get<double>());
-
   auto write_value = [](int result, bt_gatt_h handle, void* user_data) -> void {
     ScopeLogger("Entered into asynchronous function, write_value");
     Data* data = static_cast<Data*>(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<picojson::value> response =
         std::shared_ptr<picojson::value>(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<std::string>();
+
+  const auto& address = args.get(kAddress).get<std::string>();
   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<long>(args.get(kHandle).get<double>());
+  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<int>(args.get(kHandleId).get<double>());
+  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<std::string>();
+
+  const auto& address = args.get(kAddress).get<std::string>();
   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<long>(args.get(kHandle).get<double>());
+    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<int>(args.get(kHandleId).get<double>());
+    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<picojson::object>();
 
-  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<double>(listened_gatt_handle_id_it->second))));
   picojson::value byte_array = picojson::value(picojson::array());
   picojson::array& byte_array_obj = byte_array.get<picojson::array>();
 
index 9f4056585e512d978a11257ecdf827e80b42692e..e3162e00df2916c6fd9cac4c5e7559920ff33c85 100644 (file)
@@ -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<std::string, bt_gatt_client_h> gatt_clients_;
-  std::vector<bt_gatt_h> gatt_characteristic_;
+  struct GATTClient {
+    GATTClient(bt_gatt_client_h handle) : handle{handle} {
+    }
+    bt_gatt_client_h handle;
+    std::map<int, bt_gatt_h> 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<std::string, GATTClient>;
+  GATTClientsMap::iterator CreateGATTClient(const std::string& address);
+
+  std::map<std::string, GATTClient> gatt_clients_;
+  common::PlatformResult GATTHandleFromJSON(const picojson::value& json,
+                                            const std::string& gatt_object_type, bt_gatt_h* result);
+  std::map<bt_gatt_h, int> listened_gatt_handle_to_id;
 
   BluetoothInstance& instance_;
 };
index 6af1201a0aacd9e845036b542f512a73acf999c3..bb20d59dd7a7f4e3349db101203047950cbb448c 100644 (file)
@@ -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<picojson::object>();
 
-  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<std::string>(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);