request server: add an optional RealpathFileLister interface

[drakkan]

This allow to customize the responses for SSH_FXP_REALPATH requests
and so implementing features like a start directory.

I'm not sure if this is the best way to add this feature, maybe we could consider a server option too

[puellanivis]

I’m kind of hesitant to add yet more behaviors exposed through these interfaces… I’m not sure what a better solution would look like, but I feel like this path is unsustainable and overly complicated.

[drakkan]

Thank you for the review, I tryed to fix the code based on your comments. Other libraries that I use within SFTPGo have a similar structure, for example:

• https://pkg.go.dev/golang.org/x/net/webdav#FileSystem
• https://pkg.go.dev/github.com/fclairamb/ftpserverlib#ClientDriver

In this case we could also add a server option that allows to set the start directory and use this value instead of / in cleanPath.
An optional interface allows full control to the caller but I'm not sure that this is required.

Do you have better ideas?

[puellanivis]

Yeah, following up on my concern about the ReadPathLister this is direction that we’re wanting to go towards: extensible interfaces. I just had a knee-jerk reaction to it. 😆

[drakkan]

Thank you

[drakkan]

Hi @puellanivis,

I'm trying to add a start directory feature to SFTPGo, see drakkan/sftpgo#705.

I implemented the RealPathLister interface and with smart clients like sftp CLI or FileZilla this is enough. These clients request the current directory when they connect and then send absolute paths.

I started to write the test cases, take a look here:

currentDir, err := client.Getwd()
assert.NoError(t, err)
assert.Equal(t, startDir, currentDir)

entries, err := client.ReadDir(".")

client.Getwd and client.RealPath are ok, but if I do client.ReadDir(".") I get the / contents: our client sends a relative path and it is converted server side as absolute in our cleanPath method. I think this happen only for the request server.

I think it would be better to avoid path normalization server side and delegate them to the request server implementation but it would be a backward incompatible change at this point.

Add a check for relative path and call the RealPathLister could do the trick but it seems quite ugly and I don't want to add something like this.
Do you have better suggestions? Thanks

[puellanivis]

🤔 Hm. It seems the standard never really required all paths to be absolute. Although, it specifies only that relative paths be from a rooted point (the user’s home directory), and provides no way to change that directory once set. The fact that the relative directory support does not have any way to traverse directories is precisely the reason why requiring all paths be absolute has worked for so long, client have to do that relative path tracking almost entirely themselves already, so why not also do it for the user directory as well.

I think is a case where we’re slightly out of spec, and should be doing different behavior, but also… the assumptions made lead to a design that is going to be difficult to “fix” now.