Add maximum depth for FlatPath

- limit recursion to deep 10

Closes: https://gitlab.com/gitlab-org/gitaly/issues/1931

3 files changed, 67 insertions, 15 deletions

diff --git a/internal/service/commit/tree_entries.go b/internal/service/commit/tree_entries.go
index 7e7fc23ee..d0c36f34d 100644
--- a/internal/service/commit/tree_entries.go
+++ b/internal/service/commit/tree_entries.go
@@ -13,8 +13,6 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-var maxTreeEntries = 1000
-
 func validateGetTreeEntriesRequest(in *gitalypb.GetTreeEntriesRequest) error {
 	if err := git.ValidateRevision(in.Revision); err != nil {
 		return err
@@ -35,17 +33,18 @@ func populateFlatPath(c *catfile.Batch, entries []*gitalypb.TreeEntry) error {
 			continue
 		}
 
-		for {
-			subentries, err := treeEntries(c, entry.CommitOid, string(entry.FlatPath), "", false)
+		for i := 1; i < defaultFlatTreeRecursion; i++ {
+			subEntries, err := treeEntries(c, entry.CommitOid, string(entry.FlatPath), "", false)
 
 			if err != nil {
 				return err
 			}
-			if len(subentries) != 1 || subentries[0].Type != gitalypb.TreeEntry_TREE {
+
+			if len(subEntries) != 1 || subEntries[0].Type != gitalypb.TreeEntry_TREE {
 				break
 			}
 
-			entry.FlatPath = subentries[0].Path
+			entry.FlatPath = subEntries[0].Path
 		}
 	}
 
@@ -66,7 +65,9 @@ func sendTreeEntries(stream gitalypb.CommitService_GetTreeEntriesServer, c *catf
 
 	sender := chunk.New(&treeEntriesSender{stream: stream})
 	for _, e := range entries {
-		sender.Send(e)
+		if err := sender.Send(e); err != nil {
+			return err
+		}
 	}
 
 	return sender.Flush()
diff --git a/internal/service/commit/tree_entries_helper.go b/internal/service/commit/tree_entries_helper.go
index e67055f23..5c9a538ed 100644
--- a/internal/service/commit/tree_entries_helper.go
+++ b/internal/service/commit/tree_entries_helper.go
@@ -53,7 +53,10 @@ func (tef *TreeEntryFinder) FindByRevisionAndPath(revision, path string) (*gital
 	return nil, nil
 }
 
-const oidSize = 20
+const (
+	oidSize                  = 20
+	defaultFlatTreeRecursion = 10
+)
 
 func extractEntryInfoFromTreeData(treeData *bytes.Buffer, commitOid, rootOid, rootPath string, treeInfo *catfile.ObjectInfo) ([]*gitalypb.TreeEntry, error) {
 	if len(treeInfo.Oid) == 0 {
diff --git a/internal/service/commit/tree_entries_test.go b/internal/service/commit/tree_entries_test.go
index 0cca60015..6c7af65e7 100644
--- a/internal/service/commit/tree_entries_test.go
+++ b/internal/service/commit/tree_entries_test.go
@@ -6,9 +6,11 @@ import (
 	"io"
 	"os"
 	"path"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -81,13 +83,6 @@ func TestSuccessfulGetTreeEntriesWithCurlyBraces(t *testing.T) {
 }
 
 func TestSuccessfulGetTreeEntries(t *testing.T) {
-	//  Force entries to be sliced to test that behaviour
-	oldMaxTreeEntries := maxTreeEntries
-	maxTreeEntries = 1
-	defer func() {
-		maxTreeEntries = oldMaxTreeEntries
-	}()
-
 	commitID := "d25b6d94034242f3930dfcfeb6d8d9aac3583992"
 	rootOid := "21bdc8af908562ae485ed46d71dd5426c08b084a"
 
@@ -414,6 +409,59 @@ func getTreeEntriesFromTreeEntryClient(t *testing.T, client gitalypb.CommitServi
 	return entries
 }
 
+func TestSuccessfulGetTreeEntries_FlatPathMaxDeep_SingleFoldersStructure(t *testing.T) {
+	server, serverSocketPath := startTestServices(t)
+	defer server.Stop()
+
+	client, conn := newCommitServiceClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepoWithWorktree(t)
+	defer cleanupFn()
+
+	folderName := "1/2/3/4/5/6/7/8/9/10/11/12"
+	require.GreaterOrEqual(t, strings.Count(strings.Trim(folderName, "/"), "/"), defaultFlatTreeRecursion, "sanity check: construct folder deeper than default recursion value")
+
+	nestedFolder := path.Join(testRepoPath, folderName)
+	require.NoError(t, os.MkdirAll(nestedFolder, 0755))
+	// put single file into the deepest directory
+	testhelper.MustRunCommand(t, nil, "touch", path.Join(nestedFolder, ".gitkeep"))
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "add", "--all")
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "commit", "-m", "Deep folder struct")
+
+	commitID := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "HEAD"))
+	rootOid := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "HEAD^{tree}"))
+
+	// make request to folder that contains nothing except one folder
+	request := &gitalypb.GetTreeEntriesRequest{
+		Repository: testRepo,
+		Revision:   []byte(commitID),
+		Path:       []byte("1"),
+		Recursive:  false,
+	}
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	// request entries of the tree with single-folder structure on each level
+	entriesClient, err := client.GetTreeEntries(ctx, request)
+	require.NoError(t, err)
+
+	fetchedEntries := getTreeEntriesFromTreeEntryClient(t, entriesClient)
+	// We know that there is a directory "1/2/3/4/5/6/7/8/9/10/11/12"
+	// but here we only get back "1/2/3/4/5/6/7/8/9/10/11".
+	// This proves that FlatPath recursion is bounded, which is the point of this test.
+	require.Equal(t, []*gitalypb.TreeEntry{{
+		Oid:       "c836b95b37958e7179f5a42a32b7197b5dec7321",
+		RootOid:   rootOid,
+		Path:      []byte("1/2"),
+		FlatPath:  []byte("1/2/3/4/5/6/7/8/9/10/11"),
+		Type:      gitalypb.TreeEntry_TREE,
+		Mode:      040000,
+		CommitOid: commitID,
+	}}, fetchedEntries)
+}
+
 func TestFailedGetTreeEntriesRequestDueToValidationError(t *testing.T) {
 	server, serverSocketPath := startTestServices(t)
 	defer server.Stop()