Achim Hügen created SSHD-1123:
---------------------------------

             Summary: ChannelAsyncOutputStream breaks downloads of sftp client 
by not chunking when the remote window is smaller than the packet size
                 Key: SSHD-1123
                 URL: https://issues.apache.org/jira/browse/SSHD-1123
             Project: MINA SSHD
          Issue Type: Bug
    Affects Versions: 2.6.0
            Reporter: Achim Hügen
         Attachments: download.js, package.json

After the upgrade from 2.4.0 to 2.6.0 we experienced issues with a SFTP Client 
used in our test automation (Node.JS ssh2). The download of a file hangs after 
about 1MB of 4MB has been downloaded. 
Although this ssh2 version (0.4.15) is admittedly quite old and meanwhile a fix 
is available, we are reluctant to rollout 2.6.0 to production where a multitude 
of sftp clients is used by customers and might experience the same issue. 

Short story: the client only adjusts its local windows size, if the window size 
is down to 0. But mina in version 2.6 usually will stop sending data before. So 
both parties are waiting for each other.

Example: the window size of the client is 1048576 and packet size is 32768.
After successfully requesting 1015808 of file data, when the clients reads the 
next 32768 bytes, the remote window size as calculated by the MINA is 31706 
bytes. Thus the data to send is larger then the remaining space in the window. 
The old logic in 2.4 just filled the window to the brim by chunking the data. 
Some newly introduced logic (see 
https://github.com/apache/mina-sshd/commit/536d066349f41a6f6a7f95501506cd1ba66dad7a)
 now decides to stop sending more data, because the remote window size is 
smaller then the packet size.
The server waits for the client to send a SSH_MSG_CHANNEL_WINDOW_ADJUST and the 
client doesn't do this, because the window still has some space.

Unfortunately the commit doesn't contain any reference to why this was changed. 
As I understand the SSH specification, the packet size is a maximum packet 
size. So, it should be ok to send a smaller packet. 

This is the code causing the issue in ChannelAsyncOutputStream.
{code:java}
if (total > remoteWindowSize) {
    // if we have a big message and there is enough space, send the next chunk
    if (remoteWindowSize >= packetSize) {
        // send the first chunk as we have enough space in the window
        length = packetSize;
    } else {
        // do not chunk when the window is smaller than the packet size
        length = 0;
        // do a defensive copy in case the user reuses the buffer
        IoWriteFutureImpl f = new IoWriteFutureImpl(future.getId(), new 
ByteArrayBuffer(buffer.getCompactData()));
        f.addListener(w -> future.setValue(w.getException() != null ? 
w.getException() : w.isWritten()));
        pendingWrite.set(f);
        if (log.isTraceEnabled()) {
            log.trace("doWriteIfPossible({})[resume={}] waiting for window 
space {}",
                    this, resume, remoteWindowSize);
        }
    }
 {code}

I have attached code for reproducing the issue. To run you need node.js and npm 
installed: 
{code}
npm install
nodejs download.js
{code}
 

 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to