Streaming bugreport content to stdout 2/2
- Stream compressed content via the socket from dumpstate
- Remove do_zip_file, do_add_data vars in dumpstate
- getopt '-d' and '-z' options are now no-op
- Rename use_control_socket -> progress_updates_to_socket
- Rename use_socket -> stream_to_socket
do_zip_file and do_add_data are now default behaviors.
Also adding a function interface to hook dumpstate `open_socket`
function that would make it easy to do integration tests of streaming
zipped bugreport via socket. Stub the function and capture data from
target file to verify content.
Bug: 162910469
Test: adb bugreport --stream > test.zip; 7z t test.zip
Test: adb shell bugreportz -s > test.zip; 7z t test.zip
Test: atest dumpstate_test
Test: atest dumpstate_test:ZippedBugReportStreamTest
Test: atest dumpstate_smoke_test
Test: manually test combo keys voldown+volup+power to trigger bugreport
verify logcat kernel buffer shows "Starting service 'bugreport'
from keychord 114 115 116" and device viberated as expected
Change-Id: Icf79bbc42a5616e85625bd604e543fbf973911da
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 616304c..67527b2 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -719,6 +719,9 @@
return timeout_ms > MINIMUM_LOGCAT_TIMEOUT_MS ? timeout_ms : MINIMUM_LOGCAT_TIMEOUT_MS;
}
+// Opens a socket and returns its file descriptor.
+static int open_socket(const char* service);
+
Dumpstate::ConsentCallback::ConsentCallback() : result_(UNAVAILABLE), start_time_(Nanotime()) {
}
@@ -2286,20 +2289,18 @@
static void ShowUsage() {
fprintf(stderr,
- "usage: dumpstate [-h] [-b soundfile] [-e soundfile] [-o directory] [-d] [-p] "
- "[-z] [-s] [-S] [-q] [-P] [-R] [-L] [-V version]\n"
+ "usage: dumpstate [-h] [-b soundfile] [-e soundfile] [-o directory] [-p] "
+ "[-s] [-S] [-q] [-P] [-R] [-L] [-V version]\n"
" -h: display this help message\n"
" -b: play sound file instead of vibrate, at beginning of job\n"
" -e: play sound file instead of vibrate, at end of job\n"
" -o: write to custom directory (only in limited mode)\n"
- " -d: append date to filename\n"
" -p: capture screenshot to filename.png\n"
- " -z: generate zipped file\n"
- " -s: write output to control socket (for init)\n"
- " -S: write file location to control socket (for init; requires -z)\n"
+ " -s: write zipped file to control socket (for init)\n"
+ " -S: write file location to control socket (for init)\n"
" -q: disable vibrate\n"
" -P: send broadcast when started and do progress updates\n"
- " -R: take bugreport in remote mode (requires -z and -d, shouldn't be used with -P)\n"
+ " -R: take bugreport in remote mode (shouldn't be used with -P)\n"
" -w: start binder service and make it wait for a call to startBugreport\n"
" -L: output limited information that is safe for submission in feedback reports\n"
" -v: prints the dumpstate header and exit\n");
@@ -2398,21 +2399,17 @@
/*
* Prepares state like filename, screenshot path, etc in Dumpstate. Also initializes ZipWriter
- * if we are writing zip files and adds the version file.
+ * and adds the version file. Return false if zip_file could not be open to write.
*/
-static void PrepareToWriteToFile() {
+static bool PrepareToWriteToFile() {
MaybeResolveSymlink(&ds.bugreport_internal_dir_);
std::string build_id = android::base::GetProperty("ro.build.id", "UNKNOWN_BUILD");
std::string device_name = android::base::GetProperty("ro.product.name", "UNKNOWN_DEVICE");
ds.base_name_ = StringPrintf("bugreport-%s-%s", device_name.c_str(), build_id.c_str());
- if (ds.options_->do_add_date) {
- char date[80];
- strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&ds.now_));
- ds.name_ = date;
- } else {
- ds.name_ = "undated";
- }
+ char date[80];
+ strftime(date, sizeof(date), "%Y-%m-%d-%H-%M-%S", localtime(&ds.now_));
+ ds.name_ = date;
if (ds.options_->telephony_only) {
ds.base_name_ += "-telephony";
@@ -2439,18 +2436,17 @@
destination.c_str(), ds.base_name_.c_str(), ds.name_.c_str(), ds.log_path_.c_str(),
ds.tmp_path_.c_str(), ds.screenshot_path_.c_str());
- if (ds.options_->do_zip_file) {
- ds.path_ = ds.GetPath(ds.CalledByApi() ? "-zip.tmp" : ".zip");
- MYLOGD("Creating initial .zip file (%s)\n", ds.path_.c_str());
- create_parent_dirs(ds.path_.c_str());
- ds.zip_file.reset(fopen(ds.path_.c_str(), "wb"));
- if (ds.zip_file == nullptr) {
- MYLOGE("fopen(%s, 'wb'): %s\n", ds.path_.c_str(), strerror(errno));
- } else {
- ds.zip_writer_.reset(new ZipWriter(ds.zip_file.get()));
- }
- ds.AddTextZipEntry("version.txt", ds.version_);
+ ds.path_ = ds.GetPath(ds.CalledByApi() ? "-zip.tmp" : ".zip");
+ MYLOGD("Creating initial .zip file (%s)\n", ds.path_.c_str());
+ create_parent_dirs(ds.path_.c_str());
+ ds.zip_file.reset(fopen(ds.path_.c_str(), "wb"));
+ if (ds.zip_file == nullptr) {
+ MYLOGE("fopen(%s, 'wb'): %s\n", ds.path_.c_str(), strerror(errno));
+ return false;
}
+ ds.zip_writer_.reset(new ZipWriter(ds.zip_file.get()));
+ ds.AddTextZipEntry("version.txt", ds.version_);
+ return true;
}
/*
@@ -2458,14 +2454,9 @@
* printing zipped file status, etc.
*/
static void FinalizeFile() {
- bool do_text_file = true;
- if (ds.options_->do_zip_file) {
- if (!ds.FinishZipFile()) {
- MYLOGE("Failed to finish zip file; sending text bugreport instead\n");
- do_text_file = true;
- } else {
- do_text_file = false;
- }
+ bool do_text_file = !ds.FinishZipFile();
+ if (do_text_file) {
+ MYLOGE("Failed to finish zip file; sending text bugreport instead\n");
}
std::string final_path = ds.path_;
@@ -2474,7 +2465,9 @@
android::os::CopyFileToFile(ds.path_, final_path);
}
- if (ds.options_->use_control_socket) {
+ if (ds.options_->stream_to_socket) {
+ android::os::CopyFileToFd(ds.path_, ds.control_socket_fd_);
+ } else if (ds.options_->progress_updates_to_socket) {
if (do_text_file) {
dprintf(ds.control_socket_fd_,
"FAIL:could not create zip file, check %s "
@@ -2530,7 +2523,6 @@
break;
case Dumpstate::BugreportMode::BUGREPORT_WEAR:
options->do_progress_updates = true;
- options->do_zip_file = true;
options->do_screenshot = is_screenshot_requested;
options->dumpstate_hal_mode = DumpstateMode::WEAR;
break;
@@ -2543,7 +2535,6 @@
break;
case Dumpstate::BugreportMode::BUGREPORT_WIFI:
options->wifi_only = true;
- options->do_zip_file = true;
options->do_screenshot = false;
options->dumpstate_hal_mode = DumpstateMode::WIFI;
break;
@@ -2554,11 +2545,11 @@
static void LogDumpOptions(const Dumpstate::DumpOptions& options) {
MYLOGI(
- "do_zip_file: %d do_vibrate: %d use_socket: %d use_control_socket: %d do_screenshot: %d "
+ "do_vibrate: %d stream_to_socket: %d progress_updates_to_socket: %d do_screenshot: %d "
"is_remote_mode: %d show_header_only: %d telephony_only: %d "
"wifi_only: %d do_progress_updates: %d fd: %d bugreport_mode: %s dumpstate_hal_mode: %s "
"limited_only: %d args: %s\n",
- options.do_zip_file, options.do_vibrate, options.use_socket, options.use_control_socket,
+ options.do_vibrate, options.stream_to_socket, options.progress_updates_to_socket,
options.do_screenshot, options.is_remote_mode, options.show_header_only,
options.telephony_only, options.wifi_only,
options.do_progress_updates, options.bugreport_fd.get(), options.bugreport_mode.c_str(),
@@ -2569,11 +2560,6 @@
const android::base::unique_fd& bugreport_fd_in,
const android::base::unique_fd& screenshot_fd_in,
bool is_screenshot_requested) {
- // In the new API world, date is always added; output is always a zip file.
- // TODO(111441001): remove these options once they are obsolete.
- do_add_date = true;
- do_zip_file = true;
-
// Duplicate the fds because the passed in fds don't outlive the binder transaction.
bugreport_fd.reset(dup(bugreport_fd_in.get()));
screenshot_fd.reset(dup(screenshot_fd_in.get()));
@@ -2587,18 +2573,20 @@
while ((c = getopt(argc, argv, "dho:svqzpLPBRSV:w")) != -1) {
switch (c) {
// clang-format off
- case 'd': do_add_date = true; break;
- case 'z': do_zip_file = true; break;
case 'o': out_dir = optarg; break;
- case 's': use_socket = true; break;
- case 'S': use_control_socket = true; break;
+ case 's': stream_to_socket = true; break;
+ case 'S': progress_updates_to_socket = true; break;
case 'v': show_header_only = true; break;
case 'q': do_vibrate = false; break;
case 'p': do_screenshot = true; break;
case 'P': do_progress_updates = true; break;
case 'R': is_remote_mode = true; break;
case 'L': limited_only = true; break;
- case 'V': break; // compatibility no-op
+ case 'V':
+ case 'd':
+ case 'z':
+ // compatibility no-op
+ break;
case 'w':
// This was already processed
break;
@@ -2627,19 +2615,15 @@
}
bool Dumpstate::DumpOptions::ValidateOptions() const {
- if (bugreport_fd.get() != -1 && !do_zip_file) {
+ if (bugreport_fd.get() != -1 && stream_to_socket) {
return false;
}
- if ((do_zip_file || do_add_date || do_progress_updates) && !OutputToFile()) {
+ if ((progress_updates_to_socket || do_progress_updates) && stream_to_socket) {
return false;
}
- if (use_control_socket && !do_zip_file) {
- return false;
- }
-
- if (is_remote_mode && (do_progress_updates || !do_zip_file || !do_add_date)) {
+ if (is_remote_mode && (do_progress_updates || stream_to_socket)) {
return false;
}
return true;
@@ -2714,11 +2698,9 @@
* The temporary bugreport is then populated via printfs, dumping contents of files and
* output of commands to stdout.
*
- * If zipping, the temporary bugreport file is added to the zip archive. Else it's renamed to final
- * text file.
+ * A bunch of other files and dumps are added to the zip archive.
*
- * If zipping, a bunch of other files and dumps also get added to the zip archive. The log file also
- * gets added to the archive.
+ * The temporary bugreport file and the log file also get added to the archive.
*
* Bugreports are first generated in a local directory and later copied to the caller's fd
* or directory if supplied.
@@ -2766,14 +2748,9 @@
MYLOGD("dumpstate calling_uid = %d ; calling package = %s \n",
calling_uid, calling_package.c_str());
- // Redirect output if needed
- bool is_redirecting = options_->OutputToFile();
-
// TODO: temporarily set progress until it's part of the Dumpstate constructor
std::string stats_path =
- is_redirecting
- ? android::base::StringPrintf("%s/dumpstate-stats.txt", bugreport_internal_dir_.c_str())
- : "";
+ android::base::StringPrintf("%s/dumpstate-stats.txt", bugreport_internal_dir_.c_str());
progress_.reset(new Progress(stats_path));
if (acquire_wake_lock(PARTIAL_WAKE_LOCK, WAKE_LOCK_NAME) < 0) {
@@ -2796,35 +2773,33 @@
// If we are going to use a socket, do it as early as possible
// to avoid timeouts from bugreport.
- if (options_->use_socket) {
- if (!redirect_to_socket(stdout, "dumpstate")) {
- return ERROR;
- }
- }
-
- if (options_->use_control_socket) {
+ if (options_->stream_to_socket || options_->progress_updates_to_socket) {
MYLOGD("Opening control socket\n");
- control_socket_fd_ = open_socket("dumpstate");
+ control_socket_fd_ = open_socket_fn_("dumpstate");
if (control_socket_fd_ == -1) {
return ERROR;
}
- options_->do_progress_updates = 1;
+ if (options_->progress_updates_to_socket) {
+ options_->do_progress_updates = 1;
+ }
}
- if (is_redirecting) {
- PrepareToWriteToFile();
+ if (!PrepareToWriteToFile()) {
+ return ERROR;
+ }
- if (options_->do_progress_updates) {
- // clang-format off
- std::vector<std::string> am_args = {
- "--receiver-permission", "android.permission.DUMP",
- };
- // clang-format on
- // Send STARTED broadcast for apps that listen to bugreport generation events
- SendBroadcast("com.android.internal.intent.action.BUGREPORT_STARTED", am_args);
- if (options_->use_control_socket) {
- dprintf(control_socket_fd_, "BEGIN:%s\n", path_.c_str());
- }
+ // Interactive, wear & telephony modes are default to true.
+ // and may enable from cli option or when using control socket
+ if (options_->do_progress_updates) {
+ // clang-format off
+ std::vector<std::string> am_args = {
+ "--receiver-permission", "android.permission.DUMP",
+ };
+ // clang-format on
+ // Send STARTED broadcast for apps that listen to bugreport generation events
+ SendBroadcast("com.android.internal.intent.action.BUGREPORT_STARTED", am_args);
+ if (options_->progress_updates_to_socket) {
+ dprintf(control_socket_fd_, "BEGIN:%s\n", path_.c_str());
}
}
@@ -2839,40 +2814,38 @@
Vibrate(150);
}
- if (options_->do_zip_file && zip_file != nullptr) {
+ if (zip_file != nullptr) {
if (chown(path_.c_str(), AID_SHELL, AID_SHELL)) {
MYLOGE("Unable to change ownership of zip file %s: %s\n", path_.c_str(),
- strerror(errno));
+ strerror(errno));
}
}
int dup_stdout_fd;
int dup_stderr_fd;
- if (is_redirecting) {
- // Redirect stderr to log_path_ for debugging.
- TEMP_FAILURE_RETRY(dup_stderr_fd = dup(fileno(stderr)));
- if (!redirect_to_file(stderr, const_cast<char*>(log_path_.c_str()))) {
- return ERROR;
- }
- if (chown(log_path_.c_str(), AID_SHELL, AID_SHELL)) {
- MYLOGE("Unable to change ownership of dumpstate log file %s: %s\n", log_path_.c_str(),
- strerror(errno));
- }
+ // Redirect stderr to log_path_ for debugging.
+ TEMP_FAILURE_RETRY(dup_stderr_fd = dup(fileno(stderr)));
+ if (!redirect_to_file(stderr, const_cast<char*>(log_path_.c_str()))) {
+ return ERROR;
+ }
+ if (chown(log_path_.c_str(), AID_SHELL, AID_SHELL)) {
+ MYLOGE("Unable to change ownership of dumpstate log file %s: %s\n", log_path_.c_str(),
+ strerror(errno));
+ }
- // Redirect stdout to tmp_path_. This is the main bugreport entry and will be
- // moved into zip file later, if zipping.
- TEMP_FAILURE_RETRY(dup_stdout_fd = dup(fileno(stdout)));
- // TODO: why not write to a file instead of stdout to overcome this problem?
- /* TODO: rather than generating a text file now and zipping it later,
- it would be more efficient to redirect stdout to the zip entry
- directly, but the libziparchive doesn't support that option yet. */
- if (!redirect_to_file(stdout, const_cast<char*>(tmp_path_.c_str()))) {
- return ERROR;
- }
- if (chown(tmp_path_.c_str(), AID_SHELL, AID_SHELL)) {
- MYLOGE("Unable to change ownership of temporary bugreport file %s: %s\n",
- tmp_path_.c_str(), strerror(errno));
- }
+ // Redirect stdout to tmp_path_. This is the main bugreport entry and will be
+ // moved into zip file later, if zipping.
+ TEMP_FAILURE_RETRY(dup_stdout_fd = dup(fileno(stdout)));
+ // TODO: why not write to a file instead of stdout to overcome this problem?
+ /* TODO: rather than generating a text file now and zipping it later,
+ it would be more efficient to redirect stdout to the zip entry
+ directly, but the libziparchive doesn't support that option yet. */
+ if (!redirect_to_file(stdout, const_cast<char*>(tmp_path_.c_str()))) {
+ return ERROR;
+ }
+ if (chown(tmp_path_.c_str(), AID_SHELL, AID_SHELL)) {
+ MYLOGE("Unable to change ownership of temporary bugreport file %s: %s\n",
+ tmp_path_.c_str(), strerror(errno));
}
// Don't buffer stdout
@@ -2926,14 +2899,10 @@
}
/* close output if needed */
- if (is_redirecting) {
- TEMP_FAILURE_RETRY(dup2(dup_stdout_fd, fileno(stdout)));
- }
+ TEMP_FAILURE_RETRY(dup2(dup_stdout_fd, fileno(stdout)));
// Zip the (now complete) .tmp file within the internal directory.
- if (options_->OutputToFile()) {
- FinalizeFile();
- }
+ FinalizeFile();
// Share the final file with the caller if the user has consented or Shell is the caller.
Dumpstate::RunStatus status = Dumpstate::RunStatus::OK;
@@ -2976,11 +2945,9 @@
progress_->Save();
MYLOGI("done (id %d)\n", id_);
- if (is_redirecting) {
- TEMP_FAILURE_RETRY(dup2(dup_stderr_fd, fileno(stderr)));
- }
+ TEMP_FAILURE_RETRY(dup2(dup_stderr_fd, fileno(stderr)));
- if (options_->use_control_socket && control_socket_fd_ != -1) {
+ if (control_socket_fd_ != -1) {
MYLOGD("Closing control socket\n");
close(control_socket_fd_);
}
@@ -3051,10 +3018,7 @@
}
void Dumpstate::EnableParallelRunIfNeeded() {
- // The thread pool needs to create temporary files to receive dump results.
- // That's why we only enable it when the bugreport client chooses to output
- // to a file.
- if (!PropertiesHelper::IsParallelRun() || !options_->OutputToFile()) {
+ if (!PropertiesHelper::IsParallelRun()) {
return;
}
dump_pool_ = std::make_unique<DumpPool>(bugreport_internal_dir_);
@@ -3199,7 +3163,8 @@
options_(new Dumpstate::DumpOptions()),
last_reported_percent_progress_(0),
version_(version),
- now_(time(nullptr)) {
+ now_(time(nullptr)),
+ open_socket_fn_(open_socket) {
}
Dumpstate& Dumpstate::GetInstance() {
@@ -3833,7 +3798,7 @@
RunCommand(title, dumpsys, options, false, out_fd);
}
-int open_socket(const char *service) {
+static int open_socket(const char* service) {
int s = android_get_control_socket(service);
if (s < 0) {
MYLOGE("android_get_control_socket(%s): %s\n", service, strerror(errno));
@@ -3868,19 +3833,6 @@
return fd;
}
-/* redirect output to a service control socket */
-bool redirect_to_socket(FILE* redirect, const char* service) {
- int fd = open_socket(service);
- if (fd == -1) {
- return false;
- }
- fflush(redirect);
- // TODO: handle dup2 failure
- TEMP_FAILURE_RETRY(dup2(fd, fileno(redirect)));
- close(fd);
- return true;
-}
-
// TODO: should call is_valid_output_file and/or be merged into it.
void create_parent_dirs(const char *path) {
char *chp = const_cast<char *> (path);
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 9582c9d..efe06dc 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -54,6 +54,7 @@
class DumpstateTest;
class ProgressTest;
+class ZippedBugReportStreamTest;
} // namespace dumpstate
} // namespace os
@@ -197,6 +198,7 @@
*/
class Dumpstate {
friend class android::os::dumpstate::DumpstateTest;
+ friend class android::os::dumpstate::ZippedBugReportStreamTest;
public:
enum RunStatus { OK, HELP, INVALID_INPUT, ERROR, USER_CONSENT_DENIED, USER_CONSENT_TIMED_OUT };
@@ -388,12 +390,11 @@
* Structure to hold options that determine the behavior of dumpstate.
*/
struct DumpOptions {
- bool do_add_date = false;
- bool do_zip_file = false;
bool do_vibrate = true;
- // Writes bugreport content to a socket; only flatfile format is supported.
- bool use_socket = false;
- bool use_control_socket = false;
+ // Writes bugreport zipped file to a socket.
+ bool stream_to_socket = false;
+ // Writes generation progress updates to a socket.
+ bool progress_updates_to_socket = false;
bool do_screenshot = false;
bool is_screenshot_copied = false;
bool is_remote_mode = false;
@@ -434,13 +435,6 @@
/* Returns true if the options set so far are consistent. */
bool ValidateOptions() const;
- /* Returns if options specified require writing bugreport to a file */
- bool OutputToFile() const {
- // If we are not writing to socket, we will write to a file. If bugreport_fd is
- // specified, it is preferred. If not bugreport is written to /bugreports.
- return !use_socket;
- }
-
/* Returns if options specified require writing to custom file location */
bool OutputToCustomFile() {
// Custom location is only honored in limited mode.
@@ -466,7 +460,8 @@
std::unique_ptr<Progress> progress_;
- // When set, defines a socket file-descriptor use to report progress to bugreportz.
+ // When set, defines a socket file-descriptor use to report progress to bugreportz
+ // or to stream the zipped file to.
int control_socket_fd_ = -1;
// Bugreport format version;
@@ -478,8 +473,8 @@
// `bugreport-BUILD_ID`.
std::string base_name_;
- // Name is the suffix part of the bugreport files - it's typically the date (when invoked with
- // `-d`), but it could be changed by the user..
+ // Name is the suffix part of the bugreport files - it's typically the date,
+ // but it could be changed by the user..
std::string name_;
std::string bugreport_internal_dir_ = DUMPSTATE_DIRECTORY;
@@ -568,6 +563,8 @@
// called by Shell.
RunStatus CopyBugreportIfUserConsented(int32_t calling_uid);
+ std::function<int(const char *)> open_socket_fn_;
+
// Used by GetInstance() only.
explicit Dumpstate(const std::string& version = VERSION_CURRENT);
@@ -601,16 +598,6 @@
int dump_files(const std::string& title, const char* dir, bool (*skip)(const char* path),
int (*dump_from_fd)(const char* title, const char* path, int fd));
-/** opens a socket and returns its file descriptor */
-int open_socket(const char *service);
-
-/*
- * Redirects 'redirect' to a service control socket.
- *
- * Returns true if redirect succeeds.
- */
-bool redirect_to_socket(FILE* redirect, const char* service);
-
/*
* Redirects 'redirect' to a file indicated by 'path', truncating it.
*
diff --git a/cmds/dumpstate/dumpstate.rc b/cmds/dumpstate/dumpstate.rc
index e491a4b..a80da4e 100644
--- a/cmds/dumpstate/dumpstate.rc
+++ b/cmds/dumpstate/dumpstate.rc
@@ -11,7 +11,7 @@
# dumpstatez generates a zipped bugreport but also uses a socket to print the file location once
# it is finished.
-service dumpstatez /system/bin/dumpstate -S -d -z
+service dumpstatez /system/bin/dumpstate -S
socket dumpstate stream 0660 shell log
class main
disabled
diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
index 1c6583e..23c0d95 100644
--- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
@@ -212,9 +212,7 @@
static void GenerateBugreport() {
// clang-format off
char* argv[] = {
- (char*)"dumpstate",
- (char*)"-d",
- (char*)"-z"
+ (char*)"dumpstate"
};
// clang-format on
sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout)), sections));
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index 6b93692..306069f 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -14,8 +14,7 @@
* limitations under the License.
*/
-#define LOG_TAG "dumpstate"
-#include <cutils/log.h>
+#define LOG_TAG "dumpstate_test"
#include "DumpstateInternal.h"
#include "DumpstateService.h"
@@ -24,6 +23,7 @@
#include "DumpPool.h"
#include <gmock/gmock.h>
+#include <gmock/gmock-matchers.h>
#include <gtest/gtest.h>
#include <fcntl.h>
@@ -39,7 +39,9 @@
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <android/hardware/dumpstate/1.1/types.h>
+#include <cutils/log.h>
#include <cutils/properties.h>
+#include <ziparchive/zip_archive.h>
namespace android {
namespace os {
@@ -176,11 +178,9 @@
EXPECT_EQ(status, Dumpstate::RunStatus::OK);
- EXPECT_FALSE(options_.do_add_date);
- EXPECT_FALSE(options_.do_zip_file);
EXPECT_EQ("", options_.out_dir);
- EXPECT_FALSE(options_.use_socket);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_TRUE(options_.do_vibrate);
EXPECT_FALSE(options_.do_screenshot);
@@ -195,17 +195,13 @@
char* argv[] = {
const_cast<char*>("dumpstatez"),
const_cast<char*>("-S"),
- const_cast<char*>("-d"),
- const_cast<char*>("-z"),
};
// clang-format on
Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
EXPECT_EQ(status, Dumpstate::RunStatus::OK);
- EXPECT_TRUE(options_.do_add_date);
- EXPECT_TRUE(options_.do_zip_file);
- EXPECT_TRUE(options_.use_control_socket);
+ EXPECT_TRUE(options_.progress_updates_to_socket);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
@@ -213,7 +209,7 @@
EXPECT_FALSE(options_.do_screenshot);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.limited_only);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
@@ -229,13 +225,11 @@
Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
EXPECT_EQ(status, Dumpstate::RunStatus::OK);
- EXPECT_TRUE(options_.use_socket);
+ EXPECT_TRUE(options_.stream_to_socket);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
- EXPECT_FALSE(options_.do_add_date);
- EXPECT_FALSE(options_.do_zip_file);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.do_screenshot);
EXPECT_FALSE(options_.do_progress_updates);
@@ -246,105 +240,93 @@
TEST_F(DumpOptionsTest, InitializeFullBugReport) {
options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_FULL, fd, fd, true);
- EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_screenshot);
- EXPECT_TRUE(options_.do_zip_file);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::FULL);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.limited_only);
}
TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) {
options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, fd, fd, true);
- EXPECT_TRUE(options_.do_add_date);
- EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.do_progress_updates);
EXPECT_TRUE(options_.do_screenshot);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::INTERACTIVE);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.limited_only);
}
TEST_F(DumpOptionsTest, InitializeRemoteBugReport) {
options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_REMOTE, fd, fd, false);
- EXPECT_TRUE(options_.do_add_date);
- EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.is_remote_mode);
EXPECT_FALSE(options_.do_vibrate);
EXPECT_FALSE(options_.do_screenshot);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::REMOTE);
// Other options retain default values
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.do_progress_updates);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.limited_only);
}
TEST_F(DumpOptionsTest, InitializeWearBugReport) {
options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WEAR, fd, fd, true);
- EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_screenshot);
- EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.do_progress_updates);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::WEAR);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.limited_only);
}
TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) {
options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_TELEPHONY, fd, fd, false);
- EXPECT_TRUE(options_.do_add_date);
EXPECT_FALSE(options_.do_screenshot);
- EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.telephony_only);
EXPECT_TRUE(options_.do_progress_updates);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::CONNECTIVITY);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.limited_only);
}
TEST_F(DumpOptionsTest, InitializeWifiBugReport) {
options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WIFI, fd, fd, false);
- EXPECT_TRUE(options_.do_add_date);
EXPECT_FALSE(options_.do_screenshot);
- EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.wifi_only);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::WIFI);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.limited_only);
}
@@ -353,8 +335,6 @@
char* argv[] = {
const_cast<char*>("dumpstatez"),
const_cast<char*>("-S"),
- const_cast<char*>("-d"),
- const_cast<char*>("-z"),
const_cast<char*>("-q"),
const_cast<char*>("-L"),
const_cast<char*>("-o abc")
@@ -364,9 +344,7 @@
Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
EXPECT_EQ(status, Dumpstate::RunStatus::OK);
- EXPECT_TRUE(options_.do_add_date);
- EXPECT_TRUE(options_.do_zip_file);
- EXPECT_TRUE(options_.use_control_socket);
+ EXPECT_TRUE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.do_vibrate);
EXPECT_TRUE(options_.limited_only);
EXPECT_EQ(" abc", std::string(options_.out_dir));
@@ -376,7 +354,7 @@
EXPECT_FALSE(options_.do_screenshot);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
@@ -393,18 +371,16 @@
Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
EXPECT_EQ(status, Dumpstate::RunStatus::OK);
- EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_screenshot);
- EXPECT_TRUE(options_.do_zip_file);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
// Other options retain default values
EXPECT_TRUE(options_.do_vibrate);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.show_header_only);
EXPECT_FALSE(options_.do_progress_updates);
EXPECT_FALSE(options_.is_remote_mode);
- EXPECT_FALSE(options_.use_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
EXPECT_FALSE(options_.wifi_only);
EXPECT_FALSE(options_.limited_only);
}
@@ -413,8 +389,6 @@
// clang-format off
char* argv[] = {
const_cast<char*>("dumpstate"),
- const_cast<char*>("-d"),
- const_cast<char*>("-z"),
const_cast<char*>("-s"),
const_cast<char*>("-S"),
@@ -424,11 +398,9 @@
Dumpstate::RunStatus status = options_.Initialize(ARRAY_SIZE(argv), argv);
EXPECT_EQ(status, Dumpstate::RunStatus::OK);
- EXPECT_TRUE(options_.do_add_date);
- EXPECT_TRUE(options_.do_zip_file);
// TODO: Maybe we should trim the filename
- EXPECT_TRUE(options_.use_socket);
- EXPECT_TRUE(options_.use_control_socket);
+ EXPECT_TRUE(options_.stream_to_socket);
+ EXPECT_TRUE(options_.progress_updates_to_socket);
// Other options retain default values
EXPECT_FALSE(options_.show_header_only);
@@ -462,10 +434,8 @@
EXPECT_TRUE(options_.is_remote_mode);
// Other options retain default values
- EXPECT_FALSE(options_.do_add_date);
- EXPECT_FALSE(options_.do_zip_file);
- EXPECT_FALSE(options_.use_socket);
- EXPECT_FALSE(options_.use_control_socket);
+ EXPECT_FALSE(options_.stream_to_socket);
+ EXPECT_FALSE(options_.progress_updates_to_socket);
EXPECT_FALSE(options_.limited_only);
EXPECT_EQ(options_.dumpstate_hal_mode, DumpstateMode::DEFAULT);
}
@@ -498,40 +468,31 @@
EXPECT_EQ(status, Dumpstate::RunStatus::INVALID_INPUT);
}
-TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile1) {
- options_.do_zip_file = true;
- // Writing to socket = !writing to file.
- options_.use_socket = true;
+TEST_F(DumpOptionsTest, ValidateOptionsSocketUsage1) {
+ options_.progress_updates_to_socket = true;
+ options_.stream_to_socket = true;
EXPECT_FALSE(options_.ValidateOptions());
- options_.use_socket = false;
+ options_.stream_to_socket = false;
EXPECT_TRUE(options_.ValidateOptions());
}
-TEST_F(DumpOptionsTest, ValidateOptionsNeedOutfile2) {
+TEST_F(DumpOptionsTest, ValidateOptionsSocketUsage2) {
options_.do_progress_updates = true;
// Writing to socket = !writing to file.
- options_.use_socket = true;
+ options_.stream_to_socket = true;
EXPECT_FALSE(options_.ValidateOptions());
- options_.use_socket = false;
- EXPECT_TRUE(options_.ValidateOptions());
-}
-
-TEST_F(DumpOptionsTest, ValidateOptionsNeedZipfile) {
- options_.use_control_socket = true;
- EXPECT_FALSE(options_.ValidateOptions());
-
- options_.do_zip_file = true;
+ options_.stream_to_socket = false;
EXPECT_TRUE(options_.ValidateOptions());
}
TEST_F(DumpOptionsTest, ValidateOptionsRemoteMode) {
+ options_.do_progress_updates = true;
options_.is_remote_mode = true;
EXPECT_FALSE(options_.ValidateOptions());
- options_.do_zip_file = true;
- options_.do_add_date = true;
+ options_.do_progress_updates = false;
EXPECT_TRUE(options_.ValidateOptions());
}
@@ -1018,23 +979,13 @@
ds.listener_.clear();
}
-TEST_F(DumpstateTest, DumpPool_withOutputToFileAndParallelRunEnabled_notNull) {
- ds.options_->use_socket = false;
+TEST_F(DumpstateTest, DumpPool_withParallelRunEnabled_notNull) {
SetParallelRun(true);
EnableParallelRunIfNeeded();
- EXPECT_TRUE(ds.options_->OutputToFile());
EXPECT_TRUE(ds.zip_entry_tasks_);
EXPECT_TRUE(ds.dump_pool_);
}
-TEST_F(DumpstateTest, DumpPool_withNotOutputToFile_isNull) {
- ds.options_->use_socket = true;
- EnableParallelRunIfNeeded();
- EXPECT_FALSE(ds.options_->OutputToFile());
- EXPECT_FALSE(ds.zip_entry_tasks_);
- EXPECT_FALSE(ds.dump_pool_);
-}
-
TEST_F(DumpstateTest, DumpPool_withParallelRunDisabled_isNull) {
SetParallelRun(false);
EnableParallelRunIfNeeded();
@@ -1042,6 +993,69 @@
EXPECT_FALSE(ds.dump_pool_);
}
+class ZippedBugReportStreamTest : public DumpstateBaseTest {
+ public:
+ void SetUp() {
+ DumpstateBaseTest::SetUp();
+ ds_.options_.reset(new Dumpstate::DumpOptions());
+ }
+ void TearDown() {
+ CloseArchive(handle_);
+ }
+
+ // Set bugreport mode and options before here.
+ void GenerateBugreport() {
+ ds_.Initialize();
+ EXPECT_EQ(Dumpstate::RunStatus::OK, ds_.Run(/*calling_uid=*/-1, /*calling_package=*/""));
+ }
+
+ // Most bugreports droproot, ensure the file can be opened by shell to verify file content.
+ void CreateFd(const std::string& path, android::base::unique_fd* out_fd) {
+ out_fd->reset(TEMP_FAILURE_RETRY(open(path.c_str(),
+ O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_NOFOLLOW,
+ S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)));
+ ASSERT_GE(out_fd->get(), 0) << "could not create FD for path " << path;
+ }
+
+ void VerifyEntry(const ZipArchiveHandle archive, const std::string_view entry_name,
+ ZipEntry* data) {
+ int32_t e = FindEntry(archive, entry_name, data);
+ EXPECT_EQ(0, e) << ErrorCodeString(e) << " entry name: " << entry_name;
+ }
+
+ // While testing dumpstate in process, using STDOUT may get confused about
+ // the internal fd redirection. Redirect to a dedicate fd to save content.
+ void RedirectOutputToFd(android::base::unique_fd& ufd) {
+ ds_.open_socket_fn_ = [&](const char*) -> int { return ufd.release(); };
+ };
+
+ Dumpstate& ds_ = Dumpstate::GetInstance();
+ ZipArchiveHandle handle_;
+};
+
+// Generate a quick wifi report redirected to a file, open it and verify entry exist.
+TEST_F(ZippedBugReportStreamTest, StreamWifiReport) {
+ std::string out_path = kTestDataPath + "out.zip";
+ android::base::unique_fd out_fd;
+ CreateFd(out_path, &out_fd);
+ ds_.options_->wifi_only = true;
+ ds_.options_->stream_to_socket = true;
+ RedirectOutputToFd(out_fd);
+
+ GenerateBugreport();
+ OpenArchive(out_path.c_str(), &handle_);
+
+ ZipEntry entry;
+ VerifyEntry(handle_, "main_entry.txt", &entry);
+ std::string bugreport_txt_name;
+ bugreport_txt_name.resize(entry.uncompressed_length);
+ ExtractToMemory(handle_, &entry, reinterpret_cast<uint8_t*>(bugreport_txt_name.data()),
+ entry.uncompressed_length);
+ EXPECT_THAT(bugreport_txt_name,
+ testing::ContainsRegex("(bugreport-.+-wifi(-[[:digit:]]+){6}\\.txt)"));
+ VerifyEntry(handle_, bugreport_txt_name, &entry);
+}
+
class DumpstateServiceTest : public DumpstateBaseTest {
public:
DumpstateService dss;