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

Memfile stuff #561

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b9d7f36
Use errors.Is
Oct 24, 2023
8513fed
Start tracking the file mode.
Oct 31, 2023
07a6b6a
Move memfile's Setstat function, add features.
Oct 31, 2023
7bf7a25
Make a new interface for direct SetStat access.
Oct 31, 2023
6b2b978
Add SetAttributes as an alternate option.
Oct 31, 2023
688c055
Rip out SetAttributes.
Nov 1, 2023
a0a4a52
Get rid of FileStatSetter.
Nov 1, 2023
8ee6563
Use errors.Is everywhere.
Nov 1, 2023
f215740
Merge SetStat back into Filecmd.
Nov 1, 2023
d41c7d4
Export ToFileMode and FromFileMode.
Nov 1, 2023
87fa218
Silence a linter warning.
Nov 1, 2023
a5d4316
memfile: Check file permissions.
Nov 1, 2023
4404cc8
Add some comments.
Nov 1, 2023
656a9db
Explictly forbid changing the mode of a symlink.
Nov 1, 2023
b1d53f7
Expose To/FromFileMode for plan9.
Nov 1, 2023
cbf7fcd
Add FileStat.MarshalTo and FAF.ForRequest.
Nov 2, 2023
4b73c93
Don't put the flags in the attrs.
Nov 2, 2023
82cd5ca
FileOpenFlags.ForRequest
Nov 2, 2023
5fd4ff4
Revert "Expose To/FromFileMode for plan9."
Nov 2, 2023
660c973
Revert "Export ToFileMode and FromFileMode."
Nov 2, 2023
6c452cc
Revert "Use errors.Is everywhere."
Nov 2, 2023
6563249
Revert "Silence a linter warning."
Nov 2, 2023
ad0d7df
Comment fixes.
Nov 2, 2023
7e1a518
Let Setstat follow symlinks, and rework comments.
Nov 2, 2023
d3a55c7
Shadow err to avoid the value leaking.
Nov 2, 2023
bff300b
Move the chmod logic to it's own method.
Nov 2, 2023
1d040d6
Make marshalFileInfo use fileStat.MarshalTo
Nov 2, 2023
706f4f1
Use 3 digit modes, not 4.
Nov 2, 2023
1ae2738
MarshalTo now takes a []byte argument.
Nov 3, 2023
3d7324a
Add warning comments around the Extended logic.
Nov 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions request-attrs.go
Expand Up @@ -44,6 +44,37 @@ func newFileAttrFlags(flags uint32) FileAttrFlags {
}
}

func (r *Request) SetAttributes(flags FileAttrFlags, attrs FileStat) error {
zelch marked this conversation as resolved.
Show resolved Hide resolved
r.Method = "Setstat"

buf := []byte{}
zelch marked this conversation as resolved.
Show resolved Hide resolved

r.Flags = 0

if flags.Size {
r.Flags |= sshFileXferAttrSize
buf = marshalUint64(buf, attrs.Size)
}
if flags.UidGid {
r.Flags |= sshFileXferAttrUIDGID
buf = marshalUint32(buf, attrs.UID)
buf = marshalUint32(buf, attrs.GID)
}
if flags.Permissions {
r.Flags |= sshFileXferAttrPermissions
buf = marshalUint32(buf, attrs.Mode)
}

if flags.Acmodtime {
r.Flags |= sshFileXferAttrACmodTime
buf = marshalUint32(buf, attrs.Atime)
buf = marshalUint32(buf, attrs.Mtime)
}
zelch marked this conversation as resolved.
Show resolved Hide resolved

r.Attrs = buf
return nil
}

// AttrFlags returns a FileAttrFlags boolean struct based on the
// bitmap/uint32 file attribute flags from the SFTP packaet.
func (r *Request) AttrFlags() FileAttrFlags {
Expand Down
41 changes: 27 additions & 14 deletions request-example.go
Expand Up @@ -72,7 +72,7 @@ func (fs *root) putfile(pathname string, file *memFile) error {
return os.ErrInvalid
}

if _, err := fs.lfetch(pathname); err != os.ErrNotExist {
if _, err := fs.lfetch(pathname); !errors.Is(err, os.ErrNotExist) {
return os.ErrExist
}

Expand Down Expand Up @@ -110,6 +110,7 @@ func (fs *root) openfile(pathname string, flags uint32) (*memFile, error) {

file := &memFile{
modtime: time.Now(),
mode: 0644,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not be defaulting this value. Please, pass it as a parameter, and document in the godoc that it is ignored if the creation flag is not set.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, I had defaulted to the value that we previously hard coded. I'll take a crack at getting it from the initial request.

}

if err := fs.putfile(pathname, file); err != nil {
Expand Down Expand Up @@ -140,6 +141,25 @@ func (fs *root) openfile(pathname string, flags uint32) (*memFile, error) {
return file, nil
}

func (fs *root) SetStat(r *Request, flags *FileAttrFlags, attrs *FileStat) error {
zelch marked this conversation as resolved.
Show resolved Hide resolved
file, err := fs.openfile(r.Filepath, sshFxfWrite)
if err != nil {
return err
}

if flags.Permissions {
file.mode = attrs.Mode
zelch marked this conversation as resolved.
Show resolved Hide resolved
}
// We only have mtime, not atime.
if flags.Acmodtime {
file.modtime = time.Unix(int64(attrs.Mtime), 0)
}
if flags.Size {
return file.Truncate(int64(attrs.Size))
zelch marked this conversation as resolved.
Show resolved Hide resolved
}
return nil
}

func (fs *root) Filecmd(r *Request) error {
if fs.mockErr != nil {
return fs.mockErr
Expand All @@ -151,16 +171,8 @@ func (fs *root) Filecmd(r *Request) error {

switch r.Method {
case "Setstat":
file, err := fs.openfile(r.Filepath, sshFxfWrite)
if err != nil {
return err
}

if r.AttrFlags().Size {
return file.Truncate(int64(r.Attributes().Size))
}

return nil
flags := r.AttrFlags()
return fs.SetStat(r, &flags, r.Attributes())
zelch marked this conversation as resolved.
Show resolved Hide resolved

case "Rename":
// SFTP-v2: "It is an error if there already exists a file with the name specified by newpath."
Expand Down Expand Up @@ -209,7 +221,7 @@ func (fs *root) rename(oldpath, newpath string) error {
}

target, err := fs.lfetch(newpath)
if err != os.ErrNotExist {
if !errors.Is(err, os.ErrNotExist) {
if target == file {
// IEEE 1003.1: if oldpath and newpath are the same directory entry,
// then return no error, and perform no further action.
Expand Down Expand Up @@ -507,7 +519,7 @@ func (fs *root) exists(path string) bool {

_, err = fs.lfetch(path)

return err != os.ErrNotExist
return !errors.Is(err, os.ErrNotExist)
}

func (fs *root) fetch(pathname string) (*memFile, error) {
Expand Down Expand Up @@ -544,6 +556,7 @@ type memFile struct {
modtime time.Time
symlink string
isdir bool
mode uint32

mu sync.RWMutex
content []byte
Expand All @@ -569,7 +582,7 @@ func (f *memFile) Mode() os.FileMode {
if f.symlink != "" {
return os.FileMode(0777) | os.ModeSymlink
}
return os.FileMode(0644)
return os.FileMode(f.mode)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function also now needs to that directories might have non-default modes.

}
func (f *memFile) ModTime() time.Time { return f.modtime }
func (f *memFile) IsDir() bool { return f.isdir }
Expand Down
6 changes: 6 additions & 0 deletions request-interfaces.go
Expand Up @@ -155,3 +155,9 @@ type ListerAt interface {
type TransferError interface {
TransferError(err error)
}

// FileStatSetter is an optional interface that a writerAt could implement in order to allow direct setting of stats.
// This is primarily used for the inmemhandler backend.
type FileStatSetter interface {
zelch marked this conversation as resolved.
Show resolved Hide resolved
SetStat(r *Request, flags *FileAttrFlags, attrs *FileStat) error
}