Skip to content

Fix length field for SSH_FXP_WRITE packets. - #181

Merged
hierynomus merged 1 commit into
hierynomus:masterfrom
iterate-ch:issue-180
Apr 1, 2015
Merged

Fix length field for SSH_FXP_WRITE packets.#181
hierynomus merged 1 commit into
hierynomus:masterfrom
iterate-ch:issue-180

Conversation

@dkocher

@dkocher dkocher commented Feb 24, 2015

Copy link
Copy Markdown
Contributor

Proposed fix for #180.

@hierynomus

Copy link
Copy Markdown
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

dkocher commented Mar 27, 2015

Copy link
Copy Markdown
Contributor Author

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

@hierynomus

Copy link
Copy Markdown
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

dkocher commented Mar 27, 2015

Copy link
Copy Markdown
Contributor Author

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

@dkocher

dkocher commented Mar 27, 2015

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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.

2 participants