request server: add WithStartDirectory option

[drakkan]

Alternative approach to support a start directory.

I tested #497 and it works fine but I don't like that pkg/sftp clean the path and I have to clean the raw path again in my code.

This approach seems cleaner and the RealPathFileLister is useless if we go this way, we can remove this interface.

The only drawback is that NewRequest now requires an already cleaned path. Prior to this patch the path passed to NewRequest was converted to an absolute UNIX path using / as base. I'll try to fix this compatibility issue later

[drakkan]

Hi @puellanivis

thanks for your review, I'll fix the PR based on your comments.

I know that removing the RealPathLister is a breaking change, but it does not work for real use cases. It only works if you have a client that initially requests the start directory and then sends absolute paths based on the response. I think it is very unlikely that anyone actually uses this interface. If someone is really using it, they should fix their code by migrating to the new server option.

[puellanivis]

We can just label the interface as deprecated, and say that implementing it won’t actually do anything. The same as we have ErrSshFxOk. Saying “they should fix their code” is all fine and good, but semver is pretty strict on this, and we need to bump the major version in order to remove it.

[drakkan]

Thank you!

[shanehooker]

I am not finding an example of how to use this with a real file system. I am a bit confused by the InMemHandler(). I would have prefered the WithStartDirectory() to have been a method applied to NewServer() which would have done the same thing as starting 'sftp-server -d [startDirectory]'.

Is there an example of how this can be called and work with a real file system?

[puellanivis]

The reason for NewRequestServer over NewServer is that the latter can only operate upon the local OS filesystem. Meaning, you cannot establish some other backing store, like s3, or even a caching temporary filesystem held in memory, lost during system restart (that is InMemoryHandler). So, we have elected to essentially “deprecate” NewServer as an insuffienciently extendable interface.

sftp-server -d [startDirectory] works by first doing an os.Chdir() before starting up the service, and letting the inherent OS filesystem management take care of the rest. You should be able to get the NewServer to work the same way.