Ignore bugreportz output when it's not supported.
On devices running M or below, calling 'bugreportz -v' writes
'/system/bin/sh: bugreportz: not found' in the stdout output, which must
be redirected to stderr so it's not shown in the flat-file bugreport,
above the bugreport header.
BUG: 30451114
Change-Id: I942c92fdf6ae85e0cde7b9f94b9eb0b1fecad77a
diff --git a/adb/bugreport.cpp b/adb/bugreport.cpp
index 213fa2e..9ed44a7 100644
--- a/adb/bugreport.cpp
+++ b/adb/bugreport.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define TRACE_TAG ADB
+
#include "bugreport.h"
#include <string>
@@ -187,22 +189,30 @@
if (argc > 2) return usage();
// Gets bugreportz version.
- std::string bugz_stderr;
- DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr);
+ std::string bugz_stdout, bugz_stderr;
+ DefaultStandardStreamsCallback version_callback(&bugz_stdout, &bugz_stderr);
int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback);
std::string bugz_version = android::base::Trim(bugz_stderr);
+ std::string bugz_output = android::base::Trim(bugz_stdout);
if (status != 0 || bugz_version.empty()) {
- // Device does not support bugreportz: if called as 'adb bugreport', just falls out to the
- // flat-file version
- if (argc == 1) return SendShellCommand(transport_type, serial, "bugreport", false);
+ D("'bugreportz' -v results: status=%d, stdout='%s', stderr='%s'", status,
+ bugz_output.c_str(), bugz_version.c_str());
+ if (argc == 1) {
+ // Device does not support bugreportz: if called as 'adb bugreport', just falls out to
+ // the flat-file version.
+ fprintf(stderr,
+ "Failed to get bugreportz version, which is only available on devices "
+ "running Android 7.0 or later.\nTrying a plain-text bug report instead.\n");
+ return SendShellCommand(transport_type, serial, "bugreport", false);
+ }
// But if user explicitly asked for a zipped bug report, fails instead (otherwise calling
- // 'bugreport' would generate a lot of output the user might not be prepared to handle)
+ // 'bugreport' would generate a lot of output the user might not be prepared to handle).
fprintf(stderr,
"Failed to get bugreportz version: 'bugreportz -v' returned '%s' (code %d).\n"
- "If the device runs Android M or below, try 'adb bugreport' instead.\n",
- bugz_stderr.c_str(), status);
+ "If the device does not run Android 7.0 or above, try 'adb bugreport' instead.\n",
+ bugz_output.c_str(), status);
return status != 0 ? status : -1;
}
diff --git a/adb/bugreport_test.cpp b/adb/bugreport_test.cpp
index f9a0a5e..3cd2b6d 100644
--- a/adb/bugreport_test.cpp
+++ b/adb/bugreport_test.cpp
@@ -36,7 +36,9 @@
using ::testing::StrEq;
using ::testing::WithArg;
using ::testing::internal::CaptureStderr;
+using ::testing::internal::CaptureStdout;
using ::testing::internal::GetCapturedStderr;
+using ::testing::internal::GetCapturedStdout;
// Empty function so tests don't need to be linked against file_sync_service.cpp, which requires
// SELinux and its transitive dependencies...
@@ -152,19 +154,28 @@
// Tests when called with invalid number of arguments
TEST_F(BugreportTest, InvalidNumberArgs) {
- const char* args[1024] = {"bugreport", "to", "principal"};
+ const char* args[] = {"bugreport", "to", "principal"};
ASSERT_EQ(-42, br_.DoIt(kTransportLocal, "HannibalLecter", 3, args));
}
// Tests the 'adb bugreport' option when the device does not support 'bugreportz' - it falls back
// to the flat-file format ('bugreport' binary on device)
TEST_F(BugreportTest, NoArgumentsPreNDevice) {
- ExpectBugreportzVersion("");
+ // clang-format off
+ EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -v", false, _))
+ .WillOnce(DoAll(WithArg<4>(WriteOnStderr("")),
+ // Write some bogus output on stdout to make sure it's ignored
+ WithArg<4>(WriteOnStdout("Dude, where is my bugreportz?")),
+ WithArg<4>(ReturnCallbackDone(0))));
+ // clang-format on
+ std::string bugreport = "Reported the bug was.";
+ CaptureStdout();
EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreport", false, _))
- .WillOnce(Return(0));
+ .WillOnce(DoAll(WithArg<4>(WriteOnStdout(bugreport)), Return(0)));
- const char* args[1024] = {"bugreport"};
+ const char* args[] = {"bugreport"};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args));
+ ASSERT_THAT(GetCapturedStdout(), StrEq(bugreport));
}
// Tests the 'adb bugreport' option when the device supports 'bugreportz' version 1.0 - it will
@@ -181,7 +192,7 @@
true, StrEq("generating da_bugreport.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport"};
+ const char* args[] = {"bugreport"};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args));
}
@@ -201,7 +212,7 @@
true, StrEq("generating da_bugreport.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport"};
+ const char* args[] = {"bugreport"};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args));
}
@@ -215,7 +226,7 @@
true, StrEq("generating file.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -231,7 +242,7 @@
true, StrEq("generating file.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -267,7 +278,7 @@
true, StrEq("generating file.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -286,7 +297,7 @@
true, StrEq("generating da_bugreport.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport", td.path};
+ const char* args[] = {"bugreport", td.path};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -300,7 +311,7 @@
true, StrEq("generating file.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport", "file"};
+ const char* args[] = {"bugreport", "file"};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -319,7 +330,7 @@
true, StrEq("generating da_bugreport.zip")))
.WillOnce(Return(true));
- const char* args[1024] = {"bugreport", td.path};
+ const char* args[] = {"bugreport", td.path};
ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -331,7 +342,7 @@
DoAll(WithArg<4>(WriteOnStdout("FAIL:D'OH!\n")), WithArg<4>(ReturnCallbackDone())));
CaptureStderr();
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH!"));
}
@@ -346,7 +357,7 @@
WithArg<4>(ReturnCallbackDone())));
CaptureStderr();
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH!"));
}
@@ -360,7 +371,7 @@
WithArg<4>(ReturnCallbackDone())));
CaptureStderr();
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
ASSERT_THAT(GetCapturedStderr(), HasSubstr("bugreportz? What am I, a zombie?"));
}
@@ -370,7 +381,7 @@
EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -v", false, _))
.WillOnce(Return(666));
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -378,7 +389,7 @@
TEST_F(BugreportTest, BugreportzVersionEmpty) {
ExpectBugreportzVersion("");
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -388,7 +399,7 @@
EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))
.WillOnce(Return(666));
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
@@ -402,6 +413,6 @@
true, HasSubstr("file.zip")))
.WillOnce(Return(false));
- const char* args[1024] = {"bugreport", "file.zip"};
+ const char* args[] = {"bugreport", "file.zip"};
ASSERT_EQ(1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));
}
