bluetooth: Web Bluetooth can filter by empty device names, doesn't leak MACs.

content/browser/bluetooth/bluetooth_device_chooser_controller.cc

 // Copyright 2016 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 "content/browser/bluetooth/bluetooth_device_chooser_controller.h"
 
 #include <set>
 #include <string>
 #include <unordered_set>
 
@@ skipping 51 matching lines @@
 
 bool IsEmptyOrInvalidFilter(
     const blink::mojom::WebBluetoothScanFilterPtr& filter) {
   // At least one member needs to be present.
   if (filter->name.is_null() && filter->name_prefix.is_null() &&
       filter->services.is_null())
     return true;
 
   // The renderer will never send a name or a name_prefix longer than
   // kMaxLengthForDeviceName.
-  if (!filter->name.is_null() && filter->name.size() > kMaxLengthForDeviceName)
+  if (filter->name.size() > kMaxLengthForDeviceName)

[ortuno, 2016/07/31 18:00:05]

I don't think we should remove the is_null. Once we move to
base::Optional<std::string> this will become filter->name->size() and will
DCHECK when there is no name.

[scheib, 2016/08/02 03:24:33]

Done.

     return true;
-  if (!filter->name_prefix.is_null() &&
-      filter->name_prefix.size() > kMaxLengthForDeviceName)
+  if (!filter->name_prefix.is_null() && filter->name_prefix.size() == 0)
+    return true;
+  if (filter->name_prefix.size() > kMaxLengthForDeviceName)

[ortuno, 2016/07/31 18:00:05]

Same here. I don't think we should remove the check of is_null.

[scheib, 2016/08/02 03:24:33]

Done.

     return true;
 
   return false;
 }
 
 bool HasEmptyOrInvalidFilter(
     const mojo::Array<blink::mojom::WebBluetoothScanFilterPtr>& filters) {
   return filters.empty()
              ? true
              : filters.end() != std::find_if(filters.begin(), filters.end(),
                                              IsEmptyOrInvalidFilter);
 }
 
 bool MatchesFilter(const device::BluetoothDevice& device,
                    const blink::mojom::WebBluetoothScanFilterPtr& filter) {
-  DCHECK(!IsEmptyOrInvalidFilter(filter));
+  CHECK(!IsEmptyOrInvalidFilter(filter));

[ortuno, 2016/07/31 18:00:05]

Not sure if we actually need this CHECK. We already crash the renderer on
invalid filters.

[scheib, 2016/08/02 03:24:33]

Ok, removing the DCHECK here. MatchesFilters calls it, as does GetDevice().

 
-  // TODO(615720): Use the upcoming GetName (was GetDeviceName).
-  const std::string device_name = base::UTF16ToUTF8(device.GetNameForDisplay());
-
-  if (!filter->name.is_null() && (device_name != filter->name)) {
-    return false;
+  if (!filter->name.is_null()) {
+    if (!device.GetName())
+      return false;
+    if (filter->name != device.GetName().value())
+      return false;
   }
 
-  if (!filter->name_prefix.is_null() &&
-      (!base::StartsWith(device_name, filter->name_prefix.get(),
-                         base::CompareCase::SENSITIVE))) {
-    return false;
+  if (filter->name_prefix.size()) {

[ortuno, 2016/07/31 18:00:05]

Same here. I don't think we should change the is_null check.

[scheib, 2016/08/02 03:24:33]

Done.

+    if (!device.GetName())
+      return false;
+    if (!base::StartsWith(device.GetName().value(), filter->name_prefix.get(),
+                          base::CompareCase::SENSITIVE))
+      return false;
   }
 
   if (!filter->services.is_null()) {
     const auto& device_uuid_list = device.GetUUIDs();
     const std::unordered_set<BluetoothUUID, device::BluetoothUUIDHash>
         device_uuids(device_uuid_list.begin(), device_uuid_list.end());
     for (const base::Optional<BluetoothUUID>& service : filter->services) {
       if (!ContainsKey(device_uuids, service.value())) {
         return false;
       }
@@ skipping 365 matching lines @@
 
 void BluetoothDeviceChooserController::PostErrorCallback(
     blink::mojom::WebBluetoothError error) {
   if (!base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(error_callback_, error))) {
     LOG(WARNING) << "No TaskRunner.";
   }
 }
 
 }  // namespace content