Merge branch 'zj-list-directories' into 'master'

Server implementation ListDirectories

See merge request gitlab-org/gitaly!868

7 files changed, 196 insertions, 11 deletions

diff --git a/changelogs/unreleased/zj-list-directories.yml b/changelogs/unreleased/zj-list-directories.yml
new file mode 100644
index 000000000..34864c938
--- /dev/null
+++ b/changelogs/unreleased/zj-list-directories.yml
@@ -0,0 +1,5 @@
+---
+title: Server implementation ListDirectories
+merge_request: 868
+author:
+type: added
diff --git a/internal/helper/repo.go b/internal/helper/repo.go
index a7348075a..cccc9d7d0 100644
--- a/internal/helper/repo.go
+++ b/internal/helper/repo.go
@@ -3,7 +3,6 @@ package helper
 import (
 	"os"
 	"path"
-	"strings"
 
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 
@@ -53,11 +52,7 @@ func GetPath(repo *pb.Repository) (string, error) {
 		return "", err
 	}
 
-	// Disallow directory traversal for security
-	separator := string(os.PathSeparator)
-	if strings.HasPrefix(relativePath, ".."+separator) ||
-		strings.Contains(relativePath, separator+".."+separator) ||
-		strings.HasSuffix(relativePath, separator+"..") {
+	if ContainsPathTraversal(relativePath) {
 		return "", status.Errorf(codes.InvalidArgument, "GetRepoPath: relative path can't contain directory traversal")
 	}
 
diff --git a/internal/helper/security.go b/internal/helper/security.go
new file mode 100644
index 000000000..8dae4f9c5
--- /dev/null
+++ b/internal/helper/security.go
@@ -0,0 +1,15 @@
+package helper
+
+import (
+	"os"
+	"strings"
+)
+
+// ContainsPathTraversal checks if the path contains any traversal
+func ContainsPathTraversal(path string) bool {
+	// Disallow directory traversal for security
+	separator := string(os.PathSeparator)
+	return strings.HasPrefix(path, ".."+separator) ||
+		strings.Contains(path, separator+".."+separator) ||
+		strings.HasSuffix(path, separator+"..")
+}
diff --git a/internal/helper/security_test.go b/internal/helper/security_test.go
new file mode 100644
index 000000000..cd31d9f73
--- /dev/null
+++ b/internal/helper/security_test.go
@@ -0,0 +1,24 @@
+package helper
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestContainsPathTraversal(t *testing.T) {
+	testCases := []struct {
+		path              string
+		containsTraversal bool
+	}{
+		{"../parent", true},
+		{"subdir/../../parent", true},
+		{"subdir/..", true},
+		{"subdir", false},
+		{"./subdir", false},
+	}
+
+	for _, tc := range testCases {
+		assert.Equal(t, tc.containsTraversal, ContainsPathTraversal(tc.path))
+	}
+}
diff --git a/internal/service/storage/listdirectories.go b/internal/service/storage/listdirectories.go
new file mode 100644
index 000000000..9d29c05e8
--- /dev/null
+++ b/internal/service/storage/listdirectories.go
@@ -0,0 +1,62 @@
+package storage
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+func (s *server) ListDirectories(req *pb.ListDirectoriesRequest, stream pb.StorageService_ListDirectoriesServer) error {
+	storageDir, err := helper.GetStorageByName(req.StorageName)
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "storage lookup failed: %v", err)
+	}
+
+	storageDir = storageDir + "/"
+
+	maxDepth := dirDepth(storageDir) + req.GetDepth()
+
+	var dirs []string
+	err = filepath.Walk(storageDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if info.IsDir() {
+			relPath := strings.TrimPrefix(path, storageDir)
+			if relPath == "" {
+				return nil
+			}
+
+			dirs = append(dirs, relPath)
+
+			if len(dirs) > 100 {
+				stream.Send(&pb.ListDirectoriesResponse{Paths: dirs})
+				dirs = dirs[:]
+			}
+
+			if dirDepth(path)+1 > maxDepth {
+				return filepath.SkipDir
+			}
+
+			return nil
+		}
+
+		return nil
+	})
+
+	if len(dirs) > 0 {
+		stream.Send(&pb.ListDirectoriesResponse{Paths: dirs})
+	}
+
+	return err
+}
+
+func dirDepth(dir string) uint32 {
+	return uint32(len(strings.Split(dir, string(os.PathSeparator)))) + 1
+}
diff --git a/internal/service/storage/listdirectories_test.go b/internal/service/storage/listdirectories_test.go
new file mode 100644
index 000000000..b98495dbf
--- /dev/null
+++ b/internal/service/storage/listdirectories_test.go
@@ -0,0 +1,89 @@
+package storage
+
+import (
+	"io"
+	"io/ioutil"
+	"os"
+	"path"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/config"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestListDirectories(t *testing.T) {
+	testDir := path.Join(testStorage.Path, t.Name())
+	require.NoError(t, os.MkdirAll(path.Dir(testDir), 0755))
+	defer os.RemoveAll(testDir)
+
+	// Mock the storage dir being our test dir, so results aren't influenced
+	// by other tests.
+	testStorages := []config.Storage{{Name: "default", Path: testDir}}
+
+	defer func(oldStorages []config.Storage) {
+		config.Config.Storages = oldStorages
+	}(config.Config.Storages)
+	config.Config.Storages = testStorages
+
+	repoPaths := []string{"foo", "bar", "bar/baz", "bar/baz/foo/buz"}
+	for _, p := range repoPaths {
+		dirPath := filepath.Join(testDir, p)
+		require.NoError(t, os.MkdirAll(dirPath, 0755))
+		require.NoError(t, ioutil.WriteFile(filepath.Join(dirPath, "file"), []byte("Hello"), 0644))
+	}
+
+	server, socketPath := runStorageServer(t)
+	defer server.Stop()
+
+	client, conn := newStorageClient(t, socketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testCases := []struct {
+		depth uint32
+		dirs  []string
+	}{
+		{
+			depth: 0,
+			dirs:  []string{"bar", "foo"},
+		},
+		{
+			depth: 1,
+			dirs:  []string{"bar", "bar/baz", "foo"},
+		},
+		{
+			depth: 3,
+			dirs:  []string{"bar", "bar/baz", "bar/baz/foo", "bar/baz/foo/buz", "foo"},
+		},
+	}
+
+	for _, tc := range testCases {
+		stream, err := client.ListDirectories(ctx, &pb.ListDirectoriesRequest{StorageName: "default", Depth: tc.depth})
+
+		var dirs []string
+		for {
+			resp, err := stream.Recv()
+			if err == io.EOF {
+				break
+			}
+
+			require.NoError(t, err)
+			dirs = append(dirs, resp.GetPaths()...)
+		}
+
+		require.NoError(t, err)
+		require.NotEmpty(t, dirs)
+		assert.Equal(t, tc.dirs, dirs)
+
+		for _, dir := range dirs {
+			assert.False(t, strings.HasSuffix(dir, "file"))
+		}
+	}
+}
diff --git a/internal/service/storage/server.go b/internal/service/storage/server.go
index 80a068890..76d051a6a 100644
--- a/internal/service/storage/server.go
+++ b/internal/service/storage/server.go
@@ -2,7 +2,6 @@ package storage
 
 import (
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 type server struct{}
@@ -11,7 +10,3 @@ type server struct{}
 func NewServer() pb.StorageServiceServer {
 	return &server{}
 }
-
-func (*server) ListDirectories(*pb.ListDirectoriesRequest, pb.StorageService_ListDirectoriesServer) error {
-	return helper.Unimplemented
-}