Commit d8cf85ec authored by Harald Sitter's avatar Harald Sitter 🌈
Browse files

sftp: fix seekPos + file resuming when part file is of size 11

Summary:
previously seekPos would loop over offset==EAGAIN. the returned off_t of
seek is not an error, but the offset or -1. this incorrect handling
of the return value resulted in attempting to seek a file of the size 11
to get stuck in an infinite loop as EAGAIN==11 and so the loop would
always be true. any other file size would have been fine, so the impact
of this is actually super small.

also sync up the expectation and handling a bit more between copy and put
scenarios.
specifically we always seek to the size we (think) the part file has,
instead of letting the libc determine the end. this is in part so
we can simply do an offset==size comparison to check if the seek worked
to the end we expected it to.

the seekPos() helper was removed as it now serves no purpose over calling
lseek directly.

BUG: 417645
FIXED-IN: 20.04

Test Plan:
- create file
- `split -b 11` file to get a segment exactly EAGAIN size
- copy first segment to some other dir as file.part
- open sftp to other dir and copy file there
- copy doesn't get stuck, the file.part is removed, and the resulting file is same as input
- vice versa copy from sftp to local

Reviewers: ngraham, feverfew

Reviewed By: ngraham

Subscribers: bruns, kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D27871
parent ba96d3b9
......@@ -139,13 +139,6 @@ static int writeToFile(int fd, const char *buf, int len)
return 0;
}
static KIO::fileoffset_t seekPos(int fd, KIO::fileoffset_t pos, int mode)
{
KIO::fileoffset_t offset = -1;
while ((offset = QT_LSEEK(fd, pos, mode)) == EAGAIN);
return offset;
}
static bool wasUsernameChanged(const QString &username, const KIO::AuthInfo &info)
{
QString loginName (username);
......@@ -1689,9 +1682,9 @@ Result SFTPInternal::sftpPut(const QUrl &url, int permissions, JobFlags flags, i
qCDebug(KIO_SFTP_LOG) << "put got answer " << (flags & KIO::Resume);
} else {
KIO::filesize_t pos = seekPos(fd, sbPart->size, SEEK_SET);
KIO::filesize_t pos = QT_LSEEK(fd, sbPart->size, SEEK_SET);
if (pos != sbPart->size) {
qCDebug(KIO_SFTP_LOG) << "Failed to seek to" << sbPart->size << "bytes in source file. Reason given" << strerror(errno);
qCDebug(KIO_SFTP_LOG) << "Failed to seek to" << sbPart->size << "bytes in source file. Reason given:" << strerror(errno);
sftp_attributes_free(sb);
sftp_attributes_free(sbPart);
return Result::fail(ERR_CANNOT_SEEK, url.toString());
......@@ -1915,7 +1908,7 @@ Result SFTPInternal::sftpCopyGet(const QUrl &url, const QString &sCopyFile, int
// check if destination is ok ...
QFileInfo copyFile(sCopyFile);
const bool bDestExists = copyFile.exists();
if (bDestExists) {
if (bDestExists) {
if (copyFile.isDir()) {
return Result::fail(ERR_IS_DIRECTORY, sCopyFile);
}
......@@ -1959,8 +1952,9 @@ Result SFTPInternal::sftpCopyGet(const QUrl &url, const QString &sCopyFile, int
KIO::fileoffset_t offset = 0;
if (bResume) {
fd = QT_OPEN( QFile::encodeName(sPart), O_RDWR ); // append if resuming
offset = seekPos(fd, 0, SEEK_END);
if(offset < 0) {
offset = QT_LSEEK(fd, partFile.size(), SEEK_SET);
if (offset != partFile.size()) {
qCDebug(KIO_SFTP_LOG) << "Failed to seek to" << partFile.size() << "bytes in target file. Reason given:" << strerror(errno);
::close(fd);
return Result::fail(ERR_CANNOT_RESUME, sCopyFile);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment