Merge branch 'fix/wiki-find-page-invalid-encoding' into 'master'

Fix WikiFindPage when the page has invalidly-encoded content

See merge request gitlab-org/gitaly!712

3 files changed, 50 insertions, 7 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index edf731a47..7795ad274 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- Fix WikiFindPage when the page has invalidly-encoded content
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/712
 - Add danger container to the Gitaly project
   https://gitlab.com/gitlab-org/gitaly/merge_requests/711
 - Remove ruby concurrency limiter
diff --git a/internal/service/wiki/find_page_test.go b/internal/service/wiki/find_page_test.go
index bf3e7bd54..44a0e0ac5 100644
--- a/internal/service/wiki/find_page_test.go
+++ b/internal/service/wiki/find_page_test.go
@@ -25,14 +25,18 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 	page1Name := "Home Pagé"
 	page2Name := "Instálling/Step 133-b"
 	page3Name := "Installing/Step 133-c"
+	page4Name := "Encoding is fun"
 	page1Commit := createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page1Name})
 	createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page2Name})
-	page3Commit := createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page3Name})
+	createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page3Name})
+	page4Commit := createTestWikiPage(t, client, wikiRepo, createWikiPageOpts{title: page4Name, content: []byte("f\xFCr")})
+	latestCommit := page4Commit
 
 	testCases := []struct {
-		desc         string
-		request      *pb.WikiFindPageRequest
-		expectedPage *pb.WikiPage
+		desc            string
+		request         *pb.WikiFindPageRequest
+		expectedPage    *pb.WikiPage
+		expectedContent []byte
 	}{
 		{
 			desc: "title only",
@@ -42,7 +46,7 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 			},
 			expectedPage: &pb.WikiPage{
 				Version: &pb.WikiPageVersion{
-					Commit: page3Commit,
+					Commit: latestCommit,
 					Format: "markdown",
 				},
 				Title:      []byte(page1Name),
@@ -52,6 +56,7 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 				Name:       []byte(page1Name),
 				Historical: false,
 			},
+			expectedContent: mockPageContent,
 		},
 		{
 			desc: "title + revision that includes the page",
@@ -72,6 +77,7 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 				Name:       []byte(page1Name),
 				Historical: true,
 			},
+			expectedContent: mockPageContent,
 		},
 		{
 			desc: "title + revision that does not include the page",
@@ -91,7 +97,7 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 			},
 			expectedPage: &pb.WikiPage{
 				Version: &pb.WikiPageVersion{
-					Commit: page3Commit,
+					Commit: latestCommit,
 					Format: "markdown",
 				},
 				Title:      []byte("Step 133 b"),
@@ -101,6 +107,7 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 				Name:       []byte("Step 133 b"),
 				Historical: false,
 			},
+			expectedContent: mockPageContent,
 		},
 		{
 			desc: "title + directory that does not include the page",
@@ -111,6 +118,26 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 			},
 			expectedPage: nil,
 		},
+		{
+			desc: "title for invalidly-encoded page",
+			request: &pb.WikiFindPageRequest{
+				Repository: wikiRepo,
+				Title:      []byte("Encoding is fun"),
+			},
+			expectedPage: &pb.WikiPage{
+				Version: &pb.WikiPageVersion{
+					Commit: latestCommit,
+					Format: "markdown",
+				},
+				Title:      []byte(page4Name),
+				Format:     "markdown",
+				UrlPath:    "Encoding-is-fun",
+				Path:       []byte("Encoding-is-fun.md"),
+				Name:       []byte(page4Name),
+				Historical: false,
+			},
+			expectedContent: []byte("fr"),
+		},
 	}
 
 	for _, testCase := range testCases {
@@ -133,7 +160,7 @@ func TestSuccessfulWikiFindPageRequest(t *testing.T) {
 
 			require.Equal(t, expectedPage, receivedPage, "mismatched page attributes")
 			if expectedPage != nil {
-				require.Equal(t, mockPageContent, receivedContent, "mismatched page content")
+				require.Equal(t, testCase.expectedContent, receivedContent, "mismatched page content")
 			}
 		})
 	}
diff --git a/ruby/lib/gitlab/gollum.rb b/ruby/lib/gitlab/gollum.rb
index 240af7040..2eb5d78aa 100644
--- a/ruby/lib/gitlab/gollum.rb
+++ b/ruby/lib/gitlab/gollum.rb
@@ -4,3 +4,17 @@ end
 require "gollum-lib"
 
 Gollum::Page.per_page = 20 # Magic number from Kaminari.config.default_per_page
+
+module Gollum
+  class Page
+    def text_data(encoding=nil)
+      data = if raw_data.respond_to?(:encoding)
+               raw_data.force_encoding(encoding || Encoding::UTF_8)
+             else
+               raw_data
+             end
+
+      Gitlab::EncodingHelper.encode!(data)
+    end
+  end
+end