Opened 14 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;