Revision 829de23...

Go back to digest for 6th January 2013

Bug Fixes in KDE Base

Michael Pyne committed changes in [kde-runtime/KDE/4.10] /sftp:

kio_sftp: Disable parallel fetching as it is unsafe.

In code review of libssh while reviewing for bug 312320 I noticed that
attempting to download a file faster by having multiple concurrent
requests can be problematic.

The issue is that these async requests are done in two parts, sharing
the sftp_file struct between them.

When a request is first sent to the server (by sftp_async_read_begin),
the libssh code examines the first parameter (a sftp_file) to determine
what offset to start reading from in the request to the server, while
our code provides the *expected* length. Immediately after the request
is prepared and sent, the libssh code updates the sftp_file's offset to
provisionally include the length of the expected response.

When the response does arrive and is read by application code (in
sftp_async_read()), the libssh code is smart enough to correct the
sftp_file's offset if the actual data length received wasn't the same as
that which was expected. So as long as sftp_async_read_begin is
immediately followed by sftp_async_read there is no issue (and this is
how those functions are used in the libssh example code on their
website at http://api.libssh.org/stable/libssh_tutor_sftp.html)

However, if you start a sequence of sftp_async_read_begin()'s (and we
do, in ::enqueueChunks()), the offset for the second and following of
those functions are based on the *provisional* offset length, which can
be adjusted by the first sftp_async_read(). If this adjustment is made
we will likely corrupt data on download.

So, my quick-and-dirty fix is to disable concurrent fetching by limiting
to one sftp_async_read_begin/sftp_async_read pair at a time for now (so
as to avoid getting too invasive with the code). This probably needs to
be rewritten in the future however, either with a plain synchronous
function (and let the library do it right) or with proper tracking of
<offset,length> tuples for each request to make sure there are no
collisions or gaps in the resultant file.

File Changes

Modified 2 files
  • /sftp
  •   kioslave/kio_sftp.cpp
  •   kioslave/kio_sftp.h
2 files changed in total