From 5a290f877f5acb29e78533ad45351510cff87497 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowski Date: Wed, 3 Jan 2018 01:49:41 -0800 Subject: [PATCH] Remove bytes after first zero length field in legacy advertisements This is for compatibility with many existing old devices, that have non-zero bytes after zero length field. This is currently causing advertisements to be dropped, rendering those devices invisible. Bug: 68907583 Test: AdvertiseDataParserTest.RemoveTrailingZerosMalformed Change-Id: Ib51950f7e0c6a2771f56c6f69108fa10f2517f38 --- stack/btm/btm_ble_gap.cc | 3 ++- stack/include/advertise_data_parser.h | 7 +------ stack/test/ad_parser_unittest.cc | 30 ++++++++++++++++++++++++++++-- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/stack/btm/btm_ble_gap.cc b/stack/btm/btm_ble_gap.cc index d63187b19..14cf952ec 100644 --- a/stack/btm/btm_ble_gap.cc +++ b/stack/btm/btm_ble_gap.cc @@ -2013,7 +2013,8 @@ static void btm_ble_process_adv_pkt_cont( bool is_start = ble_evt_type_is_legacy(evt_type) && is_scannable && !is_scan_resp; - if (is_start) AdvertiseDataParser::RemoveTrailingZeros(tmp); + if (ble_evt_type_is_legacy(evt_type)) + AdvertiseDataParser::RemoveTrailingZeros(tmp); // We might have send scan request to this device before, but didn't get the // response. In such case make sure data is put at start, not appended to diff --git a/stack/include/advertise_data_parser.h b/stack/include/advertise_data_parser.h index fda88d837..b62dbb395 100644 --- a/stack/include/advertise_data_parser.h +++ b/stack/include/advertise_data_parser.h @@ -57,12 +57,7 @@ class AdvertiseDataParser { // end of the packet. Otherwise i.e. gluing scan response to advertise // data will result in data with zero padding in the middle. if (len == 0) { - size_t zeros_start = position; - for (size_t i = position + 1; i < ad_len; i++) { - if (ad[i] != 0) return; - } - - ad.erase(ad.begin() + zeros_start, ad.end()); + ad.erase(ad.begin() + position, ad.end()); return; } diff --git a/stack/test/ad_parser_unittest.cc b/stack/test/ad_parser_unittest.cc index 6dd59d810..36cc7f266 100644 --- a/stack/test/ad_parser_unittest.cc +++ b/stack/test/ad_parser_unittest.cc @@ -137,15 +137,41 @@ TEST(AdvertiseDataParserTest, RemoveTrailingZeros) { 0x02, 0x01, 0x02, 0x11, 0x06, 0x66, 0x9a, 0x0c, 0x20, 0x00, 0x08, 0x37, 0xa8, 0xe5, 0x11, 0x81, 0x8b, 0xd0, 0xf0, 0xf0, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; - const std::vector podo_scan_resp{ + std::vector podo_scan_resp{ 0x03, 0x19, 0x00, 0x80, 0x09, 0x09, 0x50, 0x6f, 0x64, 0x6f, 0x51, 0x35, 0x56, 0x47, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; AdvertiseDataParser::RemoveTrailingZeros(podo_ad_data); + AdvertiseDataParser::RemoveTrailingZeros(podo_scan_resp); std::vector glued(podo_ad_data); - glued.insert(glued.end(), podo_ad_data.begin(), podo_ad_data.end()); + glued.insert(glued.end(), podo_scan_resp.begin(), podo_scan_resp.end()); + + EXPECT_TRUE(AdvertiseDataParser::IsValid(glued)); +} + +// This test makes sure that RemoveTrailingZeros is removing all bytes after +// first zero length field. It does run the RemoveTrailingZeros for data with +// non-zero bytes after zero length field, then glue scan response at end of it, +// and checks that the resulting data is good. Note: specification requires all +// bytes after zero length field to be zero padding, but many legacy devices got +// this wrong, causing us to have this workaround. +TEST(AdvertiseDataParserTest, RemoveTrailingZerosMalformed) { + std::vector ad_data{0x02, 0x01, 0x02, 0x11, 0x06, 0x66, 0x9a, 0x0c, + 0x20, 0x00, 0x08, 0x37, 0xa8, 0xe5, 0x11, 0x81, + 0x8b, 0xd0, 0xf0, 0xf0, 0xf0, 0x00, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}; + std::vector scan_resp{0x03, 0x19, 0x00, 0x80, 0x09, 0x09, 0x50, 0x6f, + 0x64, 0x6f, 0x51, 0x35, 0x56, 0x47, 0x00, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}; + + AdvertiseDataParser::RemoveTrailingZeros(ad_data); + AdvertiseDataParser::RemoveTrailingZeros(scan_resp); + + std::vector glued(ad_data); + glued.insert(glued.end(), scan_resp.begin(), scan_resp.end()); EXPECT_TRUE(AdvertiseDataParser::IsValid(glued)); } \ No newline at end of file -- 2.11.0