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 0000000000000000000000000000000000000000..84a5d2a93f5fb20863aefcf7e7333dc561b8832a
--- /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 53b17f77ccd8b1b2024db3fce0a6d04b57965266..d2f5af636217455415bacdb9e63a0e0bff98b35b 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 41f179d275b8610080352aea5e9f7f50d1fb0d08..ae9996d81ef0f7978e8a6a68c04b886e2667e072 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 f09e0bd980633dde44fa8f88b3eed5267fddd20c..3c17ea1195e434e722298a16babf0295149f24dc 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 fbd54c4d4da090f2e1d8be7ceb6f5c1def9c1263..e5f4dabe42defe128340fc84703f30911a1bf1f6 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 3bb57e152fe3da2312b2f3ba270818fca2214e24..ef0bb90db4ae13a9f90c53827c20a2304b72fac9 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 4995caa0733b4006631a4c7089a2f2fe183a3cae..22bf10f36d893723c9a7c9148cea0ae416cb2942 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