From db00ff839b2b71c625cd3b0b516143ad9e0c590f Mon Sep 17 00:00:00 2001 From: "isherman@chromium.org" Date: Sat, 17 May 2014 05:49:46 +0000 Subject: [PATCH] [Bluetooth] Standardize Bluetooth device address format to XX:XX:XX:XX: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 --- .../extensions/api/bluetooth/bluetooth_api.cc | 3 +- .../dbus/fake_bluetooth_adapter_client.cc | 4 +- device/bluetooth/bluetooth_adapter.cc | 7 ++- .../bluetooth/bluetooth_adapter_chromeos.cc | 2 +- device/bluetooth/bluetooth_adapter_mac.mm | 2 +- device/bluetooth/bluetooth_adapter_win.cc | 2 +- .../bluetooth_adapter_win_unittest.cc | 2 +- device/bluetooth/bluetooth_device.cc | 36 +++++++++++ device/bluetooth/bluetooth_device.h | 5 ++ device/bluetooth/bluetooth_device_chromeos.cc | 2 +- device/bluetooth/bluetooth_device_mac.h | 6 -- device/bluetooth/bluetooth_device_mac.mm | 11 +--- device/bluetooth/bluetooth_device_unittest.cc | 59 +++++++++++++++++++ device/bluetooth/bluetooth_device_win.cc | 2 +- device/device_tests.gyp | 1 + 15 files changed, 117 insertions(+), 27 deletions(-) create mode 100644 device/bluetooth/bluetooth_device_unittest.cc diff --git a/chrome/browser/extensions/api/bluetooth/bluetooth_api.cc b/chrome/browser/extensions/api/bluetooth/bluetooth_api.cc index 026c5edf5070..772ebd4df082 100644 --- 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 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 index 636e7ad3eee5..7458ef1b3f16 100644 --- 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 index d9a24f05ad8c..1f5922292d0d 100644 --- 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 index 71fdf62549d9..af1d1867c8c6 100644 --- 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 index cf899cbc0783..88ab18a2c777 100644 --- 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 index 5fcbfa88e381..0b85a29ffee4 100644 --- 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 index 9a9a23d7f19c..c3dc9101ec4d 100644 --- 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 index 9cba011fc735..ede53829ce4d 100644 --- a/device/bluetooth/bluetooth_device.cc +++ b/device/bluetooth/bluetooth_device.cc @@ -6,6 +6,7 @@ #include +#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 index a1825948b3c8..14365baa24cd 100644 --- 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 index a3c53ac492e2..e56f907be487 100644 --- 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 index a538239a9f8d..d0668f66234d 100644 --- 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 index 60db6f4425ae..e8af48a0c954 100644 --- 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 new file mode 100644 index 000000000000..7fb505925368 --- /dev/null +++ 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 index 59e11c4c318d..d5e9e771f17c 100644 --- 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 index 8de823ad4f80..9fa2cf534b57 100644 --- 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',