Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1216583003:
Adding Hashed Address to BluetoothLowEnergyDeviceMac (Closed)
|
Created:
5 years, 5 months ago by krstnmnlsn Modified:
5 years, 5 months ago CC:
chromium-reviews, scheib+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@timeinfo Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUsing a truncated SHA256 hash to deterministically map CBPeripheral identifiers
to addresses for use within Chrome. Note that this is a temporary fix to allow
indexing of mac BLE devices in the |devices_| list, and not intended to provide
any additional security.
BluetoothLowEnergyDeviceMac instances now return a hashed address on
calls to GetAddress(). Also split BluetoothAdapterMac::UpdateDevices() into
RemoveTimedOutDevices() and AddPairedDevices(), so that Classic devices
would not be discovered and added to |devices_| when UpdateDevices() was
called during testing. Removed call to PollAdapter() in
BluetoothAdapterMac::InitForTest() for the same reason.
BUG=506248
Committed: https://crrev.com/332a71cf170683203765842710ea3da4f5a1cf22
Cr-Commit-Position: refs/heads/master@{#339295}
Patch Set 1 : #
Total comments: 9
Patch Set 2 : Scheib's fixes +memory management in the unittests #Patch Set 3 : shortening a signature.. #
Total comments: 10
Patch Set 4 : arman comments #
Total comments: 4
Patch Set 5 : fixing gyp and gn for windows #Patch Set 6 : compiles on OS X 10.6 #
Total comments: 6
Patch Set 7 : rsesek comments #Patch Set 8 : pulled in origin/master #
Total comments: 2
Patch Set 9 : agl comments #
Dependent Patchsets: Messages
Total messages: 74 (45 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
krstnmnlsn@chromium.org changed reviewers: + scheib@chromium.org
This change technically just adds hashing, though a few other changes / refactorings had to happen as well so that I could make the testing work smoothly. https://codereview.chromium.org/1216583003/diff/60001/device/device_tests.gyp File device/device_tests.gyp (right): https://codereview.chromium.org/1216583003/diff/60001/device/device_tests.gyp... device/device_tests.gyp:119: '-ObjC', Is there an equivalent flag needed in the .gn file?
More Mac savvy reviewers should scrutinize, but LGTM with a few comment changes, and GN fix: https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... File device/bluetooth/bluetooth_adapter_mac.mm (right): https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... device/bluetooth/bluetooth_adapter_mac.mm:391: // TODO(krstnmnlsn): Implement method. Have an issue number. The previous comment is probably fine. https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... File device/bluetooth/bluetooth_adapter_mac_unittest.mm (right): https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... device/bluetooth/bluetooth_adapter_mac_unittest.mm:53: // TODO(krstnmnlsn): Remove check once we move to OSX SDK >= 10.9. Shift this comment into explaining that the mock is being adjusted differently based on the OS version and why. I don't think it needs to be phrased as a TODO, more of an explanation. https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... File device/bluetooth/bluetooth_low_energy_device_mac.mm (right): https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... device/bluetooth/bluetooth_low_energy_device_mac.mm:136: // TODO(krstnmnlsn): Remove check once we move to OSX SDK >= 10.9. Rephrase as explaining that isConnected was deprecated in 10.9, and that state is preferred if available. TODO isn't necessary. https://codereview.chromium.org/1216583003/diff/60001/device/device_tests.gyp File device/device_tests.gyp (right): https://codereview.chromium.org/1216583003/diff/60001/device/device_tests.gyp... device/device_tests.gyp:119: '-ObjC', On 2015/07/01 16:45:50, krstnmnlsn wrote: > Is there an equivalent flag needed in the .gn file? Probably. I'm not familiar with the flag, but I see it appear in other .gn files: https://code.google.com/p/chromium/codesearch#search/&sq=package:chromium&typ...
Patchset #2 (id:80001) has been deleted
krstnmnlsn@chromium.org changed reviewers: + armansito@chromium.org
Sending out CL updates 5PM before a long weekend... Vincent: Thanks for the review! Arman: Plz owners review (this CL continues to build on the previous 2) and if you could, check if the way I'm handling retains / scoped_nsobject in the unittests seems reasonable. https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... File device/bluetooth/bluetooth_adapter_mac.mm (right): https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... device/bluetooth/bluetooth_adapter_mac.mm:391: // TODO(krstnmnlsn): Implement method. On 2015/07/01 17:29:44, scheib wrote: > Have an issue number. The previous comment is probably fine. Sure. I removed the comment because as of this change itself there is no longer anything blocking implementation of this method (and so I thought the comment/issue might be confusing). https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... File device/bluetooth/bluetooth_adapter_mac_unittest.mm (right): https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... device/bluetooth/bluetooth_adapter_mac_unittest.mm:53: // TODO(krstnmnlsn): Remove check once we move to OSX SDK >= 10.9. On 2015/07/01 17:29:44, scheib wrote: > Shift this comment into explaining that the mock is being adjusted differently > based on the OS version and why. I don't think it needs to be phrased as a TODO, > more of an explanation. Done. https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... File device/bluetooth/bluetooth_low_energy_device_mac.mm (right): https://codereview.chromium.org/1216583003/diff/60001/device/bluetooth/blueto... device/bluetooth/bluetooth_low_energy_device_mac.mm:136: // TODO(krstnmnlsn): Remove check once we move to OSX SDK >= 10.9. On 2015/07/01 17:29:44, scheib wrote: > Rephrase as explaining that isConnected was deprecated in 10.9, and that state > is preferred if available. TODO isn't necessary. Done. https://codereview.chromium.org/1216583003/diff/60001/device/device_tests.gyp File device/device_tests.gyp (right): https://codereview.chromium.org/1216583003/diff/60001/device/device_tests.gyp... device/device_tests.gyp:119: '-ObjC', On 2015/07/01 17:29:44, scheib wrote: > On 2015/07/01 16:45:50, krstnmnlsn wrote: > > Is there an equivalent flag needed in the .gn file? > > Probably. I'm not familiar with the flag, but I see it appear in other .gn > files: > https://code.google.com/p/chromium/codesearch#search/&sq=package:chromium&typ... Ok. For some reason I missed those.
The CQ bit was checked by krstnmnlsn@chromium.org to run a CQ dry run
The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org Link to the patchset: https://codereview.chromium.org/1216583003/#ps120001 (title: "shortening a signature")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1216583003/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...) mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) win8_chromium_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...) win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...) (exceeded global retry quota)
Generating random BD_ADDRs from NSUUID is a bit weird to me. Why don't we just create a GetIdentifier() method instead and return the opaque identifier directly (which is what I suggested a couple of weeks ago, iirc)? It would be trivial to add such a field to chrome.bluetooth.Device as well. https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.h (right): https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.h:154: const static NSTimeInterval kDiscoveryTimeoutSec; Move this above member variable declarations, per style guide. https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.mm (right): https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.mm:39: 3 * 60; // 3 minutes s/3 * 60/180/ What you have isn't any more readable. https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.mm:363: // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/461181 Is this bug still valid? Are your changes somehow addressing this or does this need to be handled separately. https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac_unittest.mm (right): https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:18: #else #else // !defined(OS_IOS) https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:20: #endif #endif // defined(OS_IOS)
Patchset #4 (id:140001) has been deleted
Thanks Arman! Comments addressed and here is an issue about the identifiers change: http://crbug.com/507824 (also cited by the BluetoothLowEnergyDeviceMac::HashAddress comment) https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.h (right): https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.h:154: const static NSTimeInterval kDiscoveryTimeoutSec; On 2015/07/07 19:07:21, armansito wrote: > Move this above member variable declarations, per style guide. Done. https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.mm (right): https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.mm:39: 3 * 60; // 3 minutes On 2015/07/07 19:07:21, armansito wrote: > s/3 * 60/180/ > > What you have isn't any more readable. Sure. https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.mm:363: // TODO(erikchen): Remove ScopedTracker below once http://crbug.com/461181 On 2015/07/07 19:07:21, armansito wrote: > Is this bug still valid? Are your changes somehow addressing this or does this > need to be handled separately. Yep this is a separate issue. Each method call here references it and is instrumented. So I'm just keeping that consistent as I add a new method call. (chatted with Erik about this) https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac_unittest.mm (right): https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:18: #else On 2015/07/07 19:07:21, armansito wrote: > #else // !defined(OS_IOS) Done. https://codereview.chromium.org/1216583003/diff/120001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:20: #endif On 2015/07/07 19:07:21, armansito wrote: > #endif // defined(OS_IOS) Done. Also fixed in bluetooth_low_energy_device.h
The CQ bit was checked by krstnmnlsn@chromium.org to run a CQ dry run
The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org Link to the patchset: https://codereview.chromium.org/1216583003/#ps160001 (title: "arman comments")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1216583003/160001
lgtm with nits https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.mm (right): https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.mm:38: const NSTimeInterval BluetoothAdapterMac::kDiscoveryTimeoutSec = nit: Add "// static" above this line: // static const NSTimeInterval BluetoothAdapterMac::kFoo https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... File device/bluetooth/bluetooth_low_energy_device_mac.h (right): https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... device/bluetooth/bluetooth_low_energy_device_mac.h:90: // construct an address. The use of fake addresses is a temporary fix before nit: Remove extra space character before the second sentence.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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_...) win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...) (exceeded global retry quota)
The CQ bit was checked by krstnmnlsn@chromium.org to run a CQ dry run
The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org, armansito@chromium.org Link to the patchset: https://codereview.chromium.org/1216583003/#ps180001 (title: "fixing gyp and gn for windows")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1216583003/180001
nits done https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.mm (right): https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.mm:38: const NSTimeInterval BluetoothAdapterMac::kDiscoveryTimeoutSec = On 2015/07/07 22:07:54, armansito wrote: > nit: Add "// static" above this line: > > // static > const NSTimeInterval BluetoothAdapterMac::kFoo Done. https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... File device/bluetooth/bluetooth_low_energy_device_mac.h (right): https://codereview.chromium.org/1216583003/diff/160001/device/bluetooth/bluet... device/bluetooth/bluetooth_low_energy_device_mac.h:90: // construct an address. The use of fake addresses is a temporary fix before On 2015/07/07 22:07:54, armansito wrote: > nit: Remove extra space character before the second sentence. Done.
The CQ bit was unchecked by krstnmnlsn@chromium.org
The CQ bit was checked by krstnmnlsn@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1216583003/180001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by krstnmnlsn@chromium.org to run a CQ dry run
The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org, armansito@chromium.org Link to the patchset: https://codereview.chromium.org/1216583003/#ps200001 (title: "adding incredibly long work around to get state message determined at runtime")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1216583003/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
Patchset #10 (id:280001) has been deleted
Patchset #9 (id:260001) has been deleted
Patchset #8 (id:240001) has been deleted
Patchset #7 (id:220001) has been deleted
Patchset #6 (id:200001) has been deleted
The CQ bit was checked by krstnmnlsn@chromium.org to run a CQ dry run
The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org, armansito@chromium.org Link to the patchset: https://codereview.chromium.org/1216583003/#ps300001 (title: "pulled in runtime checks")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1216583003/300001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...) mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...) (exceeded global retry quota)
Patchset #9 (id:360001) has been deleted
Patchset #11 (id:420001) has been deleted
Patchset #10 (id:400001) has been deleted
Patchset #8 (id:340001) has been deleted
Patchset #7 (id:320001) has been deleted
Patchset #7 (id:380001) has been deleted
Patchset #6 (id:300001) has been deleted
Patchset #7 (id:460001) has been deleted
Patchset #6 (id:440001) has been deleted
Patchset #7 (id:500001) has been deleted
Patchset #6 (id:480001) has been deleted
Patchset #6 (id:520001) has been deleted
krstnmnlsn@chromium.org changed reviewers: + rsesek@chromium.org
@rsesek: I used OCMock in the bluetooth_adapter_mac_unittest.mm file, creating a mock peripheral. Please owners review. Thank you!
https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.h (right): https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.h:136: NSDictionary* advertisementData, naming: in C++ methods, use under_scores. Only in ObjC methods and ivars do you use camelCase. https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac_unittest.mm (right): https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:55: [((CBPeripheral*)[[mock_peripheral stub] C-style casts are banned. https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:70: [[NSDictionary dictionaryWithObjectsAndKeys: Use @{} syntax here?
Thanks! https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac.h (right): https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac.h:136: NSDictionary* advertisementData, On 2015/07/16 16:54:04, Robert Sesek wrote: > naming: in C++ methods, use under_scores. Only in ObjC methods and ivars do you > use camelCase. Done. https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... File device/bluetooth/bluetooth_adapter_mac_unittest.mm (right): https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:55: [((CBPeripheral*)[[mock_peripheral stub] On 2015/07/16 16:54:04, Robert Sesek wrote: > C-style casts are banned. Done. https://codereview.chromium.org/1216583003/diff/540001/device/bluetooth/bluet... device/bluetooth/bluetooth_adapter_mac_unittest.mm:70: [[NSDictionary dictionaryWithObjectsAndKeys: On 2015/07/16 16:54:04, Robert Sesek wrote: > Use @{} syntax here? Sure
LGTM
krstnmnlsn@chromium.org changed reviewers: + agl@chromium.org
Hey Adam, please see the use of SHA256HashString in low_energy_bluetooth_device_mac.mm. Thank you!
The CL description needs to explain the motivation for this change. Is this a privacy concern? If so, then this hash can be reversed without too much effort thus would be a very weak protection and so runs the risk of misleading people. https://codereview.chromium.org/1216583003/diff/580001/device/bluetooth/bluet... File device/bluetooth/bluetooth_low_energy_device_mac.mm (right): https://codereview.chromium.org/1216583003/diff/580001/device/bluetooth/bluet... device/bluetooth/bluetooth_low_energy_device_mac.mm:221: char raw[kCanonicalAddressNumberOfBytes] = {0}; nit: seems odd to clear this array since you're overwriting it in the next line. Your call.
The motivation was originally in the bug cited, but I've added clarification in the description as well now. This change is not intended to provide any additional privacy. https://codereview.chromium.org/1216583003/diff/580001/device/bluetooth/bluet... File device/bluetooth/bluetooth_low_energy_device_mac.mm (right): https://codereview.chromium.org/1216583003/diff/580001/device/bluetooth/bluet... device/bluetooth/bluetooth_low_energy_device_mac.mm:221: char raw[kCanonicalAddressNumberOfBytes] = {0}; On 2015/07/17 17:17:29, agl wrote: > nit: seems odd to clear this array since you're overwriting it in the next line. > Your call. Done.
lgtm
The CQ bit was checked by krstnmnlsn@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org, armansito@chromium.org, rsesek@chromium.org Link to the patchset: https://codereview.chromium.org/1216583003/#ps600001 (title: "agl comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1216583003/600001
Message was sent while issue was closed.
Committed patchset #9 (id:600001)
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/332a71cf170683203765842710ea3da4f5a1cf22 Cr-Commit-Position: refs/heads/master@{#339295} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
