search

pkg / sftp

SFTP support for the go.crypto/ssh package
BSD 2-Clause "Simplified" License
1.49k stars 379 forks source link

fix: add support for attrs in `sshFxpOpenPacket` for `Server` #567

Closed mafredri closed 6 months ago

mafredri commented 7 months ago

Sending permissions (chmod) and atime/mtime is not uncommon for sftp clients via the sshFxpOpenPacket, however, flags/attrs seems to have been explicitly ignored even though they're part of the RFC (https://filezilla-project.org/specs/draft-ietf-secsh-filexfer-02.txt).

This leads to issues like https://github.com/mutagen-io/mutagen/issues/459, where the client is relying on the attrs being applied.

This only adds support for the feature in the old Server implementation, which is what I used. Further work is most likely required to support it in the RequestServer as well (however, I have no experience with that part of the code base). I believe no backwards-incompatible changes were introduced since all we're doing is parsing more of the packet.

A bug was also fixed in sshFxpSetstatPacket and sshFxpFsetstatPacket where attrs were parsed in the wrong order.

mafredri commented 7 months ago

@puellanivis thanks for the thorough and timely review/feedback, as always. It's great to come back to this project every once in a while and see that it's still kicking. ☺️

I believe I've amended all of the feedback (at least what I felt confident/comfortable changing), including a little bit of cleanup of the old code.

mafredri commented 6 months ago

@puellanivis hey, just wanted to check if there are any blockers/actions left for this PR? Cheers, and happy new year!

puellanivis commented 6 months ago

Closing in deference to #574 which is pulling in the changes.

Thanks for your contribution though!