sftp_packet_add takes ownership of the packet passed to it and (now that we
handle zombies) might free the packet. sftp_packet_read uses the packet type
byte as its return code but by this point sftp_packet_add might have freed
it. This change fixes the problem by caching the packet type before calling
sftp_packet_add.
I don't understand why sftp_packet_read uses the packet type as its return
code. A future change might get rid of this entirely.
As this function is called when the SFTP session is closed, it needs to
also kill all zombies left in the SFTP session to avoid leaking memory
just in case some zombie would still be in there.
When flushing the packetlist, we must only add the request as a zombie
if no response has already been received. Otherwise we could wrongly
make it a zombie even though the response was already received and then
we'd get a zombie stuck there "forever"...
Since the sftp_packetlist_flush() function will move all the existing
FXP_READ requests in this handle to the zombie list we must first remove
this just received packet as it is clearly not a zombie.
Exactly as the comment in the code said, checking the return code from
sftp_packet_read() with <= was wrong and it should be < 0. With the new
filtering on incoming packets that are "zombies" we can now see this
getting zero returned.
In order to be fast, sftp_read sends many read requests at once. With a small
file, this can mean that when EOF is received back, many of these requests are
still outstanding. Responses arriving after we close the file and abandon the
file handle are queued in the SFTP packet queue and never collected. This
causes transfer speed to drop as a progressively longer queue must be searched
for every packet.
This change introduces a zombie request-ID list in the SFTP session that is
used to recognise these outstanding requests and prevent them being added to
the queue.
libcrypto on win32 now depends on gdi32.dll, so move the OpenSSL LDLIBS
block to before the compiler definitions, so that libcrypto gets added
first, and then add -lgdi32 into the following common LDLIBS for gcc.
Examples are built by default. Any of the following options on the
configure command line will skip building them:
--disable-examples-build
--enable-examples-build=no
--enable-examples-build=false
If the filename parameter for file_read_publickey() was the name of a
directory instead of a file then libssh2 would spin trying to fgetc()
from the FILE * for the opened directory when trying to determine the
length of the encoded public key, since fgetc() can't report errors.
Use fread() instead to correctly detect this error condition along
with many others.
This fixes the problem reported in
http://www.libssh2.org/mail/libssh2-devel-archive-2012-04/0021.shtml
Reported-by: Oleksiy Zagorskyi <zalex_ua@i.ua>
When calling _libssh2_channel_receive_window_adjust() internally, we now
always use the 'force' option to prevent libssh2 to avoid sending the
update if the update isn't big enough.
It isn't fully analyzed but we have seen corner cases which made a
necessary window update not get send due to this and then the other side
doesn't send data our side then sits waiting for forever.
if there's not enough room to receive the data that's being requested,
the window adjustment needs to be sent to the remote and thus the force
option has to be used. _libssh2_channel_receive_window_adjust() would
otherwise "queue" small window adjustments for a later packet but that
is really terribly for the small buffer read that for example is the
final little piece of a very large file as then there is no logical next
packet!
Reported by: Armen Babakhanian
Bug: http://www.libssh2.org/mail/libssh2-devel-archive-2012-03/0130.shtml
_libssh2_channel_write() first reads outstanding packets before writing
new data. If it reads a key exchange request, it will immediately start
key re-exchange, which will require sending a response. If the output
socket is full, this will result in a return from
_libssh2_transport_read() of LIBSSH2_ERROR_EAGAIN. In order not to block
a write because there is no data to read, this error is explicitly
ignored and the code continues marshalling a packet for sending. When it
is sent, the remote end immediately drops the connection because it was
expecting a continuation of the key exchange, but got a data packet.
This change adds the same check for key exchange to
_libssh2_transport_send() that is in _libssh2_transport_read(). This
ensures that key exchange is completed before any data packet is sent.
When draining data off the socket with _libssh2_transport_read() (which
in turn has to be done so that we can be sure to have read any possible
window-increasing packets), this code previously ignored errors which
could lead to nasty loops. Now all error codes except EAGAIN will cause
the error to be returned at once.
Bug: http://www.libssh2.org/mail/libssh2-devel-archive-2012-03/0068.shtml
Reported by: Matthew Booth
sizeof(buf) expands to 8 or 4 (since its a pointer). This variable may
have been static in the past, leading to this error.
Signed-off-by: Steven Dake <sdake@redhat.com>
In the x11 example, sizeof(buf) = 8UL (on x86_64), when this should
probably represent the buffer size available. I am not sure how to
test that this change is actually correct, however.
Signed-off-by: Steven Dake <sdake@redhat.com>
Lots of places in the code translated the original error into the more
generic LIBSSH2_ERROR_SOCKET_TIMEOUT but this turns out to distort the
original error reason a lot and makes tracking down the real origin of a
problem really hard. This change makes the original error code be
preserved to a larger extent when return up to the parent function.
Commit 03ca902075 tried to simplify the window sizing logic but broke
SFTP readdir as there was no window sizing code left there so large
directory listings no longer worked.
This change introduces window sizing logic to the sftp_packet_read()
function so that it now tells the remote about the local size having a
window size that suffice when it is about to ask for directory data.
Bug: http://www.libssh2.org/mail/libssh2-devel-archive-2012-03/0069.shtml
Reported by: Eric
The call of libssh2_init returns a return code, but nothing could be done
within the _libssh2_init_if_needed execution path.
Signed-off-by: Steven Dake <sdake@redhat.com>
Although the function checks the length, if the code was in error, there
could potentially be a buffer overrun with the use of sprintf. Instead replace
with snprintf.
Signed-off-by: Steven Dake <sdake@redhat.com>
INVALID_SOCKET is a special value in Windows representing a
non-valid socket identifier. We were #defining this to -1 on
non-Windows platforms, causing unneccessary namespace pollution.
Let's have our own identifier instead.
Thanks to Matt Lawson for pointing this out.
OpenSolaris has no cfmakeraw() so to make the example more portable
we simply do the equivalent operations on struct termios ourselves.
Thanks to Tom Weber for reporting this problem, and finding a solution.
Whenever we have acked data and is about to call a function that *MAY*
return EAGAIN we must return the number now and wait to get called
again. Our API only allows data *or* EAGAIN and we must never try to get
both.
Removed the total_read variable that originally must have tracked how
much data had been written to the buffer. With non-blocking reads, we
must return straight away once we have read data into the buffer so this
variable served not purpose.
I think it was still hanging around in case the initial processing of
'leftover' data meant we wrote to the buffer but this case, like the
others, must return immediately. Now that it does, the last remaining
need for the variable is gone.
Whenever we have data and is about to call a function that *MAY* return
EAGAIN we must return the data now and wait to get called again. Our API
only allows data *or* EAGAIN and we must never try to get both.
