GitLab

"flags & SSH_FILEXFER_ATTR_CREATETIME" check was missing in listDir

CCBUG: 375305

GIT_SILENT

Task: https://phabricator.kde.org/T14518

If you want git blame to ignore formatting revisions run:
git config blame.ignoreRevsFile .git-blame-ignore-revs

Building kio-sftp fails on Ubuntu 18.04:
kio-extras/sftp/kio_sftp.cpp:1280:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type]
}
^
1 error generated.
make[2]: *** [sftp/CMakeFiles/kio_sftp.dir/kio_sftp.cpp.o] Error 1
make[1]: *** [sftp/CMakeFiles/kio_sftp.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [all] Error 2

SFTPInternal::openConnection() version for libssh < 0.8.3 is missing a
return at the end of the function. Add it.

- size_t is not necessarily the same as uint64_t, use just one
  in declaration and definition.

reads that came back with less data are fairly garbage to handle.

the offset is increased when we make requests with read_begin so the
original offset when we enter readchunks is the offset as if all pending
requests had returned.
async_read reduces that offset when a request cames back short.
since we still want the missing data we make a new request for the
missing data both we and libssh modify the offset as part of that
re-requesting.
after that dance we need to restore the most original offset to restore
that initial condition where the offset is the offset as though all
requests had returned. it remains unchanged because we've re-queued the
missing data

the original code lacked this offset restoring which meant that if we
had a short request in the middle of a file it'd break the offset,
return, the caller would queue more chunks and those chunsk would all
have wrong offsets now: increasing from (missingoffset + missingsize) up
to file.size **duplicating** whatever requests may still be pending at
that time.

failure to read without context is kinda useless, so print all errors we
can get our hand on

we only need the maximum length we can read, beyond that it has no use.
how the caller gets ahold of the maximum is entirely not a concern of
GetRequest

previously attr handling was a right mess with at least one leak I think.
attrs must be freed but doing so manually all over the place complicates
things considerably. since all attributes were ultimately scoped to the
functions they were used in anyway it's much cleaner to use a
qscopedpointer with custom deleter instead.

this also enables us to stop GetRequest taking ownership (implicitly)
of the sftp ptrs passed in. and that in turn was a huge blocker for
letting ::read() use GetRequest

Reviewers: vonreth, meven

Reviewed By: meven

Subscribers: sitter, meven, kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D29461

Summary:
sftp gives out more relevant errors such as SSH_FX_PERMISSION_DENIED,
let's forward them as KIO errors instead of using the general cannot open
error.

Test Plan: hoping for the best! bad permissions on the remote now actually raise suitable kio errors, functionally that seems to make little difference unfortunately

Reviewers: feverfew

Reviewed By: feverfew

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D29743

Summary:
servers have arbitrary limits that we should stay below. to ensure this
happens use MAX_XFER_BUF_SIZE as maximum size per request. if we read
data large than that, break it apart into multiple requests

(I am mostly guessing here, the rfc doesn't seem to impose any size
 constraint on the write requests themselves, so probably packet
 constraints apply by default)

BUG: 404890
FIXED-IN: 20.04.2

Test Plan:
- fallocate -l 128M file
- sftp to localhost
- copy file from one dir to another
- compare checksums match

- open xls file (via kio-fuse)
- change
- save
- close
- open again
- changes are still there

Reviewers: ngraham, meven, feverfew

Reviewed By: ngraham, meven, feverfew

Subscribers: meven, feverfew, kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D29634

proftpd doesn't like zero size writes and returns with an error leading us
to assume that the write failed, when in fact it was fine eitherway because
we didn't write anything one way or the other.
so, instead just break out of the loop early. there's nothing lost by
us not doing that final EOF write since we don't need it from a protocol
POV.
the break occurs just before writing because preceding it is "spin-up" tech
opening the file and so forth. we need to have that run even when putting
a 0b file.

this successfully writes 0B, 1B, and 128M files both with partial markers
enabled and disabled to proftpd 1.3.6c

BUG: 419999
FIXED-IN: 20.04.1

for not entirely clear reasons certain servers will fall over after we've
written our data. log internal sftp and ssh errors to aid in debugging

CCBUG: 418472

Reviewers: #frameworks, sitter, meven

Reviewed By: meven

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D28538

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

Reviewers: ngraham, feverfew, bruns

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

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D27872

Summary:
creation time (aka birth time) is file system dependent, if the fs doesn't
support it we seem to get createtime==0. skip over it when that is the
case.

BUG: 375305
FIXED-IN: 20.04

Test Plan: browsing a directory on btrfs over sftp doesn't show creation time as 1970

Reviewers: ngraham, bruns, meven

Reviewed By: ngraham, bruns, meven

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

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D27873

when createUDSEntry fails we'll really want to know what exactly happened
in libssh to help figure out what is wrong if something is wrong.

CCBUG: 406691

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

Summary:
e.g. `sftp://localhost` has an empty path

fall back to assume it means '/'

Test Plan: free space correctly listed on sftp://localhost

Reviewers: ngraham, meven

Reviewed By: meven

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D27835

Summary:
the Result system was originally introduced to the FTP slave and now also
makes an appearance in the SFTP slave. the system introduces a separation
between logic and fronting API class to more tightly control when state
changing calls (finished()/error()) are made. since these calls may only
be made once during a given command multiple calls are at the very least
indicative of bad code and at worst cause severe state confusion for the
slavebase that it won't be able to recover from, rendering the slave
instance broken.

in the internal class Results are returned whenever an error can appear and
these Results must be handled in some form. the only way to effectively
produce user visible errors is to forward results up the call chain.

Test Plan:
- connect
- copy remotely
- overwrite remotely
- copy to local
- overwrite to local
- copy from local to remote
- copy form local to remote and overwrite

Reviewers: dfaure, feverfew

Reviewed By: dfaure, feverfew

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D27153

Summary: This implements the new method added in D26148 in the sftp/smb slaves.

Test Plan:
Tested sftp locally with truncate. Seems to work.
Haven't tested locally with smb though.

Reviewers: sitter, dfaure, fvogt

Reviewed By: sitter, dfaure, fvogt

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

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D26191

Summary:
With D23523 and D24582, we can avoid some IO by not using SlaveBase::config()

Will wait for KF 5.64 release before landing.

Test Plan: Compiles

Reviewers: dfaure

Reviewed By: dfaure

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

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D24686

Summary:
This patch does the following:

1. Fixing instances of finished() being called after error() in smb slave.

2. Changing behaviour of the sftp/smb slave to conform to  what is stated in D23194. In particular, the sftp slave should not call finished() in the open()/read()/write()/seek() methods.

Test Plan: It compiles...

Reviewers: chinmoyr, fvogt, sitter, dfaure

Reviewed By: dfaure

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

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D23207

Summary: Signed-off-by: Andreas Schneider <asn@cryptomilk.org>

Reviewers: sitter, ngraham, broulik

Reviewed By: ngraham, broulik

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D15675

Reviewers: brute4s99

Reviewed By: brute4s99

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D22659

Summary:
The fixed plugin works on Windows. Right now, KDE Connect uses it to establish an initial connection over SFTP, which can then be offloaded to any sftp:// handling application.

Currently, this can be achieved seamlessly by using [WinSCP](https://winscp.net) which is a free and open source SFTP Client.

Test Plan:

0. Install WinSCP : https://winscp.net/eng/download.php.

1. set Craft to use `master` for `kio-extras`

```
craft --set version=master kio-extras
```

2. install `kio-extras` and all deps

```
craft -i kio-extras
```

3. apply this patch

4. re-build `kio-extras`

```
craft --compile --install --qmerge kio-extras
```

5. checkout `milestone2` branch from invent.kde.org/piyushaggarwal/kdeconnect-kde

6. build `kdeconnect-kde` again

```
craft --compile --install --qmerge kdeconnect-kde
```

7. Run `kdeconnect-indicator.exe` from within `CraftRoot/bin/`

8. Right  Click on the dark icon in sys tray, go to your phone and click on **Browse Device**.

9. Press YES, followed by an OK on password prompt (the correct password comes pre-filled).

10. Expect WinSCP to take point from there on.

Reviewers: albertvaka, vonreth, sredman, sitter, dfaure

Reviewed By: vonreth, dfaure

Subscribers: pino, andriusr, kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D22105

Summary:
Fixes pointer was allocated with malloc (by libssh) but freed with
delete. This is not safe and causes undefined behavior.

Test Plan: Build and run kio_sftp with address sanitizer

Reviewers: #frameworks, dfaure, sitter, aacid

Reviewed By: aacid

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

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D19767

Summary:
Fixes memory leak of ssh_session. The mSession member variable is
allocated with a call to ssh_new in sftpOpenConnection but in the
closeConnection a call to ssh_free was missing causing the ssh_session
to be leaked.

Test Plan: Build and run kio_sftp with address sanitizer

Reviewers: #frameworks, sitter, aacid

Reviewed By: aacid

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D19768