droyo / styx

Go library for the 9P filesystem protocol
MIT License
64 stars 18 forks source link

Fix Rwalk() setting QIDs erroneously #30

Closed seh-msft closed 2 years ago

seh-msft commented 3 years ago

Commit message: request.go: add getQid() to permit lookup of QIDs without accidentally setting non-existant QIDs. ;; make Rwalk() use getQid()

Rwalk() originally called Put() in the Qid Pool to check if the Qid existed in the tree.

In the case of a new file being created in the tree, a Twalk, then a Tcreate, would occur.

Tcreate should be the only origin of 'new' QIDs to the core file system.

The original Rwalk call would, for a file that doesn't exist yet, set a QID with a type of 128, being a directory.

Thus, any new file created would be created with a Qtype of a directory, even if the file mode wasn't that of a directory.

Weird behavior ensues where writes fail from the client side as 'file is a directory'.

Additionally, the Put() function for the QID pool does not override existing entries with the provided value. This may be intended, but also meant that when the original value was erroneously set by Rwalk, that future correct calls, such as from Rcreate, would fail to set the Qtype correctly.

This PR resolves the Rwalk() Qid creation issue by introducing a getQid() function which acts similar to a standard lookup on a Go map type.

seh-msft commented 3 years ago

Incidentally, I've noticed that this caught the go.(mod sum) files.

Styx won't build by default under newer Go versions that expect modules.

Tested on:

styx» go version
go version go1.16.5 windows/amd64
styx»

No issues seem to arise from using Go modules with styx.