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

diff --git a/stack/btm/btm_ble_gap.cc b/stack/btm/btm_ble_gap.cc
index d63187b..14cf952 100644 (file)
--- 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 fda88d8..b62dbb3 100644 (file)
--- 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 6dd59d8..36cc7f2 100644 (file)
--- 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<uint8_t> podo_scan_resp{
+  std::vector<uint8_t> 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<uint8_t> 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<uint8_t> 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<uint8_t> 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<uint8_t> glued(ad_data);
+  glued.insert(glued.end(), scan_resp.begin(), scan_resp.end());
 
   EXPECT_TRUE(AdvertiseDataParser::IsValid(glued));
 }
\ No newline at end of file