Hello all.
I have 2 patches for SFTP subsystem. Please review.
These patches are based on version 0.14. However CVS version still has
the same bugs.
1. libssh2_sftp_read.patch
Function libssh2_sftp_read returns error if it receives
SSH_FXP_STATUS packet. But not all
return codes in this packet are errors, there is also SSH_FX_EOF
code. I think libssh2_sftp_read
should return 0 in such case.
2. libssh2_read_packet_size.patch
libssh2_sftp_packet_read function has a limit of incoming packet
length. It is defined by
LIBSSH2_SFTP_PACKET_MAXLEN macro and currently has value 40000. However
libssh2_sftp_read function requests amount of data equal to passed
buffer size.
Server sends all requested data with one packet and libssh2 gives
error "SFTP packet too large".
This patch fixes libssh2_sftp_read function so it requests data by
pieces it can handle.
P.S. Btw, libssh2_sftp_read function (and many others) ignore error code
returned from libssh2_sftp_packet_requirev and replaces it with too
common ""Timeout waiting for status
message". So user doesn't see real error until he build and install
debug version. I haven't fixed it yet, but it's
very annoying...
--- ../libssh2-0.14.patched1/src/sftp.c 2007-03-29 15:47:00.000000000 +0700
+++ src/sftp.c 2007-03-29 15:55:21.000000000 +0700
@@ -661,66 +661,85 @@
unsigned char *packet, *s, *data;
unsigned char read_responses[2] = { SSH_FXP_DATA, SSH_FXP_STATUS };
size_t bytes_read = 0;
+ size_t bytes_requested = 0;
+ size_t total_read = 0;
#ifdef LIBSSH2_DEBUG_SFTP
_libssh2_debug(session, LIBSSH2_DBG_SFTP, "Reading %lu bytes from SFTP handle", (unsigned long)buffer_maxlen);
#endif
- s = packet = LIBSSH2_ALLOC(session, packet_len);
+ packet = LIBSSH2_ALLOC(session, packet_len);
if (!packet) {
libssh2_error(session, LIBSSH2_ERROR_ALLOC, "Unable to allocate memory for FXP_CLOSE packet", 0);
return -1;
}
- libssh2_htonu32(s, packet_len - 4); s += 4;
- *(s++) = SSH_FXP_READ;
- request_id = sftp->request_id++;
- libssh2_htonu32(s, request_id); s += 4;
- libssh2_htonu32(s, handle->handle_len); s += 4;
- memcpy(s, handle->handle, handle->handle_len); s += handle->handle_len;
- libssh2_htonu64(s, handle->u.file.offset); s += 8;
- libssh2_htonu32(s, buffer_maxlen); s += 4;
+ while (total_read < buffer_maxlen) {
+ s = packet;
- if (packet_len != libssh2_channel_write(channel, packet, packet_len)) {
- libssh2_error(session, LIBSSH2_ERROR_SOCKET_SEND, "Unable to send FXP_READ command", 0);
- LIBSSH2_FREE(session, packet);
- return -1;
- }
- LIBSSH2_FREE(session, packet);
+ /* If buffer_maxlen bytes will be requested, server may return all with one packet.
+ * But libssh2 have packet length limit. So we request data by pieces. */
+ bytes_requested = buffer_maxlen - total_read;
+ if (bytes_requested > LIBSSH2_SFTP_PACKET_MAXLEN-10) /* packet_type(1)+request_id(4)+data_length(4)+end_of_line_flag(1) */
+ bytes_requested = LIBSSH2_SFTP_PACKET_MAXLEN-10;
+
+#ifdef LIBSSH2_DEBUG_SFTP
+ _libssh2_debug(session, LIBSSH2_DBG_SFTP, "Requesting %lu bytes from SFTP handle", (unsigned long)bytes_requested);
+#endif
+
+ libssh2_htonu32(s, packet_len - 4); s += 4;
+ *(s++) = SSH_FXP_READ;
+ request_id = sftp->request_id++;
+ libssh2_htonu32(s, request_id); s += 4;
+ libssh2_htonu32(s, handle->handle_len); s += 4;
+ memcpy(s, handle->handle, handle->handle_len); s += handle->handle_len;
+ libssh2_htonu64(s, handle->u.file.offset); s += 8;
+ libssh2_htonu32(s, bytes_requested); s += 4;
+
+ if (packet_len != libssh2_channel_write(channel, packet, packet_len)) {
+ libssh2_error(session, LIBSSH2_ERROR_SOCKET_SEND, "Unable to send FXP_READ command", 0);
+ LIBSSH2_FREE(session, packet);
+ return -1;
+ }
- if (libssh2_sftp_packet_requirev(sftp, 2, read_responses, request_id, &data, &data_len)) {
- libssh2_error(session, LIBSSH2_ERROR_SOCKET_TIMEOUT, "Timeout waiting for status message", 0);
- return -1;
- }
+ if (libssh2_sftp_packet_requirev(sftp, 2, read_responses, request_id, &data, &data_len)) {
+ libssh2_error(session, LIBSSH2_ERROR_SOCKET_TIMEOUT, "Timeout waiting for status message", 0);
+ LIBSSH2_FREE(session, packet);
+ return -1;
+ }
- switch (data[0]) {
- case SSH_FXP_STATUS: {
- int retcode;
-
- retcode = libssh2_ntohu32(data + 5);
- LIBSSH2_FREE(session, data);
- if (retcode == LIBSSH2_FX_EOF) {
- return 0;
- } else {
- sftp->last_errno = retcode;
- libssh2_error(session, LIBSSH2_ERROR_SFTP_PROTOCOL, "SFTP Protocol Error", 0);
- return -1;
+ switch (data[0]) {
+ case SSH_FXP_STATUS: {
+ int retcode;
+
+ retcode = libssh2_ntohu32(data + 5);
+ LIBSSH2_FREE(session, packet);
+ LIBSSH2_FREE(session, data);
+ if (retcode == LIBSSH2_FX_EOF) {
+ return total_read;
+ } else {
+ sftp->last_errno = retcode;
+ libssh2_error(session, LIBSSH2_ERROR_SFTP_PROTOCOL, "SFTP Protocol Error", 0);
+ return -1;
+ }
}
+ case SSH_FXP_DATA:
+ bytes_read = libssh2_ntohu32(data + 5);
+ if (bytes_read > (data_len - 9)) {
+ return -1;
+ }
+#ifdef LIBSSH2_DEBUG_SFTP
+ _libssh2_debug(session, LIBSSH2_DBG_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);
}
- case SSH_FXP_DATA:
- bytes_read = libssh2_ntohu32(data + 5);
- if (bytes_read > (data_len - 9)) {
- return -1;
- }
-#ifdef LIBSSH2_DEBUG_SFTP
- _libssh2_debug(session, LIBSSH2_DBG_SFTP, "%lu bytes returned", (unsigned long)bytes_read);
-#endif
- memcpy(buffer, data + 9, bytes_read);
- handle->u.file.offset += bytes_read;
- LIBSSH2_FREE(session, data);
- return bytes_read;
}
- return -1;
+ LIBSSH2_FREE(session, packet);
+
+ return total_read;
}
/* }}} */
--- src/sftp.c 2005-06-18 00:15:50.000000000 +0700
+++ src/sftp.c.new 2006-09-03 18:54:22.000000000 +0700
@@ -693,11 +693,19 @@
}
switch (data[0]) {
- case SSH_FXP_STATUS:
- sftp->last_errno = libssh2_ntohu32(data + 5);
- libssh2_error(session, LIBSSH2_ERROR_SFTP_PROTOCOL, "SFTP Protocol Error", 0);
+ case SSH_FXP_STATUS: {
+ int retcode;
+
+ retcode = libssh2_ntohu32(data + 5);
LIBSSH2_FREE(session, data);
- return -1;
+ if (retcode == LIBSSH2_FX_EOF) {
+ return 0;
+ } else {
+ sftp->last_errno = retcode;
+ libssh2_error(session, LIBSSH2_ERROR_SFTP_PROTOCOL, "SFTP Protocol Error", 0);
+ return -1;
+ }
+ }
case SSH_FXP_DATA:
bytes_read = libssh2_ntohu32(data + 5);
if (bytes_read > (data_len - 9)) {
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
libssh2-devel mailing list
libssh2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
