-
Notifications
You must be signed in to change notification settings - Fork 599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ReadAheadRemoteFileInputStream #183
Comments
Hi bkarge, Do you have a sample or testcase to reproduce (preferably) both behaviours? That would help tremendously in fixing it. |
Sir! You use an ordered request fifo with entries (r+0, l1), (r+1, l2), ... If l1 > lk, then you overwrite the end of my buffer. An idea how to fix the problem would be to hide the request fifo behind an InputStream and fill myBuf byte by byte (and skip response packets internally when needed). You don't have to literally use InputStream, it's just to illustrate the technique. |
Below is a demonstration of the behaviour I have submitted a pull request...
|
Fixed by merge of #199 |
…ffer is too big If an instance of ReadAheadRemoteFileInputStream before this change is wrapped into a BufferedInputStream with a big buffer, the SSH client requests big packets from the server. It turned out that if the server had sent a response smaller than requested, the client wouldn't have adjusted to decreased window size, and would have read the file incorrectly. This change detects cases when the server is not able to fulfil client's requests. Since this change, the client adjusts the maximum request length, sends new read-ahead requests, and starts to ignore all read-ahead requests sent earlier. Just specifying some allegedly small constant buffer size wouldn't have helped in all possible cases. There is no way to explicitly get the maximum request length inside a client. All that limits differ from server to server. For instance, OpenSSH defines SFTP_MAX_MSG_LENGTH as 256 * 1024. Apache SSHD defines MAX_READDATA_PACKET_LENGTH as 63 * 1024, and it allows to redefine that size. Interestingly, a similar issue hierynomus#183 was fixed many years ago, but the bug was actually in the code introduced for that fix.
…ffer is too big If an instance of ReadAheadRemoteFileInputStream before this change is wrapped into a BufferedInputStream with a big buffer, the SSH client requests big packets from the server. It turned out that if the server had sent a response smaller than requested, the client wouldn't have adjusted to decreased window size, and would have read the file incorrectly. This change detects cases when the server is not able to fulfil client's requests. Since this change, the client adjusts the maximum request length, sends new read-ahead requests, and starts to ignore all read-ahead requests sent earlier. Just specifying some allegedly small constant buffer size wouldn't have helped in all possible cases. There is no way to explicitly get the maximum request length inside a client. All that limits differ from server to server. For instance, OpenSSH defines SFTP_MAX_MSG_LENGTH as 256 * 1024. Apache SSHD defines MAX_READDATA_PACKET_LENGTH as 63 * 1024, and it allows to redefine that size. Interestingly, a similar issue hierynomus#183 was fixed many years ago, but the bug was actually in the code introduced for that fix.
…ffer is too big (#769) If an instance of ReadAheadRemoteFileInputStream before this change is wrapped into a BufferedInputStream with a big buffer, the SSH client requests big packets from the server. It turned out that if the server had sent a response smaller than requested, the client wouldn't have adjusted to decreased window size, and would have read the file incorrectly. This change detects cases when the server is not able to fulfil client's requests. Since this change, the client adjusts the maximum request length, sends new read-ahead requests, and starts to ignore all read-ahead requests sent earlier. Just specifying some allegedly small constant buffer size wouldn't have helped in all possible cases. There is no way to explicitly get the maximum request length inside a client. All that limits differ from server to server. For instance, OpenSSH defines SFTP_MAX_MSG_LENGTH as 256 * 1024. Apache SSHD defines MAX_READDATA_PACKET_LENGTH as 63 * 1024, and it allows to redefine that size. Interestingly, a similar issue #183 was fixed many years ago, but the bug was actually in the code introduced for that fix.
sshj.sftp.RemoteFile.ReadAheadRemoteFileInputStream.read() leads to data overwrites and/or ArrayIndexOutOfBoundsException when len < res.recvLen.
The problem does not occur with regular read() because there, the size of the response is limited by the len argument.
With read ahead, the size of the response is bounded by a previous len argument.
This bug is not trivial to fix, because it relies on the RemoteFile.checkReadResponse swiss army knife, so there is no easy way to buffer partially evaluated response packets.
The text was updated successfully, but these errors were encountered: