pkg
Misc questions & thoughts on Server(s) & code
Some misc questions & notes. Feel free to close if I misdirected this to the wrong forum.
I'm a little confused why the sftp package has two servers: the Server type and the RequestServer type. I guess the Server type hard-codes *os.File and serves the os filesystem (rooted at /), and the RequestServer is the extensible one.
But RequestServer looks like maybe it's a work-in-progress?
-
https://godoc.org/github.com/pkg/sftp#Handlers doesn't say the precedence between FileGet and FileCmd if both are set.
-
https://godoc.org/github.com/pkg/sftp#FileReader returns an
io.ReaderAt, but how does it handle Close calls from the client, then?
It seems that ideally there'd just be one server implementation, and "serve the local disk" would be one Handler implementation (even the default one) if not overridden by the user.
Maybe request-example.go should be renamed mem-handler.go or something, as that's all that file is about.
Also, typo in "Handlers" and no period at the end of the sentence:
// InMemHandler returns a Hanlders object with the test handlers
Hey @bradfitz, this is the perfect forum for these questions.
The primary reason there are 2 servers right now is historical, that the filesystem based Server type predates the RequestServer. The Request based server was written as I (and others) wanted an SFTP server that you could write different backends for to store the files in places other than the filesystem (S3 in my case). My long term plans is to merge the 2 by converting the filesystem Server into a RequestServer backend. I was first giving the Request server time to mature before proposing this change.
I must apologize for the poor state of the RequestServer documentation as both your questions are in essence documentation bugs. I really need to find some time to try to improve it. I can answer your specific questions though.
https://godoc.org/github.com/pkg/sftp#Handlers doesn't say the precedence between FileGet and FileCmd if both are set.
When implementing Handlers for the RequestServer you usually should implement all 4 handlers. Each handler handles different types of client requests. FileGet is read requests, FileCmd are for commands on files (Rename, Remove, etc), FilePut is for uploading files and FileList are for requests that return file info (List, Stat, etc).
https://godoc.org/github.com/pkg/sftp#FileReader returns an io.ReaderAt, but how does it handle Close calls from the client, then?
The RequestServer does support what would basically be an io.ReaderAtCloser (if such a thing existed). When done with the Request it attempts to assert the io.ReaderAt object to io.Closer and calls Close if successful. When doing the initial implementation I wasn't sure if every backend would require Close (still not 100%) and I noticed this pattern in other places in the standard library.
Maybe request-example.go should be renamed mem-handler.go or something, as that's all that file is about.
This was my plan when I convert the filesystem server to a RequestServer backend. The example memory server would be converted to a second RequestServer backend.
Thank you for taking the time to look at the code and ask your questions. Makes me think maybe I should go ahead and create roadmap and documentation tickets.
Oh.. and I'll be sure to fix that missing punctuation. :)
https://godoc.org/github.com/pkg/sftp#FileReader returns an io.ReaderAt, but how does it handle Close calls from the client, then?
The RequestServer does support what would basically be an io.ReaderAtCloser (if such a thing existed).
@bradfitz may have a point here.
I just implemented a request server backend, and as requirement I have to know when files are closed after reading or writing (so essentially detect if an upload was finished, or download was finished).
Afaict, the RequestServer does not see the close(), only the file (or actually the io.WriterAtCloser). So in order to track the close() calls i had to 1) wrap the file handle to intercept the close() and 2) duplicate the bookkeeping of open files in the requestserver.
It seems to works ok, wrapping the file is no problem, but the duplication of bookkeeping does not feel like an elegant solution.
There might be an easier way to do this, but i'm not very familiar with the code yet and I did not immediately see a another way.
A pending problem I still have : I can not distinguish between a close() from the client and a close() for cleanup of a disconnected session - which is what I actually need. I am not sure what the best solution for this would be, but relaying explicite close() request would give that information. Maybe i could create a "new issue" for this.
I must say however that the "requestserver" approach feels very nice and easy to implement a backend on.
@sugacube I don't fully understand your issues and it doesn't really seem directly related to Brad's questions... so may I suggest a new ticket. I'm curious about why you needed to replicate the bookkeeping and why you need to know about when close is called and the circumstances around the call. If I better understood your use case I might have an idea of how the library might be tweaked to help (if possible).
Thanks.
I have been utterly confused, and I'll admit I'm not a great Go programmer, but perhaps someone has an example on how to use the RequestServer to use an underlying OS with examples of how to force the user to stay in their configurable base directory? There are elegant sources such as sftpgo but they are a much for my understanding and small use cases. Can anyone share some simple code to go from the in-mem source FS to a real one? I'd be eternally grateful!
Unfortunately, there are a lot of really weird edge-cases involved in confining a user to a specific directory. 😔 It’s far harder than you might think than just ensuring the directory path is /directory/prefix.
I’m not sure the in-memory source FS even properly restricts on to a specific directory as well. I did a lot of work to make sure it was as close to POSIX as possible, which of course, carries over all the wonderful symlink issues that make a real FS so hard to chroot jail.
