From 1ed57f0dc333c0bc0800e222c569cca8a71deb89 Mon Sep 17 00:00:00 2001 From: David Pursell Date: Tue, 6 Oct 2015 15:30:03 -0700 Subject: [PATCH] adb: non-interactive shell stdin. Non-interactive `adb shell` previously only read from the remote shell, but we want it to write as well so interactive and non-interactive shells can both send data. With this CL, we can now do: $ echo foo | adb shell cat foo This is primarily usable with newer devices that support the shell_v2 features. Older devices will receive stdin but the shell will still hang after all input has been sent, requiring user Ctrl+C. This seems better than closing communication altogether which could potentially miss an unpredictable amount of return data by closing too early. Known issue: non-interactive stdin to a PTY shell isn't reliable. However I don't think this is a common case as ssh doesn't seem to handle it properly either. Examples: * echo 'echo foo' | adb shell * echo 'foo' | adb shell -t cat Bug: http://b/24565284 Change-Id: I5b017fd12d8478765bb6e8400ea76d535c24ce42 --- adb/commandline.cpp | 179 ++++++++++++++++++++----------------- adb/shell_service.cpp | 36 ++++++-- adb/shell_service.h | 7 +- adb/shell_service_test.cpp | 19 ++++ adb/sysdeps.h | 4 + adb/sysdeps_win32.cpp | 9 ++ 6 files changed, 163 insertions(+), 91 deletions(-) diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 0531cf93b..bc5ba38a0 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -425,6 +425,7 @@ namespace { // Used to pass multiple values to the stdin read thread. struct StdinReadArgs { int stdin_fd, write_fd; + bool raw_stdin; std::unique_ptr protocol; }; @@ -452,26 +453,42 @@ static void* stdin_read_thread(void* x) { D("stdin_read_thread(): pre unix_read(fdi=%d,...)", args->stdin_fd); int r = unix_read(args->stdin_fd, buffer_ptr, buffer_size); D("stdin_read_thread(): post unix_read(fdi=%d,...)", args->stdin_fd); - if (r <= 0) break; - for (int n = 0; n < r; n++){ - switch(buffer_ptr[n]) { - case '\n': - state = 1; - break; - case '\r': - state = 1; - break; - case '~': - if(state == 1) state++; - break; - case '.': - if(state == 2) { - fprintf(stderr,"\n* disconnect *\n"); - stdin_raw_restore(args->stdin_fd); - exit(0); + if (r <= 0) { + // Only devices using the shell protocol know to close subprocess + // stdin. For older devices we want to just leave the connection + // open, otherwise an unpredictable amount of return data could + // be lost due to the FD closing before all data has been received. + if (args->protocol) { + args->protocol->Write(ShellProtocol::kIdCloseStdin, 0); + } + break; + } + // If we made stdin raw, check input for the "~." escape sequence. In + // this situation signals like Ctrl+C are sent remotely rather than + // interpreted locally so this provides an emergency out if the remote + // process starts ignoring the signal. SSH also does this, see the + // "escape characters" section on the ssh man page for more info. + if (args->raw_stdin) { + for (int n = 0; n < r; n++){ + switch(buffer_ptr[n]) { + case '\n': + state = 1; + break; + case '\r': + state = 1; + break; + case '~': + if(state == 1) state++; + break; + case '.': + if(state == 2) { + stdin_raw_restore(args->stdin_fd); + fprintf(stderr,"\n* disconnect *\n"); + exit(0); + } + default: + state = 0; } - default: - state = 0; } } if (args->protocol) { @@ -488,8 +505,44 @@ static void* stdin_read_thread(void* x) { return nullptr; } -static int interactive_shell(const std::string& service_string, - bool use_shell_protocol) { +// Returns a shell service string with the indicated arguments and command. +static std::string ShellServiceString(bool use_shell_protocol, + const std::string& type_arg, + const std::string& command) { + std::vector args; + if (use_shell_protocol) { + args.push_back(kShellServiceArgShellProtocol); + } + if (!type_arg.empty()) { + args.push_back(type_arg); + } + + // Shell service string can look like: shell[,arg1,arg2,...]:[command]. + return android::base::StringPrintf("shell%s%s:%s", + args.empty() ? "" : ",", + android::base::Join(args, ',').c_str(), + command.c_str()); +} + +// Connects to a shell on the device and read/writes data. +// +// Note: currently this function doesn't properly clean up resources; the +// FD connected to the adb server is never closed and the stdin read thread +// may never exit. +// +// On success returns the remote exit code if |use_shell_protocol| is true, +// 0 otherwise. On failure returns 1. +static int RemoteShell(bool use_shell_protocol, const std::string& type_arg, + const std::string& command) { + std::string service_string = ShellServiceString(use_shell_protocol, + type_arg, command); + + // Make local stdin raw if the device allocates a PTY, which happens if: + // 1. We are explicitly asking for a PTY shell, or + // 2. We don't specify shell type and are starting an interactive session. + bool raw_stdin = (type_arg == kShellServiceArgPty || + (type_arg.empty() && command.empty())); + std::string error; int fd = adb_connect(service_string, &error); if (fd < 0) { @@ -502,13 +555,16 @@ static int interactive_shell(const std::string& service_string, LOG(ERROR) << "couldn't allocate StdinReadArgs object"; return 1; } - args->stdin_fd = 0; + args->stdin_fd = STDIN_FILENO; args->write_fd = fd; + args->raw_stdin = raw_stdin; if (use_shell_protocol) { args->protocol.reset(new ShellProtocol(args->write_fd)); } - stdin_raw_init(args->stdin_fd); + if (raw_stdin) { + stdin_raw_init(STDIN_FILENO); + } int exit_code = 0; if (!adb_thread_create(stdin_read_thread, args)) { @@ -519,7 +575,12 @@ static int interactive_shell(const std::string& service_string, exit_code = read_and_dump(fd, use_shell_protocol); } - stdin_raw_restore(args->stdin_fd); + if (raw_stdin) { + stdin_raw_restore(STDIN_FILENO); + } + + // TODO(dpursell): properly exit stdin_read_thread and close |fd|. + return exit_code; } @@ -795,25 +856,6 @@ static bool wait_for_device(const char* service, TransportType t, const char* se return adb_command(cmd); } -// Returns a shell service string with the indicated arguments and command. -static std::string ShellServiceString(bool use_shell_protocol, - const std::string& type_arg, - const std::string& command) { - std::vector args; - if (use_shell_protocol) { - args.push_back(kShellServiceArgShellProtocol); - } - if (!type_arg.empty()) { - args.push_back(type_arg); - } - - // Shell service string can look like: shell[,arg1,arg2,...]:[command]. - return android::base::StringPrintf("shell%s%s:%s", - args.empty() ? "" : ",", - android::base::Join(args, ',').c_str(), - command.c_str()); -} - // Connects to the device "shell" service with |command| and prints the // resulting output. static int send_shell_command(TransportType transport_type, const char* serial, @@ -1320,51 +1362,26 @@ int adb_commandline(int argc, const char **argv) { } } + std::string command; + if (argc) { + // We don't escape here, just like ssh(1). http://b/20564385. + command = android::base::Join( + std::vector(argv, argv + argc), ' '); + } + if (h) { printf("\x1b[41;33m"); fflush(stdout); } - if (!argc) { - D("starting interactive shell"); - std::string service_string = - ShellServiceString(use_shell_protocol, shell_type_arg, ""); - r = interactive_shell(service_string, use_shell_protocol); - if (h) { - printf("\x1b[0m"); - fflush(stdout); - } - return r; + r = RemoteShell(use_shell_protocol, shell_type_arg, command); + + if (h) { + printf("\x1b[0m"); + fflush(stdout); } - // We don't escape here, just like ssh(1). http://b/20564385. - std::string command = android::base::Join( - std::vector(argv, argv + argc), ' '); - std::string service_string = - ShellServiceString(use_shell_protocol, shell_type_arg, command); - - while (true) { - D("non-interactive shell loop. cmd=%s", service_string.c_str()); - std::string error; - int fd = adb_connect(service_string, &error); - int r; - if (fd >= 0) { - D("about to read_and_dump(fd=%d)", fd); - r = read_and_dump(fd, use_shell_protocol); - D("read_and_dump() done."); - adb_close(fd); - } else { - fprintf(stderr,"error: %s\n", error.c_str()); - r = -1; - } - - if (h) { - printf("\x1b[0m"); - fflush(stdout); - } - D("non-interactive shell loop. return r=%d", r); - return r; - } + return r; } else if (!strcmp(argv[0], "exec-in") || !strcmp(argv[0], "exec-out")) { int exec_in = !strcmp(argv[0], "exec-in"); diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp index 8aeea810d..544afcebb 100644 --- a/adb/shell_service.cpp +++ b/adb/shell_service.cpp @@ -382,7 +382,7 @@ void* Subprocess::ThreadHandler(void* userdata) { subprocess->PassDataStreams(); subprocess->WaitForExit(); - D("deleting Subprocess"); + D("deleting Subprocess for PID %d", subprocess->pid()); delete subprocess; return nullptr; @@ -501,11 +501,31 @@ ScopedFd* Subprocess::PassInput() { return &protocol_sfd_; } - // We only care about stdin packets. - if (stdinout_sfd_.valid() && input_->id() == ShellProtocol::kIdStdin) { - input_bytes_left_ = input_->data_length(); - } else { - input_bytes_left_ = 0; + if (stdinout_sfd_.valid()) { + switch (input_->id()) { + case ShellProtocol::kIdStdin: + input_bytes_left_ = input_->data_length(); + break; + case ShellProtocol::kIdCloseStdin: + if (type_ == SubprocessType::kRaw) { + if (adb_shutdown(stdinout_sfd_.fd(), SHUT_WR) == 0) { + return nullptr; + } + PLOG(ERROR) << "failed to shutdown writes to FD " + << stdinout_sfd_.fd(); + return &stdinout_sfd_; + } else { + // PTYs can't close just input, so rather than close the + // FD and risk losing subprocess output, leave it open. + // This only happens if the client starts a PTY shell + // non-interactively which is rare and unsupported. + // If necessary, the client can manually close the shell + // with `exit` or by killing the adb client process. + D("can't close input for PTY FD %d", + stdinout_sfd_.fd()); + } + break; + } } } @@ -532,7 +552,9 @@ ScopedFd* Subprocess::PassInput() { ScopedFd* Subprocess::PassOutput(ScopedFd* sfd, ShellProtocol::Id id) { int bytes = adb_read(sfd->fd(), output_->data(), output_->data_capacity()); if (bytes == 0 || (bytes < 0 && errno != EAGAIN)) { - if (bytes < 0) { + // read() returns EIO if a PTY closes; don't report this as an error, + // it just means the subprocess completed. + if (bytes < 0 && !(type_ == SubprocessType::kPty && errno == EIO)) { PLOG(ERROR) << "error reading output FD " << sfd->fd(); } return sfd; diff --git a/adb/shell_service.h b/adb/shell_service.h index 8868f1074..01410a9ba 100644 --- a/adb/shell_service.h +++ b/adb/shell_service.h @@ -50,11 +50,12 @@ class ShellProtocol { public: // This is an unscoped enum to make it easier to compare against raw bytes. enum Id : uint8_t { - kIdStdin = 0, + kIdStdin = 0, kIdStdout = 1, kIdStderr = 2, - kIdExit = 3, - kIdInvalid = 255, // Indicates an invalid or unknown packet. + kIdExit = 3, + kIdCloseStdin = 4, // Close subprocess stdin if possible. + kIdInvalid = 255, // Indicates an invalid or unknown packet. }; // ShellPackets will probably be too large to allocate on the stack so they diff --git a/adb/shell_service_test.cpp b/adb/shell_service_test.cpp index 20efd84e0..e18f905e6 100644 --- a/adb/shell_service_test.cpp +++ b/adb/shell_service_test.cpp @@ -245,6 +245,25 @@ TEST_F(ShellServiceTest, InteractivePtySubprocess) { EXPECT_FALSE(stdout.find("--abc123--") == std::string::npos); } +// Tests closing raw subprocess stdin. +TEST_F(ShellServiceTest, CloseClientStdin) { + ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( + "cat; echo TEST_DONE", + SubprocessType::kRaw, SubprocessProtocol::kShell)); + + std::string input = "foo\nbar"; + ShellProtocol* protocol = new ShellProtocol(subprocess_fd_); + memcpy(protocol->data(), input.data(), input.length()); + ASSERT_TRUE(protocol->Write(ShellProtocol::kIdStdin, input.length())); + ASSERT_TRUE(protocol->Write(ShellProtocol::kIdCloseStdin, 0)); + delete protocol; + + std::string stdout, stderr; + EXPECT_EQ(0, ReadShellProtocol(subprocess_fd_, &stdout, &stderr)); + ExpectLinesEqual(stdout, {"foo", "barTEST_DONE"}); + ExpectLinesEqual(stderr, {}); +} + // Tests that nothing breaks when the stdin/stdout pipe closes. TEST_F(ShellServiceTest, CloseStdinStdoutSubprocess) { ASSERT_NO_FATAL_FAILURE(StartTestSubprocess( diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 5918a94f0..501a75a7b 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -488,6 +488,10 @@ static __inline__ int adb_shutdown(int fd) { return shutdown(fd, SHUT_RDWR); } +static __inline__ int adb_shutdown(int fd, int direction) +{ + return shutdown(fd, direction); +} #undef shutdown #define shutdown ____xxx_shutdown diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index 42f6d9b76..994b85104 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -3265,6 +3265,15 @@ int unix_read(int fd, void* buf, size_t len) { // terminal. return _console_read(_console_handle, buf, len); } else { + // On older versions of Windows (definitely 7, definitely not 10), + // ReadConsole() with a size >= 31367 fails, so if |fd| is a console + // we need to limit the read size. This may also catch devices like NUL, + // but that is OK as we just want to avoid capping pipes and files which + // don't need size limiting. This isatty() test is very simple and quick + // and doesn't call the OS. + if (isatty(fd) && len > 4096) { + len = 4096; + } // Just call into C Runtime which can read from pipes/files and which // can do LF/CR translation (which is overridable with _setmode()). // Undefine the macro that is set in sysdeps.h which bans calls to