diff --git a/conn.go b/conn.go index bcbc21e..aa632ac 100644 --- a/conn.go +++ b/conn.go @@ -161,10 +161,6 @@ func (c *conn) qid(name string, qtype uint8) styxproto.Qid { return c.qidpool.Put(name, qtype) } -func (c *conn) getQid(name string, qtype uint8) (styxproto.Qid, bool) { - return c.qidpool.Get(name) -} - // All request contexts must have their cancel functions // called, to free up resources in the context. Returns false // if the tag is already cancelled diff --git a/go.mod b/go.mod index 7b14752..05d4ff7 100644 --- a/go.mod +++ b/go.mod @@ -1,5 +1,5 @@ module aqwari.net/net/styx -go 1.16 +go 1.19 require aqwari.net/retry v0.0.0-20180428204214-1281ce5d8df0 diff --git a/internal/qidpool/pool.go b/internal/qidpool/pool.go index 84ef385..5cfbc19 100644 --- a/internal/qidpool/pool.go +++ b/internal/qidpool/pool.go @@ -42,8 +42,6 @@ func (p *Pool) Put(name string, qtype uint8) styxproto.Qid { m[name] = qid } }) - - p.m.Put(name, qid) return qid } diff --git a/internal/styxfile/file.go b/internal/styxfile/file.go index e6be429..0d1b01a 100644 --- a/internal/styxfile/file.go +++ b/internal/styxfile/file.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "path/filepath" "time" "aqwari.net/net/styx/internal/sys" @@ -110,6 +111,10 @@ func Stat(buf []byte, file Interface, name string, qid styxproto.Qid) (styxproto return nil, err } } else { + // name is an absolute path, make sure we don't pass an absolute path to statGuess, + // otherwise we may get back an absolute path if the file does not have a Name() method, + // which would be incorrect since stat names cannot contain slashes. + name := filepath.Base(name) fi = statGuess{file, name, qid.Type()} } uid, gid, muid := sys.FileOwner(fi) @@ -125,6 +130,8 @@ func Stat(buf []byte, file Interface, name string, qid styxproto.Qid) (styxproto return stat, nil } +// FIXME: When statGuess is used with a styxfile.Directory, none of the stat methods are found, +// and we fall back on incorrectly using guessed values every time. type statGuess struct { file Interface name string diff --git a/request.go b/request.go index c7be0e9..d9aed12 100644 --- a/request.go +++ b/request.go @@ -1,10 +1,11 @@ package styx import ( - "context" "os" "path" + "context" + "aqwari.net/net/styx/internal/styxfile" "aqwari.net/net/styx/internal/sys" "aqwari.net/net/styx/styxproto" @@ -265,7 +266,6 @@ func (t Tcreate) Rcreate(rwc interface{}, err error) { } if dir, ok := rwc.(Directory); t.Mode.IsDir() && ok { - f = styxfile.NewDir(dir, path.Join(t.Path(), t.Name), t.session.conn.qidpool) } else { f, err = styxfile.New(rwc) diff --git a/server_test.go b/server_test.go index f88a461..4a9103d 100644 --- a/server_test.go +++ b/server_test.go @@ -14,6 +14,7 @@ import ( "time" "aqwari.net/net/styx/internal/netutil" + "aqwari.net/net/styx/internal/styxfile" "aqwari.net/net/styx/styxproto" ) @@ -86,25 +87,46 @@ func (emptyFS) Serve9P(s *Session) { switch req := s.Request().(type) { case Tstat: if req.Path() == "/" { - req.Rstat(emptyDir(req.Path()), nil) + req.Rstat(emptyDir{emptyStatDir(req.Path())}, nil) } case Topen: - req.Ropen(emptyDir(req.Path()), nil) + req.Ropen(emptyDir{emptyStatDir(req.Path())}, nil) } } } -type emptyDir string +type emptyStatFile string + +// fs.FileInfo +func (s emptyStatFile) Mode() os.FileMode { return 0222 } +func (s emptyStatFile) IsDir() bool { return s.Mode().IsDir() } +func (s emptyStatFile) Name() string { return string(s) } +func (s emptyStatFile) Sys() interface{} { return nil } +func (s emptyStatFile) Size() int64 { return 0 } +func (s emptyStatFile) ModTime() time.Time { return time.Time{} } + +type emptyStatDir string + +// fs.FileInfo +func (s emptyStatDir) Mode() os.FileMode { return 0222 | os.ModeDir } +func (s emptyStatDir) IsDir() bool { return s.Mode().IsDir() } +func (s emptyStatDir) Name() string { return string(s) } +func (s emptyStatDir) Sys() interface{} { return nil } +func (s emptyStatDir) Size() int64 { return 0 } +func (s emptyStatDir) ModTime() time.Time { return time.Time{} } + +type emptyFile struct{ emptyStatFile } + +var _ styxfile.Interface = emptyFile{} + +func (f emptyFile) ReadAt(p []byte, offset int64) (written int, err error) { return 0, io.EOF } +func (f emptyFile) WriteAt(p []byte, offset int64) (int, error) { return 0, styxfile.ErrNotSupported } +func (f emptyFile) Close() error { return nil } + +type emptyDir struct{ emptyStatDir } + +var _ styxfile.Directory = emptyDir{} -// os.FileInfo -func (d emptyDir) Mode() os.FileMode { return os.ModeDir } -func (d emptyDir) IsDir() bool { return d.Mode().IsDir() } -func (d emptyDir) Name() string { return string(d) } -func (d emptyDir) Sys() interface{} { return nil } -func (d emptyDir) Size() int64 { return 0 } -func (d emptyDir) ModTime() time.Time { return time.Time{} } - -// styx.Directory func (d emptyDir) Readdir(int) ([]os.FileInfo, error) { return nil, nil } func chanServer(t *testing.T, handler Handler) (in, out chan styxproto.Msg) { @@ -488,6 +510,98 @@ func TestWalk(t *testing.T) { } } +func TestWalkNonexistent(t *testing.T) { + srv := testServer{test: t} + srv.callback = func(req, rsp styxproto.Msg) { + if _, ok := req.(styxproto.Twalk); ok { + if _, ok := rsp.(styxproto.Rerror); !ok { + t.Errorf("expected Rerror response to nonexistent Twalk, instead got: %T", rsp) + } + } + } + srv.handler = HandlerFunc(func(s *Session) { + for s.Next() { + switch req := s.Request().(type) { + case Twalk: + t.Logf("Twalk %s", req.Path()) + req.Rwalk(nil, errors.New("not found")) + // If the walk resulted in an error, then no Qid should have been created for this path + if _, ok := s.conn.qidpool.Get(req.Path()); ok { + t.Error("qid was created when it shouldn't have been") + } + } + } + }) + + srv.runMsg(func(enc *styxproto.Encoder) { + enc.Twalk(1, 0, 1, "nonexistent") + }) +} + +func TestTcreate(t *testing.T) { + srv := testServer{test: t} + + type expectedstat struct { + name string + mode os.FileMode + } + fidnames := map[uint32]expectedstat{ + 1: {name: "dir", mode: 0222 | os.ModeDir}, + 2: {name: "file", mode: 0222}, + } + + srv.callback = func(req, rsp styxproto.Msg) { + if _, ok := rsp.(styxproto.Rerror); ok { + t.Errorf("got %T response to %T", rsp, req) + } + if req, ok := req.(styxproto.Tstat); ok { + if rsp, ok := rsp.(styxproto.Rstat); !ok { + t.Errorf("got %T response to %T", rsp, req) + } else { + expected := fidnames[req.Fid()] + name := string(rsp.Stat().Name()) + if name != expected.name { + t.Errorf("expected name to be %s, instead got %s", expected.name, name) + } + // FIXME: For directories, the mode does not match + mode := styxfile.ModeOS(rsp.Stat().Mode()) + if mode != expected.mode { + t.Errorf("expected mode to be %s, instead got %s", expected.mode, mode) + } + } + } + } + srv.handler = HandlerFunc(func(s *Session) { + for s.Next() { + switch req := s.Request().(type) { + case Tcreate: + t.Logf("Tcreate %s %s", req.Path(), req.NewPath()) + var f any + if req.Mode.IsDir() { + f = emptyDir{emptyStatDir(req.Name)} + } else { + f = emptyFile{emptyStatFile(req.Name)} + } + req.Rcreate(f, nil) + case Twalk: + // Empty walks get automatically handled, no need to handle + case Tstat: + // Because Rcreate returns an opened file, Tstat is called on styxfile.Interface or styxfile.Directory, + // so it will use styxfile.Stat to get stat, no need to handle + } + } + }) + + srv.runMsg(func(enc *styxproto.Encoder) { + enc.Twalk(1, 0, 1) + enc.Tcreate(1, 1, "dir", 0222|styxproto.DMDIR, styxproto.DMREAD) + enc.Tstat(1, 1) + enc.Twalk(1, 0, 2) + enc.Tcreate(1, 2, "file", 0222, styxproto.DMREAD) + enc.Tstat(1, 2) + }) +} + func blankQid() styxproto.Qid { buf := make([]byte, styxproto.QidLen) qid, _, err := styxproto.NewQid(buf, 0, 0, 0) diff --git a/walk.go b/walk.go index de66bf4..91c3a60 100644 --- a/walk.go +++ b/walk.go @@ -1,7 +1,6 @@ package styx import ( - "errors" "fmt" "os" "path" @@ -181,12 +180,7 @@ func (t Twalk) Rwalk(info os.FileInfo, err error) { var mode os.FileMode if err == nil { mode = info.Mode() - fqid, found := t.session.conn.getQid(t.Path(), styxfile.QidType(styxfile.Mode9P(mode))) - if !found { - err = errors.New("rwalk did not find file") - } else { - qid = fqid - } + qid = t.session.conn.qid(t.Path(), styxfile.QidType(styxfile.Mode9P(mode))) } t.walk.filled[t.index] = 1 elem := walkElem{qid: qid, index: t.index, err: err}