am cb591aff: am 2535ea1e: am ca0d66d5: Merge "adb: non-interactive shell stdin."
* commit 'cb591aff17a6559468b5108a3577eecf2c0d7d95':
adb: non-interactive shell stdin.
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index 7581c42..d128df7 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -425,6 +425,7 @@
// Used to pass multiple values to the stdin read thread.
struct StdinReadArgs {
int stdin_fd, write_fd;
+ bool raw_stdin;
std::unique_ptr<ShellProtocol> protocol;
};
@@ -452,26 +453,42 @@
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 @@
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<std::string> 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 @@
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 @@
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 @@
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<std::string> 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 @@
}
}
+ std::string command;
+ if (argc) {
+ // We don't escape here, just like ssh(1). http://b/20564385.
+ command = android::base::Join(
+ std::vector<const char*>(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<const char*>(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 8aeea81..544afce 100644
--- a/adb/shell_service.cpp
+++ b/adb/shell_service.cpp
@@ -382,7 +382,7 @@
subprocess->PassDataStreams();
subprocess->WaitForExit();
- D("deleting Subprocess");
+ D("deleting Subprocess for PID %d", subprocess->pid());
delete subprocess;
return nullptr;
@@ -501,11 +501,31 @@
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::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 8868f10..01410a9 100644
--- a/adb/shell_service.h
+++ b/adb/shell_service.h
@@ -50,11 +50,12 @@
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 20efd84..e18f905 100644
--- a/adb/shell_service_test.cpp
+++ b/adb/shell_service_test.cpp
@@ -245,6 +245,25 @@
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 5918a94..501a75a 100644
--- a/adb/sysdeps.h
+++ b/adb/sysdeps.h
@@ -488,6 +488,10 @@
{
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 42f6d9b..994b851 100644
--- a/adb/sysdeps_win32.cpp
+++ b/adb/sysdeps_win32.cpp
@@ -3265,6 +3265,15 @@
// 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