Skip to content
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

Fix length field for SSH_FXP_WRITE packets. #181

Merged
merged 1 commit into from
Apr 1, 2015
Merged

Fix length field for SSH_FXP_WRITE packets. #181

merged 1 commit into from
Apr 1, 2015

Conversation

dkocher
Copy link
Contributor

@dkocher dkocher commented Feb 24, 2015

Proposed fix for #180.

@hierynomus
Copy link
Owner

Wondering whether the code @shikhar wrote is actually correct here. According to the spec there is no length field, i.e.

  Writing to a file is achieved using the SSH_FXP_WRITE message, which
   has the following format:

    uint32     id
    string     handle
    uint64     offset
    string     data

   where `id' is a request identifier, `handle' is a file handle

(source: http://www.openssh.com/txt/draft-ietf-secsh-filexfer-02.txt)

@dkocher
Copy link
Contributor Author

dkocher commented Mar 27, 2015

I think the length field is just omitted in the spec because it is described in the General Packet Format section.

@hierynomus
Copy link
Owner

That field is actually handled in the SFTPEngine#transmit method. That ensures that it transmits the packet length first, and then the payload (packet type, etc). So I do think that the length field should not be there (at least according to the spec)

@dkocher
Copy link
Contributor Author

dkocher commented Mar 27, 2015

I will run some interoperability tests with OpenSSH if omitting the field is fine then.

@dkocher
Copy link
Contributor Author

dkocher commented Mar 27, 2015

I have errors when omitting the length in the before the raw bytes in the WRITE packet.

net.schmizz.sshj.sftp.SFTPException: EOF while reading packet
    at net.schmizz.sshj.sftp.PacketReader.readIntoBuffer(PacketReader.java:54)
    at net.schmizz.sshj.sftp.PacketReader.getPacketLength(PacketReader.java:59)
    at net.schmizz.sshj.sftp.PacketReader.readPacket(PacketReader.java:75)
    at net.schmizz.sshj.sftp.PacketReader.run(PacketReader.java:87)

@hierynomus
Copy link
Owner

I'll merge this PR, as it indeed fixes the bug... Still wondering though why we need to length field in the SSH_FXP_WRITE packet. Opened #187 for this.

hierynomus added a commit that referenced this pull request Apr 1, 2015
Fix length field for SSH_FXP_WRITE packets.
@hierynomus hierynomus merged commit b123a6a into hierynomus:master Apr 1, 2015
hierynomus added a commit that referenced this pull request Apr 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants