Opened 9 years ago

#7232 new Bug report

sftp implementation presumes a fixed size buffer of 32KB for transfer of data to client

Reported by: ak Owned by:
Priority: normal Component: FileZilla Client
Keywords: download sftp buffersize Cc:
Component version: Operating system type: Windows
Operating system version:

Description

If the server does not return exactly 32KB of data the following error is generated:
error while reading: received a short buffer from FXP_READ, but not at EOF
-
The specific issue is around line 1331 of sftp.c. There is a rather broad comment on 1289 in which the developer was unsure how to handle a short read.
-
The RFC is a bit fuzzy in describing this case (section 8.2.1):
"
However, the server MAY respond with less
data if EOF is reached, an error is encountered, or the servers
internal buffers can not handle such a large request.
"
I believe the key point is "MAY respond with less if the servers internal buffer cannot handle such a large request" is sufficient broad that the client should not assume that fewer than 'length' bytes is indeed EOF or an error.
-
I'm unsure about windows API; but for variants of UNIX (including linux) this closely follows the read API in which the implementation is free to return fewer than N bytes (hence the return values indicate the number of bytes read). While typically synchronous reads will indeed return N bytes they are not required to do so.
-
Anyways we ran into this issue when the server did in fact return fewer bytes and the client (filezilla) abruptly exited.
-
Following the above convention we believe the loop should be written as:

while(toread > 0 ) {

n = read(buf,MIN(bufsize,toread);
if( n indicate error ) error;
toread -= n;

}
rather than
while(toread > 0 ) {

n = read(buf,MIN(bufsize,toread);
if( n != MIN(bufsize,toread) ) error;

}
-
Here is the code from the standard linux sftp.c client:
-
code to actually read the data:
while (total_read < buffer_maxlen) {
bytes_requested = buffer_maxlen - total_read;
*(s++) = SSH_FXP_READ;

request_id = sftp->request_id++;

.
.
retcode = _libssh2_channel_write(channel, 0, (char *) packet,

packet_len)

retcode =

sftp_packet_requirev(sftp, 2,read_responses,

request_id, &data, &data_len);

.
.

case SSH_FXP_DATA:

bytes_read = _libssh2_ntohu32(data + 5);

if (bytes_read > (data_len - 9)) {

sftp->read_packet = NULL;
sftp->read_state = libssh2_NB_state_idle;
return -1;

}

#ifdef LIBSSH2_DEBUG_SFTP

_libssh2_debug(session, LIBSSH2_TRACE_SFTP, "%lu bytes returned",

(unsigned long) bytes_read);

#endif

memcpy(buffer + total_read, data + 9, bytes_read);
handle->u.file.offset += bytes_read;
total_read += bytes_read;
LIBSSH2_FREE(session, data);
/*

  • Set the state back to allocated, so a new one will be
  • created to either request more data or get EOF */

sftp->read_state = libssh2_NB_state_allocated;

Change History (0)

Note: See TracTickets for help on using tickets.