[Bluetooth] Standardize Bluetooth device address format to XX:XX:XX:X…

…X:XX:XX. BUG=371014 TEST=device_unittests R=keybuk@chromium.org Review URL: https://codereview.chromium.org/288903003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271179 0039d316-1c4b-4281-b951-d872f2087c98
dednal · May 17, 2014 · db00ff8 · db00ff8
1 parent 1cbaeba
commit db00ff8
Show file tree

Hide file tree

Showing 15 changed files with 117 additions and 27 deletions.
diff --git a/chrome/browser/extensions/api/bluetooth/bluetooth_api.cc b/chrome/browser/extensions/api/bluetooth/bluetooth_api.cc
@@ -296,9 +296,8 @@ bool BluetoothGetDeviceFunction::DoWork(
 
   scoped_ptr<GetDevice::Params> params(GetDevice::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get() != NULL);
-  const std::string& device_address = params->device_address;
 
-  BluetoothDevice* device = adapter->GetDevice(device_address);
+  BluetoothDevice* device = adapter->GetDevice(params->device_address);
   if (device) {
     bluetooth::Device extension_device;
     bluetooth::BluetoothDeviceToApiDevice(*device, &extension_device);

diff --git a/chromeos/dbus/fake_bluetooth_adapter_client.cc b/chromeos/dbus/fake_bluetooth_adapter_client.cc
@@ -27,14 +27,14 @@ const char FakeBluetoothAdapterClient::kAdapterPath[] =
 const char FakeBluetoothAdapterClient::kAdapterName[] =
     "Fake Adapter";
 const char FakeBluetoothAdapterClient::kAdapterAddress[] =
-    "01:1a:2b:1a:2b:03";
+    "01:1A:2B:1A:2B:03";
 
 const char FakeBluetoothAdapterClient::kSecondAdapterPath[] =
     "/fake/hci1";
 const char FakeBluetoothAdapterClient::kSecondAdapterName[] =
     "Second Fake Adapter";
 const char FakeBluetoothAdapterClient::kSecondAdapterAddress[] =
-    "00:de:51:10:01:00";
+    "00:DE:51:10:01:00";
 
 FakeBluetoothAdapterClient::Properties::Properties(
     const PropertyChangedCallback& callback)

diff --git a/device/bluetooth/bluetooth_adapter.cc b/device/bluetooth/bluetooth_adapter.cc
@@ -73,7 +73,12 @@ BluetoothDevice* BluetoothAdapter::GetDevice(const std::string& address) {
 
 const BluetoothDevice* BluetoothAdapter::GetDevice(
     const std::string& address) const {
-  DevicesMap::const_iterator iter = devices_.find(address);
+  std::string canonicalized_address =
+      BluetoothDevice::CanonicalizeAddress(address);
+  if (canonicalized_address.empty())
+    return NULL;
+
+  DevicesMap::const_iterator iter = devices_.find(canonicalized_address);
   if (iter != devices_.end())
     return iter->second;
 

diff --git a/device/bluetooth/bluetooth_adapter_chromeos.cc b/device/bluetooth/bluetooth_adapter_chromeos.cc
@@ -128,7 +128,7 @@ std::string BluetoothAdapterChromeOS::GetAddress() const {
           GetProperties(object_path_);
   DCHECK(properties);
 
-  return properties->address.value();
+  return BluetoothDevice::CanonicalizeAddress(properties->address.value());
 }
 
 std::string BluetoothAdapterChromeOS::GetName() const {

diff --git a/device/bluetooth/bluetooth_adapter_mac.mm b/device/bluetooth/bluetooth_adapter_mac.mm
@@ -249,7 +249,7 @@ - (void)deviceInquiryComplete:(IOBluetoothDeviceInquiry*)sender
 
   if (controller != nil) {
     name = base::SysNSStringToUTF8([controller nameAsString]);
-    address = BluetoothDeviceMac::NormalizeAddress(
+    address = BluetoothDevice::CanonicalizeAddress(
         base::SysNSStringToUTF8([controller addressAsString]));
     powered = ([controller powerState] == kBluetoothHCIPowerStateON);
   }

diff --git a/device/bluetooth/bluetooth_adapter_win.cc b/device/bluetooth/bluetooth_adapter_win.cc
@@ -196,7 +196,7 @@ void BluetoothAdapterWin::AdapterStateChanged(
   name_ = state.name;
   bool was_present = IsPresent();
   bool is_present = !state.address.empty();
-  address_ = state.address;
+  address_ = BluetoothDevice::CanonicalizeAddress(state.address);
   if (was_present != is_present) {
     FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
                       AdapterPresentChanged(this, is_present));

diff --git a/device/bluetooth/bluetooth_adapter_win_unittest.cc b/device/bluetooth/bluetooth_adapter_win_unittest.cc
@@ -15,7 +15,7 @@
 
 namespace {
 
-const char kAdapterAddress[] = "Bluetooth Adapter Address";
+const char kAdapterAddress[] = "A1:B2:C3:D4:E5:F6";
 const char kAdapterName[] = "Bluetooth Adapter Name";
 
 

diff --git a/device/bluetooth/bluetooth_device.cc b/device/bluetooth/bluetooth_device.cc
@@ -6,6 +6,7 @@
 
 #include <string>
 
+#include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "device/bluetooth/bluetooth_gatt_service.h"
 #include "grit/device_bluetooth_strings.h"
@@ -188,4 +189,39 @@ BluetoothGattService* BluetoothDevice::GetGattService(
   return NULL;
 }
 
+// static
+std::string BluetoothDevice::CanonicalizeAddress(const std::string& address) {
+  std::string canonicalized = address;
+  if (address.size() == 12) {
+    // Might be an address in the format "1A2B3C4D5E6F". Add separators.
+    for (size_t i = 2; i < canonicalized.size(); i += 3) {
+      canonicalized.insert(i, ":");
+    }
+  }
+
+  // Verify that the length matches the canonical format "1A:2B:3C:4D:5E:6F".
+  const size_t kCanonicalAddressLength = 17;
+  if (canonicalized.size() != kCanonicalAddressLength)
+    return std::string();
+
+  const char separator = canonicalized[2];
+  for (size_t i = 0; i < canonicalized.size(); ++i) {
+    bool is_separator = (i + 1) % 3 == 0;
+    if (is_separator) {
+      // All separators in the input |address| should be consistent.
+      if (canonicalized[i] != separator)
+        return std::string();
+
+      canonicalized[i] = ':';
+    } else {
+      if (!IsHexDigit(canonicalized[i]))
+        return std::string();
+
+      canonicalized[i] = base::ToUpperASCII(canonicalized[i]);
+    }
+  }
+
+  return canonicalized;
+}
+
 }  // namespace device
diff --git a/device/bluetooth/bluetooth_device.h b/device/bluetooth/bluetooth_device.h
@@ -418,6 +418,11 @@ class BluetoothDevice {
   virtual BluetoothGattService* GetGattService(
       const std::string& identifier) const;
 
+  // Returns the |address| in the canoncial format: XX:XX:XX:XX:XX:XX, where
+  // each 'X' is a hex digit.  If the input |address| is invalid, returns an
+  // empty string.
+  static std::string CanonicalizeAddress(const std::string& address);
+
  protected:
   BluetoothDevice();
 

diff --git a/device/bluetooth/bluetooth_device_chromeos.cc b/device/bluetooth/bluetooth_device_chromeos.cc
@@ -194,7 +194,7 @@ std::string BluetoothDeviceChromeOS::GetAddress() const {
           GetProperties(object_path_);
   DCHECK(properties);
 
-  return properties->address.value();
+  return CanonicalizeAddress(properties->address.value());
 }
 
 BluetoothDevice::VendorIDSource

diff --git a/device/bluetooth/bluetooth_device_mac.h b/device/bluetooth/bluetooth_device_mac.h
@@ -81,12 +81,6 @@ class BluetoothDeviceMac : public BluetoothDevice {
   // normalized format (see below).
   static std::string GetDeviceAddress(IOBluetoothDevice* device);
 
-  // Returns the address formatted according to Chrome's expectations rather
-  // than per the system convention: octets are separated by colons rather than
-  // by dashes. That is, the returned format is XX:XX:XX:XX:XX:XX rather than
-  // xx-xx-xx-xx-xx-xx.
-  static std::string NormalizeAddress(const std::string& address);
-
  protected:
   // BluetoothDevice override
   virtual std::string GetDeviceName() const OVERRIDE;

diff --git a/device/bluetooth/bluetooth_device_mac.mm b/device/bluetooth/bluetooth_device_mac.mm
@@ -252,16 +252,7 @@ @interface IOBluetoothHostController (UndocumentedAPI)
 
 // static
 std::string BluetoothDeviceMac::GetDeviceAddress(IOBluetoothDevice* device) {
-  return NormalizeAddress(base::SysNSStringToUTF8([device addressString]));
-}
-
-// static
-std::string BluetoothDeviceMac::NormalizeAddress(const std::string& address) {
-  std::string normalized;
-  base::ReplaceChars(address, "-", ":", &normalized);
-  // TODO(isherman): Restore StringToUpperASCII(&normalized) call for M37.
-  // http://crbug.com/371014
-  return normalized;
+  return CanonicalizeAddress(base::SysNSStringToUTF8([device addressString]));
 }
 
 }  // namespace device
diff --git a/device/bluetooth/bluetooth_device_unittest.cc b/device/bluetooth/bluetooth_device_unittest.cc
@@ -0,0 +1,59 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/bluetooth/bluetooth_device.h"
+
+#include "base/macros.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace device {
+
+TEST(BluetoothDeviceTest, CanonicalizeAddressFormat_AcceptsAllValidFormats) {
+  // There are three valid separators (':', '-', and none).
+  // Case shouldn't matter.
+  const char* const kValidFormats[] = {
+    "1A:2B:3C:4D:5E:6F",
+    "1a:2B:3c:4D:5e:6F",
+    "1a:2b:3c:4d:5e:6f",
+    "1A-2B-3C-4D-5E-6F",
+    "1a-2B-3c-4D-5e-6F",
+    "1a-2b-3c-4d-5e-6f",
+    "1A2B3C4D5E6F",
+    "1a2B3c4D5e6F",
+    "1a2b3c4d5e6f",
+  };
+
+  for (size_t i = 0; i < arraysize(kValidFormats); ++i) {
+    SCOPED_TRACE(std::string("Input format: '") + kValidFormats[i] + "'");
+    EXPECT_EQ("1A:2B:3C:4D:5E:6F",
+              BluetoothDevice::CanonicalizeAddress(kValidFormats[i]));
+  }
+}
+
+TEST(BluetoothDeviceTest, CanonicalizeAddressFormat_RejectsInvalidFormats) {
+  const char* const kValidFormats[] = {
+    // Empty string.
+    "",
+    // Too short.
+    "1A:2B:3C:4D:5E",
+    // Too long.
+    "1A:2B:3C:4D:5E:6F:70",
+    // Missing a separator.
+    "1A:2B:3C:4D:5E6F",
+    // Mixed separators.
+    "1A:2B-3C:4D-5E:6F",
+    // Invalid characters.
+    "1A:2B-3C:4D-5E:6X",
+    // Separators in the wrong place.
+    "1:A2:B3:C4:D5:E6F",
+  };
+
+  for (size_t i = 0; i < arraysize(kValidFormats); ++i) {
+    SCOPED_TRACE(std::string("Input format: '") + kValidFormats[i] + "'");
+    EXPECT_EQ(std::string(),
+              BluetoothDevice::CanonicalizeAddress(kValidFormats[i]));
+  }
+}
+
+}  // namespace device
diff --git a/device/bluetooth/bluetooth_device_win.cc b/device/bluetooth/bluetooth_device_win.cc
@@ -39,7 +39,7 @@ BluetoothDeviceWin::BluetoothDeviceWin(
       net_log_(net_log),
       net_log_source_(net_log_source) {
   name_ = state.name;
-  address_ = state.address;
+  address_ = CanonicalizeAddress(state.address);
   bluetooth_class_ = state.bluetooth_class;
   visible_ = state.visible;
   connected_ = state.connected;

diff --git a/device/device_tests.gyp b/device/device_tests.gyp
@@ -25,6 +25,7 @@
         'bluetooth/bluetooth_adapter_mac_unittest.mm',
         'bluetooth/bluetooth_adapter_unittest.cc',
         'bluetooth/bluetooth_adapter_win_unittest.cc',
+        'bluetooth/bluetooth_device_unittest.cc',
         'bluetooth/bluetooth_device_win_unittest.cc',
         'bluetooth/bluetooth_chromeos_unittest.cc',
         'bluetooth/bluetooth_gatt_chromeos_unittest.cc',