Merge changes I4a8aa840,I9708f2a3
* changes: adb: check for an error response from adbd between each write. adbd: restore the old error handling behavior.
This commit is contained in:
commit
44b08c722d
3 changed files with 79 additions and 23 deletions
|
@ -88,7 +88,8 @@ class SyncConnection {
|
|||
: total_bytes_(0),
|
||||
start_time_ms_(CurrentTimeMs()),
|
||||
expected_total_bytes_(0),
|
||||
expect_multiple_files_(false) {
|
||||
expect_multiple_files_(false),
|
||||
expect_done_(false) {
|
||||
max = SYNC_DATA_MAX; // TODO: decide at runtime.
|
||||
|
||||
std::string error;
|
||||
|
@ -117,6 +118,16 @@ class SyncConnection {
|
|||
|
||||
bool IsValid() { return fd >= 0; }
|
||||
|
||||
bool ReceivedError(const char* from, const char* to) {
|
||||
adb_pollfd pfd = {.fd = fd, .events = POLLIN};
|
||||
int rc = adb_poll(&pfd, 1, 0);
|
||||
if (rc < 0) {
|
||||
Error("failed to poll: %s", strerror(errno));
|
||||
return true;
|
||||
}
|
||||
return rc != 0;
|
||||
}
|
||||
|
||||
bool SendRequest(int id, const char* path_and_mode) {
|
||||
size_t path_length = strlen(path_and_mode);
|
||||
if (path_length > 1024) {
|
||||
|
@ -175,6 +186,7 @@ class SyncConnection {
|
|||
p += sizeof(SyncRequest);
|
||||
|
||||
WriteOrDie(lpath, rpath, &buf[0], (p - &buf[0]));
|
||||
expect_done_ = true;
|
||||
total_bytes_ += data_length;
|
||||
return true;
|
||||
}
|
||||
|
@ -220,6 +232,11 @@ class SyncConnection {
|
|||
total_bytes_ += bytes_read;
|
||||
bytes_copied += bytes_read;
|
||||
|
||||
// Check to see if we've received an error from the other side.
|
||||
if (ReceivedError(lpath, rpath)) {
|
||||
break;
|
||||
}
|
||||
|
||||
ReportProgress(rpath, bytes_copied, total_size);
|
||||
}
|
||||
|
||||
|
@ -228,17 +245,24 @@ class SyncConnection {
|
|||
syncmsg msg;
|
||||
msg.data.id = ID_DONE;
|
||||
msg.data.size = mtime;
|
||||
expect_done_ = true;
|
||||
return WriteOrDie(lpath, rpath, &msg.data, sizeof(msg.data));
|
||||
}
|
||||
|
||||
bool CopyDone(const char* from, const char* to) {
|
||||
syncmsg msg;
|
||||
if (!ReadFdExactly(fd, &msg.status, sizeof(msg.status))) {
|
||||
Error("failed to copy '%s' to '%s': no ID_DONE: %s", from, to, strerror(errno));
|
||||
Error("failed to copy '%s' to '%s': couldn't read from device", from, to);
|
||||
return false;
|
||||
}
|
||||
if (msg.status.id == ID_OKAY) {
|
||||
return true;
|
||||
if (expect_done_) {
|
||||
expect_done_ = false;
|
||||
return true;
|
||||
} else {
|
||||
Error("failed to copy '%s' to '%s': received premature success", from, to);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
if (msg.status.id != ID_FAIL) {
|
||||
Error("failed to copy '%s' to '%s': unknown reason %d", from, to, msg.status.id);
|
||||
|
@ -357,6 +381,7 @@ class SyncConnection {
|
|||
|
||||
uint64_t expected_total_bytes_;
|
||||
bool expect_multiple_files_;
|
||||
bool expect_done_;
|
||||
|
||||
LinePrinter line_printer_;
|
||||
|
||||
|
|
|
@ -183,8 +183,6 @@ static bool handle_send_file(int s, const char* path, uid_t uid,
|
|||
}
|
||||
|
||||
while (true) {
|
||||
unsigned int len;
|
||||
|
||||
if (!ReadFdExactly(s, &msg.data, sizeof(msg.data))) goto fail;
|
||||
|
||||
if (msg.data.id != ID_DATA) {
|
||||
|
@ -193,17 +191,17 @@ static bool handle_send_file(int s, const char* path, uid_t uid,
|
|||
break;
|
||||
}
|
||||
SendSyncFail(s, "invalid data message");
|
||||
goto fail;
|
||||
goto abort;
|
||||
}
|
||||
len = msg.data.size;
|
||||
if (len > buffer.size()) { // TODO: resize buffer?
|
||||
|
||||
if (msg.data.size > buffer.size()) { // TODO: resize buffer?
|
||||
SendSyncFail(s, "oversize data message");
|
||||
goto fail;
|
||||
goto abort;
|
||||
}
|
||||
|
||||
if (!ReadFdExactly(s, &buffer[0], len)) goto fail;
|
||||
if (!ReadFdExactly(s, &buffer[0], msg.data.size)) goto abort;
|
||||
|
||||
if (!WriteFdExactly(fd, &buffer[0], len)) {
|
||||
if (!WriteFdExactly(fd, &buffer[0], msg.data.size)) {
|
||||
SendSyncFailErrno(s, "write failed");
|
||||
goto fail;
|
||||
}
|
||||
|
@ -221,6 +219,35 @@ static bool handle_send_file(int s, const char* path, uid_t uid,
|
|||
return WriteFdExactly(s, &msg.status, sizeof(msg.status));
|
||||
|
||||
fail:
|
||||
// If there's a problem on the device, we'll send an ID_FAIL message and
|
||||
// close the socket. Unfortunately the kernel will sometimes throw that
|
||||
// data away if the other end keeps writing without reading (which is
|
||||
// the case with old versions of adb). To maintain compatibility, keep
|
||||
// reading and throwing away ID_DATA packets until the other side notices
|
||||
// that we've reported an error.
|
||||
while (true) {
|
||||
if (!ReadFdExactly(s, &msg.data, sizeof(msg.data))) goto fail;
|
||||
|
||||
if (msg.data.id == ID_DONE) {
|
||||
goto abort;
|
||||
} else if (msg.data.id != ID_DATA) {
|
||||
char id[5];
|
||||
memcpy(id, &msg.data.id, sizeof(msg.data.id));
|
||||
id[4] = '\0';
|
||||
D("handle_send_fail received unexpected id '%s' during failure", id);
|
||||
goto abort;
|
||||
}
|
||||
|
||||
if (msg.data.size > buffer.size()) {
|
||||
D("handle_send_fail received oversized packet of length '%u' during failure",
|
||||
msg.data.size);
|
||||
goto abort;
|
||||
}
|
||||
|
||||
if (!ReadFdExactly(s, &buffer[0], msg.data.size)) goto abort;
|
||||
}
|
||||
|
||||
abort:
|
||||
if (fd >= 0) adb_close(fd);
|
||||
if (do_unlink) adb_unlink(path);
|
||||
return false;
|
||||
|
@ -403,18 +430,6 @@ static bool handle_sync_command(int fd, std::vector<char>& buffer) {
|
|||
void file_sync_service(int fd, void* cookie) {
|
||||
std::vector<char> buffer(SYNC_DATA_MAX);
|
||||
|
||||
// If there's a problem on the device, we'll send an ID_FAIL message and
|
||||
// close the socket. Unfortunately the kernel will sometimes throw that
|
||||
// data away if the other end keeps writing without reading (which is
|
||||
// the normal case with our protocol --- they won't read until the end).
|
||||
// So set SO_LINGER to give the client 20s to get around to reading our
|
||||
// failure response. Without this, the other side's ability to report
|
||||
// useful errors is reduced.
|
||||
struct linger l;
|
||||
l.l_onoff = 1;
|
||||
l.l_linger = 20;
|
||||
adb_setsockopt(fd, SOL_SOCKET, SO_LINGER, &l, sizeof(l));
|
||||
|
||||
while (handle_sync_command(fd, buffer)) {
|
||||
}
|
||||
|
||||
|
|
|
@ -766,6 +766,22 @@ class FileOperationsTest(DeviceTest):
|
|||
if host_dir is not None:
|
||||
shutil.rmtree(host_dir)
|
||||
|
||||
@requires_non_root
|
||||
def test_push_error_reporting(self):
|
||||
"""Make sure that errors that occur while pushing a file get reported
|
||||
|
||||
Bug: http://b/26816782
|
||||
"""
|
||||
with tempfile.NamedTemporaryFile() as tmp_file:
|
||||
tmp_file.write('\0' * 1024 * 1024)
|
||||
tmp_file.flush()
|
||||
try:
|
||||
self.device.push(local=tmp_file.name, remote='/system/')
|
||||
self.fail("push should not have succeeded")
|
||||
except subprocess.CalledProcessError as e:
|
||||
output = e.output
|
||||
|
||||
self.assertIn("Permission denied", output)
|
||||
|
||||
def _test_pull(self, remote_file, checksum):
|
||||
tmp_write = tempfile.NamedTemporaryFile(mode='wb', delete=False)
|
||||
|
|
Loading…
Reference in a new issue