From 869dc8d2f30ae395a29753d1766aad87b5a15301 Mon Sep 17 00:00:00 2001
From: Kassio Borges <kborges@gitlab.com>
Date: Tue, 11 May 2021 14:19:58 +0100
Subject: [PATCH] GithubImporter: Fix "ArgumentError: string contains null
byte"
Changelog: fixed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61480
---
...hub-importer-strings-with-invalid-char.yml | 5 +++++
.../importer/diff_note_importer.rb | 3 +--
.../github_import/importer/note_importer.rb | 3 +--
.../importer/pull_request_importer.rb | 3 +--
lib/gitlab/github_import/markdown_text.rb | 11 +++++++++-
.../importer/note_importer_spec.rb | 21 ++++++++++++++++++-
.../github_import/markdown_text_spec.rb | 14 +++++++++++++
7 files changed, 52 insertions(+), 8 deletions(-)
create mode 100644 changelogs/unreleased/kassio-github-importer-strings-with-invalid-char.yml
diff --git a/changelogs/unreleased/kassio-github-importer-strings-with-invalid-char.yml b/changelogs/unreleased/kassio-github-importer-strings-with-invalid-char.yml
new file mode 100644
index 0000000000000..84a5d2a93f5fb
--- /dev/null
+++ b/changelogs/unreleased/kassio-github-importer-strings-with-invalid-char.yml
@@ -0,0 +1,5 @@
+---
+title: 'GithubImporter: Fix "ArgumentError: string contains null byte"'
+merge_request: 61480
+author:
+type: fixed
diff --git a/lib/gitlab/github_import/importer/diff_note_importer.rb b/lib/gitlab/github_import/importer/diff_note_importer.rb
index 53b17f77ccd8b..d2f5af6362174 100644
--- a/lib/gitlab/github_import/importer/diff_note_importer.rb
+++ b/lib/gitlab/github_import/importer/diff_note_importer.rb
@@ -21,8 +21,7 @@ def execute
author_id, author_found = user_finder.author_id_for(note)
- note_body =
- MarkdownText.format(note.note, note.author, author_found)
+ note_body = MarkdownText.format(note.note, note.author, author_found)
attributes = {
noteable_type: 'MergeRequest',
diff --git a/lib/gitlab/github_import/importer/note_importer.rb b/lib/gitlab/github_import/importer/note_importer.rb
index 41f179d275b86..ae9996d81ef0f 100644
--- a/lib/gitlab/github_import/importer/note_importer.rb
+++ b/lib/gitlab/github_import/importer/note_importer.rb
@@ -21,8 +21,7 @@ def execute
author_id, author_found = user_finder.author_id_for(note)
- note_body =
- MarkdownText.format(note.note, note.author, author_found)
+ note_body = MarkdownText.format(note.note, note.author, author_found)
attributes = {
noteable_type: note.noteable_type,
diff --git a/lib/gitlab/github_import/importer/pull_request_importer.rb b/lib/gitlab/github_import/importer/pull_request_importer.rb
index f09e0bd980633..3c17ea1195e43 100644
--- a/lib/gitlab/github_import/importer/pull_request_importer.rb
+++ b/lib/gitlab/github_import/importer/pull_request_importer.rb
@@ -44,8 +44,7 @@ def execute
def create_merge_request
author_id, author_found = user_finder.author_id_for(pull_request)
- description = MarkdownText
- .format(pull_request.description, pull_request.author, author_found)
+ description = MarkdownText.format(pull_request.description, pull_request.author, author_found)
attributes = {
iid: pull_request.iid,
diff --git a/lib/gitlab/github_import/markdown_text.rb b/lib/gitlab/github_import/markdown_text.rb
index fbd54c4d4da09..e5f4dabe42def 100644
--- a/lib/gitlab/github_import/markdown_text.rb
+++ b/lib/gitlab/github_import/markdown_text.rb
@@ -3,7 +3,7 @@
module Gitlab
module GithubImport
class MarkdownText
- attr_reader :text, :author, :exists
+ include Gitlab::EncodingHelper
def self.format(*args)
new(*args).to_s
@@ -19,6 +19,15 @@ def initialize(text, author, exists = false)
end
def to_s
+ # Gitlab::EncodingHelper#clean remove `null` chars from the string
+ clean(format)
+ end
+
+ private
+
+ attr_reader :text, :author, :exists
+
+ def format
if author&.login.present? && !exists
"*Created by: #{author.login}*\n\n#{text}"
else
diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
index 3bb57e152fe3d..ef0bb90db4ae1 100644
--- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb
@@ -8,13 +8,14 @@
let(:user) { create(:user) }
let(:created_at) { Time.new(2017, 1, 1, 12, 00) }
let(:updated_at) { Time.new(2017, 1, 1, 12, 15) }
+ let(:note_body) { 'This is my note' }
let(:github_note) do
Gitlab::GithubImport::Representation::Note.new(
noteable_id: 1,
noteable_type: 'Issue',
author: Gitlab::GithubImport::Representation::User.new(id: 4, login: 'alice'),
- note: 'This is my note',
+ note: note_body,
created_at: created_at,
updated_at: updated_at,
github_id: 1
@@ -92,6 +93,24 @@
importer.execute
end
end
+
+ context 'when the note have invalid chars' do
+ let(:note_body) { %{There were an invalid char "\u0000" <= right here} }
+
+ it 'removes invalid chars' do
+ expect(importer.user_finder)
+ .to receive(:author_id_for)
+ .with(github_note)
+ .and_return([user.id, true])
+
+ expect { importer.execute }
+ .to change(project.notes, :count)
+ .by(1)
+
+ expect(project.notes.last.note)
+ .to eq('There were an invalid char "" <= right here')
+ end
+ end
end
context 'when the noteable does not exist' do
diff --git a/spec/lib/gitlab/github_import/markdown_text_spec.rb b/spec/lib/gitlab/github_import/markdown_text_spec.rb
index 4995caa0733b4..22bf10f36d893 100644
--- a/spec/lib/gitlab/github_import/markdown_text_spec.rb
+++ b/spec/lib/gitlab/github_import/markdown_text_spec.rb
@@ -20,11 +20,25 @@
expect(text.to_s).to eq('Hello')
end
+ it 'returns the text when the author has no login' do
+ author = double(:author, login: nil)
+ text = described_class.new('Hello', author, true)
+
+ expect(text.to_s).to eq('Hello')
+ end
+
it 'returns the text with an extra header when the author was not found' do
author = double(:author, login: 'Alice')
text = described_class.new('Hello', author)
expect(text.to_s).to eq("*Created by: Alice*\n\nHello")
end
+
+ it 'cleans invalid chars' do
+ author = double(:author, login: 'Alice')
+ text = described_class.format("\u0000Hello", author)
+
+ expect(text.to_s).to eq("*Created by: Alice*\n\nHello")
+ end
end
end
--
GitLab