Fix encoding bug in UserCommitFiles

3 files changed, 59 insertions, 16 deletions

diff --git a/changelogs/unreleased/user-commit-files-encoding.yml b/changelogs/unreleased/user-commit-files-encoding.yml
new file mode 100644
index 000000000..0aa2c4a60
--- /dev/null
+++ b/changelogs/unreleased/user-commit-files-encoding.yml
@@ -0,0 +1,5 @@
+---
+title: Fix encoding bug in UserCommitFiles
+merge_request: 782
+author:
+type: fixed
diff --git a/internal/service/operations/commit_files_test.go b/internal/service/operations/commit_files_test.go
index 2928a8126..67a19270a 100644
--- a/internal/service/operations/commit_files_test.go
+++ b/internal/service/operations/commit_files_test.go
@@ -42,7 +42,7 @@ func TestSuccessfulUserCommitFilesRequest(t *testing.T) {
 	defer newRepoCleanupFn()
 
 	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
-	filePath := "my/file.txt"
+	filePath := "héllo/wörld"
 	authorName := []byte("Jane Doe")
 	authorEmail := []byte("janedoe@gitlab.com")
 	testCases := []struct {
@@ -171,19 +171,53 @@ func TestFailedUserCommitFilesRequestDueToIndexError(t *testing.T) {
 
 	md := testhelper.GitalyServersMetadata(t, serverSocketPath)
 	ctx := metadata.NewOutgoingContext(ctxOuter, md)
-	headerRequest := headerRequest(testRepo, user, "feature", commitFilesMessage, nil, nil)
-	actionsRequest1 := createFileHeaderRequest("README.md")
-	actionsRequest2 := actionContentRequest("This file already exists")
-
-	stream, err := client.UserCommitFiles(ctx)
-	require.NoError(t, err)
-	require.NoError(t, stream.Send(headerRequest))
-	require.NoError(t, stream.Send(actionsRequest1))
-	require.NoError(t, stream.Send(actionsRequest2))
-
-	r, err := stream.CloseAndRecv()
-	require.NoError(t, err)
-	require.Equal(t, r.GetIndexError(), "A file with this name already exists")
+	testCases := []struct {
+		desc       string
+		requests   []*pb.UserCommitFilesRequest
+		indexError string
+	}{
+		{
+			desc: "file already exists",
+			requests: []*pb.UserCommitFilesRequest{
+				headerRequest(testRepo, user, "feature", commitFilesMessage, nil, nil),
+				createFileHeaderRequest("README.md"),
+				actionContentRequest("This file already exists"),
+			},
+			indexError: "A file with this name already exists",
+		},
+		{
+			desc: "dir already exists",
+			requests: []*pb.UserCommitFilesRequest{
+				headerRequest(testRepo, user, "utf-dir", commitFilesMessage, nil, nil),
+				actionRequest(&pb.UserCommitFilesAction{
+					UserCommitFilesActionPayload: &pb.UserCommitFilesAction_Header{
+						Header: &pb.UserCommitFilesActionHeader{
+							Action:        pb.UserCommitFilesActionHeader_CREATE_DIR,
+							Base64Content: false,
+							FilePath:      []byte("héllo"),
+						},
+					},
+				}),
+				actionContentRequest("This file already exists, as a directory"),
+			},
+			indexError: "A directory with this name already exists",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			stream, err := client.UserCommitFiles(ctx)
+			require.NoError(t, err)
+
+			for _, req := range tc.requests {
+				require.NoError(t, stream.Send(req))
+			}
+
+			r, err := stream.CloseAndRecv()
+			require.NoError(t, err)
+			require.Equal(t, tc.indexError, r.GetIndexError())
+		})
+	}
 }
 
 func TestFailedUserCommitFilesRequest(t *testing.T) {
diff --git a/ruby/lib/gitaly_server/operations_service.rb b/ruby/lib/gitaly_server/operations_service.rb
index 7abb11aca..5a56a8451 100644
--- a/ruby/lib/gitaly_server/operations_service.rb
+++ b/ruby/lib/gitaly_server/operations_service.rb
@@ -311,8 +311,12 @@ module GitalyServer
     def commit_files_action_from_gitaly_request(header)
       {
         action: header.action.downcase,
-        file_path: header.file_path,
-        previous_path: header.previous_path,
+        # Forcing the encoding to UTF-8 here is unusual. But these paths get
+        # compared with Rugged::Index entries, which are also force-encoded to
+        # UTF-8. See
+        # https://github.com/libgit2/rugged/blob/f8172c2a177a6795553f38f01248daff923f4264/ext/rugged/rugged_index.c#L514
+        file_path: set_utf8!(header.file_path),
+        previous_path: set_utf8!(header.previous_path),
         encoding: header.base64_content ? 'base64' : '',
         content: ''
       }