From 7f29cc5ce2b30e3cc701ff2448a30a29921363df Mon Sep 17 00:00:00 2001
From: Ajay Panicker <apanicke@google.com>
Date: Wed, 22 Aug 2018 16:24:03 -0700
Subject: [PATCH] Don't add items to a response if a previous item fails to be
 added

Bug: 112164711
Test: Run native test net_test_avrcp
Change-Id: Iace287bb716b28eda62a8a0500deb99cc46ae485
---
 profile/avrcp/device.cc                  | 11 ++++++++---
 profile/avrcp/tests/avrcp_device_test.cc | 16 ++++++++++++----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/profile/avrcp/device.cc b/profile/avrcp/device.cc
index 60f8ae5c4..67186cf63 100644
--- a/profile/avrcp/device.cc
+++ b/profile/avrcp/device.cc
@@ -1036,7 +1036,7 @@ void Device::GetVFSListResponse(uint8_t label,
       // right now we always use folders of mixed type
       FolderItem folder_item(vfs_ids_.get_uid(folder.media_id), 0x00,
                              folder.is_playable, folder.name);
-      builder->AddFolder(folder_item);
+      if (!builder->AddFolder(folder_item)) break;
     } else if (items[i].type == ListItem::SONG) {
       auto song = items[i].song;
       auto title =
@@ -1053,7 +1053,9 @@ void Device::GetVFSListResponse(uint8_t label,
             filter_attributes_requested(song, pkt->GetAttributesRequested());
       }
 
-      builder->AddSong(song_item);
+      // If we fail to add a song, don't accidentally add one later that might
+      // fit.
+      if (!builder->AddSong(song_item)) break;
     }
   }
 
@@ -1086,7 +1088,10 @@ void Device::GetNowPlayingListResponse(
       item.attributes_ =
           filter_attributes_requested(song, pkt->GetAttributesRequested());
     }
-    builder->AddSong(item);
+
+    // If we fail to add a song, don't accidentally add one later that might
+    // fit.
+    if (!builder->AddSong(item)) break;
   }
 
   send_message(label, true, std::move(builder));
diff --git a/profile/avrcp/tests/avrcp_device_test.cc b/profile/avrcp/tests/avrcp_device_test.cc
index 56f03e4f4..2c588aea3 100644
--- a/profile/avrcp/tests/avrcp_device_test.cc
+++ b/profile/avrcp/tests/avrcp_device_test.cc
@@ -527,16 +527,24 @@ TEST_F(AvrcpDeviceTest, getFolderItemsMtuTest) {
       base::Bind([](MockFunction<void(uint8_t, bool, const AvrcpResponse&)>* a,
                     uint8_t b, bool c, AvrcpResponse d) { a->Call(b, c, d); },
                  &response_cb);
-  Device device(RawAddress::kAny, true, cb, 0xFFFF, truncated_packet->size());
+
+  Device device(RawAddress::kAny, true, cb, 0xFFFF,
+                truncated_packet->size() + FolderItem::kHeaderSize() + 5);
   device.RegisterInterfaces(&interface, &a2dp_interface, nullptr);
 
   FolderInfo info0 = {"test_id0", true, "Test Folder0"};
   FolderInfo info1 = {"test_id1", true, "Test Folder1"};
-  FolderInfo info2 = {"test_id1", true, "Truncated folder"};
+  FolderInfo info2 = {"test_id2", true, "Truncated folder"};
+  // Used to ensure that adding an item that would fit in the MTU fails if
+  // adding a large item failed.
+  FolderInfo small_info = {"test_id2", true, "Small"};
+
   ListItem item0 = {ListItem::FOLDER, info0, SongInfo()};
   ListItem item1 = {ListItem::FOLDER, info1, SongInfo()};
-  ListItem item2 = {ListItem::FOLDER, info1, SongInfo()};
-  std::vector<ListItem> list0 = {item0, item1, item2};
+  ListItem item2 = {ListItem::FOLDER, info2, SongInfo()};
+  ListItem item3 = {ListItem::FOLDER, small_info, SongInfo()};
+
+  std::vector<ListItem> list0 = {item0, item1, item2, item3};
   EXPECT_CALL(interface, GetFolderItems(_, "", _))
       .WillRepeatedly(InvokeCb<2>(list0));
 
-- 
2.11.0