bluetooth: Allow name(Prefix) filters longer than 23 characters. |
|||||
Issue descriptionWe are mistakenly rejecting promises if the name filter is longer than 29 bytes. Devices can't advertise a name longer than 29 bytes but it's possible the device name that was acquired through gap.device_name and is longer than 29 bytes.
,
Nov 24 2016
Things to do: - Fix renderer side code [1] - Fix browser side code [2] - Fix layout tests [3] - Add new layout tests to make sure nothing breaks if the name is longer than 29 bytes. [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp?q=Bluetooth.cpp&sq=package:chromium&dr&l=74 [2] https://cs.chromium.org/chromium/src/content/browser/bluetooth/bluetooth_device_chooser_controller.cc?sq=package:chromium&dr=CSs&rcl=1480004359&l=92 [3] https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/bluetooth/requestDevice/?q=f:layouttests+requestdevice&sq=package:chromium&dr
,
Dec 31 2016
Does this involve removing the max length check or increasing the size of kMaxFilterNameLength?
,
Dec 31 2016
Probably not completely unbounded, we should still have some kind of length constraint to avoid malicious use of the API sending unbounded data from the renderer. To determine the length we should look at the bluetooth specification for what name lengths can be. https://www.bluetooth.com/specifications/assigned-numbers/generic-access-profile cites where in the spec this is (vol 3, C, 8.1.2) I tried to take a quick look at Core spec 5.0 https://www.bluetooth.com/specifications/adopted-specifications and it seems the length is limited by the EIR packet size (240 bytes). It would be good to double check me there. I think we can then increase the kMaxFilterNameLength to that value and explain why it is that number and cite the specification source.
,
Jan 4 2017
Thanks, in section 7.7.7 Remote Name Request Complete Event it states that the Remote Name parameter can be 248 bytes: "A UTF-8 encoded user-friendly descriptive name for the remote device. If the name contained in the parameter is shorter than 248 octets, the end of the name is indicated by a NULL octet (0x00), and the following octets (to fill up 248 octets, which is the length of the parameter) do not have valid values." Which is a bit confusing if the EIR packet size is 240 bytes. If the name is >240 bytes, are the last 8 bytes simply omitted? Looking at (vol 3, C, 8) it seems that way: "If applications on a Host provide more than 240 bytes of extended inquiry response data, it is up to the Host to limit it to 240 octets." I'm new to Bluetooth but it seems like 240 bytes is correct.
,
Jan 5 2017
Thanks for spec reading as well. I also think it's OK to round up and allow e.g. 512 bytes. The main intent is to avoid renderers terribly misbehaving - so that's an "order of magnitude" sort of thing.
,
Jan 6 2017
,
Jan 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/97239d883153487960a88a907740755e5459b2a6 commit 97239d883153487960a88a907740755e5459b2a6 Author: jeffcarp <jeffcarp@chromium.org> Date: Wed Jan 18 23:54:14 2017 Bump Bluetooth max filter name length from 29 bytes to 240 bytes A device can't advertise a name longer than 29 bytes, but it can acquire a name via gap.device_name that is longer than 29 bytes, limited by the maximum EIR packet size of 240 bytes. BUG= 653718 Review-Url: https://codereview.chromium.org/2615273002 Cr-Commit-Position: refs/heads/master@{#444551} [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/content/browser/bluetooth/bluetooth_device_chooser_controller.cc [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h [add] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/device-name-longer-than-29-bytes.html [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-namePrefix.html [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-device-name-name.html [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-device-name-namePrefix.html [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/Source/modules/bluetooth/Bluetooth.cpp [modify] https://crrev.com/97239d883153487960a88a907740755e5459b2a6/third_party/WebKit/Source/modules/bluetooth/testing/clusterfuzz/wbt_fakes.py
,
Jan 19 2017
BT5.0 3.C.3.2.2.3 says the device name can be up to 248 bytes (encoded in UTF-8). The Extended Inquiry Response is roughly the Classic version of an Advertising packet, and could hold up to 238 bytes of the name (+ 1 byte for length + 1 byte for EIR Data Type makes 240 bytes). 3.C.8 does say to send a shortened version of the name if it's longer than the EIR packet. The EIR isn't all that relevant for Web Bluetooth for now since we only support BLE. From BT5.0 6.B.2.3.4.9, it looks like the extended advertising payload can contain up to 254 bytes when not fragmented or 1650 bytes when fragmented, which is more than enough to hold the whole 248-byte name. So we should probably limit to 248 bytes, not 240.
,
Jan 19 2017
That sounds good, thanks for catching that detail, I'll submit a follow up CL to update the limit to 248 bytes and change the description. Is 'BT5.0 6.B.2.3.4.9' the correct way to refer to that part of the spec? (i.e. is that how it should be referred to in the comment?)
,
Jan 19 2017
Yeah, "BT 5.0" or "Bluetooth 5.0" or "Bluetooth Core 5.0" are fine. Thanks!
,
Feb 5 2017
As of https://crrev.com/2642123003 the maximum size is 248 bytes. Is this fixed now?
,
Feb 5 2017
Thanks!
,
May 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf commit dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf Author: scheib <scheib@chromium.org> Date: Sat May 13 01:59:13 2017 bluetooth: web: Remove now-duplicate max name length tests. Code used to be stricter about filters and device names and return different error types. We realized that the longest name should always be viable to filter on. Only TypeErrors are produced now, making these tests redundant. 4 test are kept, originally named: "*max-length-for-device-name*" and remove 4 "*max-length-for-name-in-adv-name*" These duplicate tests should have been removed in https://codereview.chromium.org/2642123003 The names are simplified to: "max-length-name*.html" BUG= 653718 Review-Url: https://codereview.chromium.org/2866783002 Cr-Commit-Position: refs/heads/master@{#471540} [delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-name.html [delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-for-name-in-adv-namePrefix.html [rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-name-unicode.html [rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-name.html [rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-namePrefix-unicode.html [rename] https://crrev.com/dd54acf6aaaeac88dd7aa1487c1cca25c9b6ffcf/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/max-length-namePrefix.html [delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-name-in-adv-name.html [delete] https://crrev.com/16d5c759172948ac4c1eeca0d5bcdbc7fb7535c6/third_party/WebKit/LayoutTests/bluetooth/requestDevice/canonicalizeFilter/unicode-max-length-for-name-in-adv-namePrefix.html |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by scheib@chromium.org
, Oct 17 2016