droyo/styx

Weird behavior when using styx.Directory with 9front

Closed this issue · 2 comments

If one happens to Topen a file before doing a Tstat or Twalk, then the file mode check in Ropen seems to incorrect. This leads to the IsDir() check failing and preventing the styx.Directory being used. This seems to persistent for the session, and prevents even proper stats from being done on the file itself.

The client I was using to test was 9front, which didn't seem to Twalk or Tstat "/" before opening it if you cd(Topen) into the mounted directory right after mounting it.

My local solution was to remove the mode.IsDir() check from this code snippet in request.go:152

if dir, ok := rwc.(Directory); ok && mode.IsDir() {
		f = styxfile.NewDir(dir, t.Path(), t.session.conn.qidpool)
	} else {
		f, err = styxfile.New(rwc)
}

but I assume there is a better solution.

this seems to have solves the problem of something like pwd unable to open the parent file. However, if the user performs does do a cd first, the client seems to never request a Tread for the directory itself.

Example of a bad transaction(cd before an ls)

accepted connection from 192.168.1.6:40204
→ 65535 Tversion msize=8216 version="9P2000"
← 65535 Rversion msize=8216 version="9P2000"
→ 048 Tauth afid=1503 uname="moody" aname=""
← 048 Rerror ename="not supported"
→ 048 Tattach fid=1503 afid=NOFID uname="moody" aname=""
← 048 Rattach qid="type=128 ver=0 path=1"
→ 048 Twalk fid=1503 newfid=1438 ""
← 048 Rwalk wqid=""
→ 048 Topen fid=1438 mode=0
← 048 Ropen qid="type=0 ver=0 path=2" iounit=0
→ 048 Tclunk fid=1438
← 048 Rclunk
→ 048 Twalk fid=1503 newfid=1527 ""
← 048 Rwalk wqid=""
→ 048 Tstat fid=1527
← 048 Rstat type=0 dev=0 qid="type=0 ver=0 path=2" mode=20000000777 atime=1557208107 mtime=1557208107 length=0 name="." uid="" gid="" muid=""
→ 046 Tclunk fid=1527
← 046 Rclunk

Performing the ls first

accepted connection from 192.168.1.6:62090
→ 65535 Tversion msize=8216 version="9P2000"
← 65535 Rversion msize=8216 version="9P2000"
→ 085 Tauth afid=1526 uname="moody" aname=""
← 085 Rerror ename="not supported"
→ 085 Tattach fid=1526 afid=NOFID uname="moody" aname=""
← 085 Rattach qid="type=128 ver=0 path=1"
→ 048 Twalk fid=1526 newfid=1527 ""
← 048 Rwalk wqid=""
→ 048 Tstat fid=1527
← 048 Rstat type=0 dev=0 qid="type=128 ver=0 path=2" mode=20000000777 atime=1557208127 mtime=1557208127 length=0 name="." uid="" gid="" muid=""
→ 046 Tclunk fid=1527
← 046 Rclunk
→ 048 Twalk fid=1526 newfid=1524 ""
← 048 Rwalk wqid=""
→ 048 Topen fid=1524 mode=0
← 048 Ropen qid="type=128 ver=0 path=2" iounit=0
→ 048 Tread fid=1524 offset=0 count=8192
← 048 Rread count=61
→ 048 Tread fid=1524 offset=61 count=8192
← 048 Rread count=0
→ 048 Tclunk fid=1524
← 048 Rclunk
droyo commented

Thanks for the report.

The first indication that something is wrong is that the qid.path in the Ropen for the root does not match the qid.path in the Rattach. I think the problem is here:

styx/conn.go

Lines 344 to 346 in 175c4b5

s.files.Put(m.Fid(), file{name: "/", rwc: nil})
c.clearTag(m.Tag())
c.Rattach(m.Tag(), c.qid(".", styxproto.QTDIR))

We name the root directory / but store its qid at .. This makes the assumption in the comments here incorrect:

styx/request.go

Lines 147 to 149 in 175c4b5

// The type of the file (regular or directory) will have been
// established in a previous Twalk request.
qid := t.session.conn.qid(t.Path(), 0)

The reason the second example (ls first) works is that it gives the message to the styx.Handler which responds with the appropriate file type.

The fix would be to name the root file consistently (either "." or "/") and address any unintended side effects.