Add ability for server to receive file permissions for Open and Mkdir requests #546
Conversation
capnspacehook
force-pushed
the
capnspacehook/server-side-open-perms
branch
from
2004cc3
to
2434a4f
Compare
There was a problem hiding this comment.
I’m a bit warry at providing asymmetric capabilities. This code might allow the server to support this, but the client still wouldn’t be able to support this. As well, I’m unsure how this would impact the request-server-side of the implementation.
But it could still have some utility, so I’m not hostile to the idea of patching this in.
| defaultFileMode = 0o644 | ||
| defaultDirMode = 0o755 |
There was a problem hiding this comment.
These constants should be grouped individually in their own const so that a block-level documentation can cover them both.
Prefer to also have them be typed constants, so that we can simply use mode := defaultDirMode .
| if p.Attrs != nil { | ||
| attrs, _ := unmarshalFileStat(p.Flags, p.Attrs.([]byte)) |
There was a problem hiding this comment.
Either p.Attrs should be typed []byte or we should properly handle the case where it is not []byte.
It should be sufficient to use:
switch pattrs := p.Attrs.(type) {
case []byte:
…
case *FileStat:
… // since this would be a fast toFileMode(pattrs.Mode) might as well support it.
}
(I can’t speak to the legacy code doing differently. If I were redoing the FXP_SET_STAT packets, I’d do it quite a bit differently… I mean, I already did it’s in internal/encoding/ssh/filexfer…)
With this change, SFTP clients that support sending file attributes in SFTP
openandmkdirrequests can now specify the permissions of files and directories when they are created. To avoid breaking backwards compatibility and to keep this PR as small as possible, the SFTP client is not changed so support this.