sftp: fix partial transfer resuming when copying to local
ClosedPublic

Authored by sitter on Mar 5 2020, 2:58 PM.

Details

Summary

the previous condition checked if the final target path size was >0,
which it would only be when the file already exists (i.e. overwrite) in
all other scenarios it would always be false and as such resuming wouldn't
work. what we actually want to check is whether the part file is >0 (i.e.
there's actual pending bytes to resume from).

this makes resuming work when copying remote->local

CCBUG: 417645

Test Plan
  • create file of suitably large size (1g)
  • split -b somesize the file into two segments
  • copy first segment to /tmp/file.part
  • connect to /tmp over sftp and copy the file there
  • progress starts at 50% and resulting file is same as input file

Diff Detail

Repository
R320 KIO Extras
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Mar 5 2020, 2:58 PM
Restricted Application added projects: Dolphin, Frameworks. · View Herald TranscriptMar 5 2020, 2:58 PM
Restricted Application added subscribers: kfm-devel, kde-frameworks-devel. · View Herald Transcript
sitter requested review of this revision.Mar 5 2020, 2:58 PM
bruns requested changes to this revision.Mar 5 2020, 10:09 PM
bruns added a subscriber: bruns.

Erroneous submit?

This revision now requires changes to proceed.Mar 5 2020, 10:09 PM
bruns requested changes to this revision.Mar 6 2020, 7:05 PM
bruns added inline comments.
sftp/kio_sftp.cpp
1935

Dependent on the file system, this will no longer catch partFile.isDir(). stat.st_size is only defined for regular files and symlinks, for all other types it is implementation defined. E.g. XFS and Btrfs return 0 for empty directories.

1936

should be if (!partFile.isFile()) - we cant resume a pipe or socket ...

This revision now requires changes to proceed.Mar 6 2020, 7:05 PM
sitter added inline comments.Mar 9 2020, 11:10 AM
sftp/kio_sftp.cpp
1935

Your comment is dazzling. What you meant to say is that the size check ought to be after the dir check?

1936

Please make a diff.

This revision was not accepted when it landed; it landed in state Needs Revision.Mar 25 2020, 12:35 PM
This revision was automatically updated to reflect the committed changes.