Revert incorrect Rwalk changes, introduce additional test

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

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

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
 }
 

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

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)

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)

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}