Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(263)

Issue 1666003002: bluetooth: Add Web Bluetooth blacklist checks to requestDevice. (Closed)

Created:
4 years, 2 months ago by scheib
Modified:
4 years, 1 month ago
Reviewers:
ortuno, Ilya Sherman
CC:
blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, darin-cc_chromium.org, dglazkov+blink, jam, ortuno+watch_chromium.org, scheib+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@bt-blacklist-
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

bluetooth: Add Web Bluetooth blacklist checks to requestDevice. Disallow access to blacklisted service UUIDs, as per Web Bluetooth specification: https://webbluetoothcg.github.io/web-bluetooth/#the-gatt-blacklist BUG=584398 Committed: https://crrev.com/0f3c66db2769c5773d9701e576e66bc8cf467cda Cr-Commit-Position: refs/heads/master@{#375372}

Patch Set 1 : #

Total comments: 12

Patch Set 2 : address ortuno #

Total comments: 2

Patch Set 3 : optional_services name and check from IsOriginAllowedToAccessService #

Total comments: 1

Patch Set 4 : fix test expectation with updated error string #

Messages

Total messages: 38 (20 generated)
scheib
4 years, 1 month ago (2016-02-12 05:08:39 UTC) #10
ortuno
https://codereview.chromium.org/1666003002/diff/90001/content/browser/bluetooth/bluetooth_dispatcher_host.cc File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right): https://codereview.chromium.org/1666003002/diff/90001/content/browser/bluetooth/bluetooth_dispatcher_host.cc#newcode996 content/browser/bluetooth/bluetooth_dispatcher_host.cc:996: // Use local optional_services_blacklist_filtered in this method. Unsure what ...
4 years, 1 month ago (2016-02-12 16:41:47 UTC) #11
scheib
Thanks, https://codereview.chromium.org/1666003002/diff/90001/content/browser/bluetooth/bluetooth_dispatcher_host.cc File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right): https://codereview.chromium.org/1666003002/diff/90001/content/browser/bluetooth/bluetooth_dispatcher_host.cc#newcode996 content/browser/bluetooth/bluetooth_dispatcher_host.cc:996: // Use local optional_services_blacklist_filtered in this method. On ...
4 years, 1 month ago (2016-02-12 18:25:32 UTC) #12
ortuno
https://codereview.chromium.org/1666003002/diff/110001/content/browser/bluetooth/bluetooth_dispatcher_host.cc File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right): https://codereview.chromium.org/1666003002/diff/110001/content/browser/bluetooth/bluetooth_dispatcher_host.cc#newcode997 content/browser/bluetooth/bluetooth_dispatcher_host.cc:997: optional_services_before_being_filtered_by_blacklist) { I'm not sure how to handle the ...
4 years, 1 month ago (2016-02-12 18:34:59 UTC) #13
scheib
https://codereview.chromium.org/1666003002/diff/110001/content/browser/bluetooth/bluetooth_dispatcher_host.cc File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right): https://codereview.chromium.org/1666003002/diff/110001/content/browser/bluetooth/bluetooth_dispatcher_host.cc#newcode997 content/browser/bluetooth/bluetooth_dispatcher_host.cc:997: optional_services_before_being_filtered_by_blacklist) { On 2016/02/12 18:34:59, ortuno wrote: > I'm ...
4 years, 1 month ago (2016-02-12 21:00:52 UTC) #14
ortuno
lgtm
4 years, 1 month ago (2016-02-12 21:02:54 UTC) #16
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1666003002/150001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1666003002/150001
4 years, 1 month ago (2016-02-12 21:04:18 UTC) #18
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_chromium_gn_compile_dbg/builds/21592)
4 years, 1 month ago (2016-02-12 21:14:41 UTC) #20
scheib
4 years, 1 month ago (2016-02-12 21:25:53 UTC) #22
scheib
isherman, histograms.xml review please.
4 years, 1 month ago (2016-02-12 21:26:16 UTC) #23
Ilya Sherman
histograms.xml lgtm
4 years, 1 month ago (2016-02-12 22:47:19 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1666003002/150001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1666003002/150001
4 years, 1 month ago (2016-02-13 05:08:55 UTC) #26
ortuno
https://codereview.chromium.org/1666003002/diff/150001/third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp File third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp (right): https://codereview.chromium.org/1666003002/diff/150001/third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp#newcode65 third_party/WebKit/Source/modules/bluetooth/BluetoothError.cpp:65: MAP_ERROR(RequestDeviceWithBlacklistedUUID, SecurityError, "requestDevice() called with a filter containing a ...
4 years, 1 month ago (2016-02-13 05:44:53 UTC) #27
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/173719)
4 years, 1 month ago (2016-02-13 06:28:01 UTC) #29
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1666003002/150001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1666003002/150001
4 years, 1 month ago (2016-02-13 22:44:33 UTC) #31
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1666003002/170001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1666003002/170001
4 years, 1 month ago (2016-02-13 22:52:52 UTC) #34
commit-bot: I haz the power
Committed patchset #4 (id:170001)
4 years, 1 month ago (2016-02-14 00:18:53 UTC) #36
commit-bot: I haz the power
4 years, 1 month ago (2016-02-16 22:48:01 UTC) #38
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/0f3c66db2769c5773d9701e576e66bc8cf467cda
Cr-Commit-Position: refs/heads/master@{#375372}

Powered by Google App Engine
This is Rietveld 408576698