Merge pull request #526 from georgmu/request-server-relative-symlinks

request server: handle relative symlinks
This commit is contained in:
Nicola Murino 2022-10-16 15:57:09 +02:00 committed by GitHub
commit 43b3a5532d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 111 additions and 37 deletions

View File

@ -391,21 +391,6 @@ func (fs *root) Filelist(r *Request) (ListerAt, error) {
return nil, err
}
return listerat{file}, nil
case "Readlink":
symlink, err := fs.readlink(r.Filepath)
if err != nil {
return nil, err
}
// SFTP-v2: The server will respond with a SSH_FXP_NAME packet containing only
// one name and a dummy attributes value.
return listerat{
&memFile{
name: symlink,
err: os.ErrNotExist, // prevent accidental use as a reader/writer.
},
}, nil
}
return nil, errors.New("unsupported")
@ -434,7 +419,7 @@ func (fs *root) readdir(pathname string) ([]os.FileInfo, error) {
return files, nil
}
func (fs *root) readlink(pathname string) (string, error) {
func (fs *root) Readlink(pathname string) (string, error) {
file, err := fs.lfetch(pathname)
if err != nil {
return "", err
@ -525,8 +510,8 @@ func (fs *root) exists(path string) bool {
return err != os.ErrNotExist
}
func (fs *root) fetch(path string) (*memFile, error) {
file, err := fs.lfetch(path)
func (fs *root) fetch(pathname string) (*memFile, error) {
file, err := fs.lfetch(pathname)
if err != nil {
return nil, err
}
@ -537,7 +522,12 @@ func (fs *root) fetch(path string) (*memFile, error) {
return nil, errTooManySymlinks
}
file, err = fs.lfetch(file.symlink)
linkTarget := file.symlink
if !path.IsAbs(linkTarget) {
linkTarget = path.Join(path.Dir(file.name), linkTarget)
}
file, err = fs.lfetch(linkTarget)
if err != nil {
return nil, err
}

View File

@ -74,6 +74,11 @@ type StatVFSFileCmder interface {
// FileLister should return an object that fulfils the ListerAt interface
// Note in cases of an error, the error text will be sent to the client.
// Called for Methods: List, Stat, Readlink
//
// Since Filelist returns an os.FileInfo, this can make it non-ideal for impelmenting Readlink.
// This is because the Name receiver method defined by that interface defines that it should only return the base name.
// However, Readlink is required to be capable of returning essentially any arbitrary valid path relative or absolute.
// In order to implement this more expressive requirement, implement [ReadlinkFileLister] which will then be used instead.
type FileLister interface {
Filelist(*Request) (ListerAt, error)
}
@ -94,7 +99,7 @@ type LstatFileLister interface {
//
// Up to v1.13.5 the signature for the RealPath method was:
//
// RealPath(string) string
// # RealPath(string) string
//
// we have added a legacyRealPathFileLister that implements the old method
// to ensure that your code does not break.
@ -104,6 +109,14 @@ type RealPathFileLister interface {
RealPath(string) (string, error)
}
// ReadlinkFileLister is a FileLister that implements the Readlink method.
// By implementing the Readlink method, it is possible to return any arbitrary valid path relative or absolute.
// This allows giving a better response than via the default FileLister (which is limited to os.FileInfo, whose Name method should only return the base name of a file)
type ReadlinkFileLister interface {
FileLister
Readlink(string) (string, error)
}
// This interface is here for backward compatibility only
type legacyRealPathFileLister interface {
FileLister

View File

@ -568,27 +568,67 @@ func TestRequestSymlink(t *testing.T) {
p := clientRequestServerPair(t)
defer p.Close()
_, err := putTestFile(p.cli, "/foo", "hello")
const CONTENT_FOO = "hello"
const CONTENT_DIR_FILE_TXT = "file"
const CONTENT_SUB_FILE_TXT = "file-in-sub"
// prepare all files
_, err := putTestFile(p.cli, "/foo", CONTENT_FOO)
require.NoError(t, err)
err = p.cli.Mkdir("/dir")
require.NoError(t, err)
err = p.cli.Mkdir("/dir/sub")
require.NoError(t, err)
_, err = putTestFile(p.cli, "/dir/file.txt", CONTENT_DIR_FILE_TXT)
require.NoError(t, err)
_, err = putTestFile(p.cli, "/dir/sub/file-in-sub.txt", CONTENT_SUB_FILE_TXT)
require.NoError(t, err)
err = p.cli.Symlink("/foo", "/bar")
require.NoError(t, err)
err = p.cli.Symlink("/bar", "/baz")
require.NoError(t, err)
type symlink struct {
name string // this is the filename of the symbolic link
target string // this is the file or directory the link points to
//for testing
expectsNotExist bool
expectedFileContent string
}
symlinks := []symlink{
{name: "/bar", target: "/foo", expectedFileContent: CONTENT_FOO},
{name: "/baz", target: "/bar", expectedFileContent: CONTENT_FOO},
{name: "/link-to-non-existent-file", target: "non-existent-file", expectsNotExist: true},
{name: "/dir/rel-link.txt", target: "file.txt", expectedFileContent: CONTENT_DIR_FILE_TXT},
{name: "/dir/abs-link.txt", target: "/dir/file.txt", expectedFileContent: CONTENT_DIR_FILE_TXT},
{name: "/dir/rel-subdir-link.txt", target: "sub/file-in-sub.txt", expectedFileContent: CONTENT_SUB_FILE_TXT},
{name: "/dir/abs-subdir-link.txt", target: "/dir/sub/file-in-sub.txt", expectedFileContent: CONTENT_SUB_FILE_TXT},
{name: "/dir/sub/parentdir-link.txt", target: "../file.txt", expectedFileContent: CONTENT_DIR_FILE_TXT},
}
for _, s := range symlinks {
err := p.cli.Symlink(s.target, s.name)
require.NoError(t, err, "Creating symlink %q with target %q failed", s.name, s.target)
rl, err := p.cli.ReadLink(s.name)
require.NoError(t, err, "ReadLink(%q) failed", s.name)
require.Equal(t, s.target, rl, "Unexpected result when reading symlink %q", s.name)
}
// test fetching via symlink
r := p.testHandler()
fi, err := r.lfetch("/bar")
require.NoError(t, err)
assert.True(t, fi.Mode()&os.ModeSymlink == os.ModeSymlink)
for _, s := range symlinks {
fi, err := r.lfetch(s.name)
require.NoError(t, err, "lfetch(%q) failed", s.name)
require.True(t, fi.Mode()&os.ModeSymlink == os.ModeSymlink, "Expected %q to be a symlink but it is not.", s.name)
fi, err = r.lfetch("/baz")
require.NoError(t, err)
assert.True(t, fi.Mode()&os.ModeSymlink == os.ModeSymlink)
content, err := getTestFile(p.cli, "/baz")
require.NoError(t, err)
assert.Equal(t, []byte("hello"), content)
content, err := getTestFile(p.cli, s.name)
if s.expectsNotExist {
require.True(t, os.IsNotExist(err), "Reading symlink %q expected os.ErrNotExist", s.name)
} else {
require.NoError(t, err, "getTestFile(%q) failed", s.name)
require.Equal(t, []byte(s.expectedFileContent), content, "Reading symlink %q returned unexpected content", s.name)
}
}
checkRequestServerAllocator(t, p)
}
@ -714,9 +754,17 @@ func TestRequestReadlink(t *testing.T) {
require.NoError(t, err)
err = p.cli.Symlink("/foo", "/bar")
require.NoError(t, err)
rl, err := p.cli.ReadLink("/bar")
assert.NoError(t, err)
assert.Equal(t, "foo", rl)
assert.Equal(t, "/foo", rl)
_, err = p.cli.ReadLink("/foo")
assert.Error(t, err, "Readlink on non-symlink should fail")
_, err = p.cli.ReadLink("/foobar")
assert.Error(t, err, "Readlink on non-existent file should fail")
checkRequestServerAllocator(t, p)
}

View File

@ -187,6 +187,7 @@ func requestFromPacket(ctx context.Context, pkt hasPath, baseDir string) *Reques
// NOTE: given a POSIX compliant signature: symlink(target, linkpath string)
// this makes Request.Target the linkpath, and Request.Filepath the target.
request.Target = cleanPathWithBase(baseDir, p.Linkpath)
request.Filepath = p.Targetpath
case *sshFxpExtendedPacketHardlink:
request.Target = cleanPathWithBase(baseDir, p.Newpath)
}
@ -294,7 +295,12 @@ func (r *Request) call(handlers Handlers, pkt requestPacket, alloc *allocator, o
return filecmd(handlers.FileCmd, r, pkt)
case "List":
return filelist(handlers.FileList, r, pkt)
case "Stat", "Lstat", "Readlink":
case "Stat", "Lstat":
return filestat(handlers.FileList, r, pkt)
case "Readlink":
if readlinkFileLister, ok := handlers.FileList.(ReadlinkFileLister); ok {
return readlink(readlinkFileLister, r, pkt)
}
return filestat(handlers.FileList, r, pkt)
default:
return statusFromError(pkt.id(), fmt.Errorf("unexpected method: %s", r.Method))
@ -598,6 +604,23 @@ func filestat(h FileLister, r *Request, pkt requestPacket) responsePacket {
}
}
func readlink(readlinkFileLister ReadlinkFileLister, r *Request, pkt requestPacket) responsePacket {
resolved, err := readlinkFileLister.Readlink(r.Filepath)
if err != nil {
return statusFromError(pkt.id(), err)
}
return &sshFxpNamePacket{
ID: pkt.id(),
NameAttrs: []*sshFxpNameAttr{
{
Name: resolved,
LongName: resolved,
Attrs: emptyFileStat,
},
},
}
}
// init attributes of request object from packet data
func requestMethod(p requestPacket) (method string) {
switch p.(type) {