Fix Repository SearchFilesByName not handle non-ASCII file names well

Repository SearchFilesByName doesn't work well with non-ASCII file
names. Underlying, this RPC uses git-ls-tree to get the file names. Git
automatically escapes non-ASCII characters and add the quotes around the
paths. Git defines this behavior with core.quotePath option.
SearchFilesByName did not handle this case well. The solution is to
force git-ls-tree to use NULL-byte termination.

Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/4440
Changelog: fixed

2 files changed, 104 insertions, 1 deletions

diff --git a/internal/gitaly/service/repository/search_files.go b/internal/gitaly/service/repository/search_files.go
index 4c6a006bb..ba7598888 100644
--- a/internal/gitaly/service/repository/search_files.go
+++ b/internal/gitaly/service/repository/search_files.go
@@ -131,6 +131,11 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea
 			git.Flag{Name: "--full-tree"},
 			git.Flag{Name: "--name-status"},
 			git.Flag{Name: "-r"},
+			// We use -z to force NULL byte termination here to prevent git from
+			// quoting and escaping unusual file names. Lstree parser would be a
+			// more ideal solution. Unfortunately, it supports parsing full
+			// output while we are interested in the filenames only.
+			git.Flag{Name: "-z"},
 		}, Args: []string{string(req.GetRef()), req.GetQuery()}})
 	if err != nil {
 		return helper.ErrInternalf("SearchFilesByName: cmd start failed: %v", err)
@@ -140,7 +145,7 @@ func (s *server) SearchFilesByName(req *gitalypb.SearchFilesByNameRequest, strea
 		return stream.Send(&gitalypb.SearchFilesByNameResponse{Files: objs})
 	}
 
-	return lines.Send(cmd, lr, lines.SenderOpts{Delimiter: '\n', Limit: math.MaxInt32, Filter: filter})
+	return lines.Send(cmd, lr, lines.SenderOpts{Delimiter: 0x00, Limit: math.MaxInt32, Filter: filter})
 }
 
 type searchFilesRequest interface {
diff --git a/internal/gitaly/service/repository/search_files_test.go b/internal/gitaly/service/repository/search_files_test.go
index ff4f20f13..092ef976a 100644
--- a/internal/gitaly/service/repository/search_files_test.go
+++ b/internal/gitaly/service/repository/search_files_test.go
@@ -337,6 +337,104 @@ func TestSearchFilesByNameSuccessful(t *testing.T) {
 	}
 }
 
+func TestSearchFilesByNameUnusualFileNamesSuccessful(t *testing.T) {
+	t.Parallel()
+	ctx := testhelper.Context(t)
+
+	cfg, repo, repoPath, client := setupRepositoryService(ctx, t)
+
+	ref := []byte("unusual_file_names")
+	gittest.WriteCommit(t, cfg, repoPath,
+		gittest.WithBranch(string(ref)),
+		gittest.WithMessage("commit message"),
+		gittest.WithTreeEntries(
+			gittest.TreeEntry{Path: "\"file with quote.txt", Mode: "100644", Content: "something"},
+			gittest.TreeEntry{Path: ".vimrc", Mode: "100644", Content: "something"},
+			gittest.TreeEntry{Path: "cuộc đời là những chuyến đi.md", Mode: "100644", Content: "something"},
+			gittest.TreeEntry{Path: "编码 'foo'.md", Mode: "100644", Content: "something"},
+		),
+	)
+
+	testCases := []struct {
+		desc          string
+		query         string
+		filter        string
+		expectedFiles [][]byte
+	}{
+		{
+			desc:          "file with quote",
+			query:         "\"file with quote.txt",
+			expectedFiles: [][]byte{[]byte("\"file with quote.txt")},
+		},
+		{
+			desc:          "dotfiles",
+			query:         ".vimrc",
+			expectedFiles: [][]byte{[]byte(".vimrc")},
+		},
+		{
+			desc:          "latin-base language",
+			query:         "cuộc đời là những chuyến đi.md",
+			expectedFiles: [][]byte{[]byte("cuộc đời là những chuyến đi.md")},
+		},
+		{
+			desc:          "non-latin language",
+			query:         "编码 'foo'.md",
+			expectedFiles: [][]byte{[]byte("编码 'foo'.md")},
+		},
+		{
+			desc:          "filter file with quote",
+			query:         ".",
+			filter:        "^\"file.*",
+			expectedFiles: [][]byte{[]byte("\"file with quote.txt")},
+		},
+		{
+			desc:          "filter dotfiles",
+			query:         ".",
+			filter:        "^\\..*",
+			expectedFiles: [][]byte{[]byte(".vimrc")},
+		},
+		{
+			desc:          "filter latin-base language",
+			query:         ".",
+			filter:        "cuộc đời .*\\.md",
+			expectedFiles: [][]byte{[]byte("cuộc đời là những chuyến đi.md")},
+		},
+		{
+			desc:          "filter non-latin language",
+			query:         ".",
+			filter:        "编码 'foo'\\.(md|txt|rdoc)",
+			expectedFiles: [][]byte{[]byte("编码 'foo'.md")},
+		},
+		{
+			desc:   "wildcard filter",
+			query:  ".",
+			filter: ".*",
+			expectedFiles: [][]byte{
+				[]byte("\"file with quote.txt"),
+				[]byte(".vimrc"),
+				[]byte("cuộc đời là những chuyến đi.md"),
+				[]byte("编码 'foo'.md"),
+			},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			stream, err := client.SearchFilesByName(ctx, &gitalypb.SearchFilesByNameRequest{
+				Repository: repo,
+				Ref:        ref,
+				Query:      tc.query,
+				Filter:     tc.filter,
+			})
+			require.NoError(t, err)
+
+			var files [][]byte
+			files, err = consumeFilenameByName(stream)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedFiles, files)
+		})
+	}
+}
+
 func TestSearchFilesByNameFailure(t *testing.T) {
 	t.Parallel()
 	cfg := testcfg.Build(t)