Commit 44eb3197 authored by Sean McGivern's avatar Sean McGivern

Handle non-UTF-8 conflicts gracefully

These can't be resolved in the UI because if they aren't in a UTF-8
compatible encoding, they can't be rendered as JSON. Even if they could,
we would be implicitly changing the file encoding anyway, which seems
like a bad idea.
parent b2bf01f4
...@@ -10,6 +10,9 @@ v 8.12.0 (unreleased) ...@@ -10,6 +10,9 @@ v 8.12.0 (unreleased)
- Added tests for diff notes - Added tests for diff notes
- Added 'only_allow_merge_if_build_succeeds' project setting in the API. !5930 (Duck) - Added 'only_allow_merge_if_build_succeeds' project setting in the API. !5930 (Duck)
v 8.11.3 (unreleased)
- Don't try to show merge conflict resolution info if a merge conflict contains non-UTF-8 characters
v 8.11.2 (unreleased) v 8.11.2 (unreleased)
- Show "Create Merge Request" widget for push events to fork projects on the source project - Show "Create Merge Request" widget for push events to fork projects on the source project
......
...@@ -26,6 +26,7 @@ this is similar to performing `git checkout feature; git merge master` locally. ...@@ -26,6 +26,7 @@ this is similar to performing `git checkout feature; git merge master` locally.
GitLab allows resolving conflicts in a file where all of the below are true: GitLab allows resolving conflicts in a file where all of the below are true:
- The file is text, not binary - The file is text, not binary
- The file is in a UTF-8 compatible encoding
- The file does not already contain conflict markers - The file does not already contain conflict markers
- The file, with conflict markers added, is not over 200 KB in size - The file, with conflict markers added, is not over 200 KB in size
- The file exists under the same path in both branches - The file exists under the same path in both branches
......
...@@ -13,10 +13,19 @@ module Gitlab ...@@ -13,10 +13,19 @@ module Gitlab
class UnmergeableFile < ParserError class UnmergeableFile < ParserError
end end
class UnsupportedEncoding < ParserError
end
def parse(text, our_path:, their_path:, parent_file: nil) def parse(text, our_path:, their_path:, parent_file: nil)
raise UnmergeableFile if text.blank? # Typically a binary file raise UnmergeableFile if text.blank? # Typically a binary file
raise UnmergeableFile if text.length > 102400 raise UnmergeableFile if text.length > 102400
begin
text.to_json
rescue Encoding::UndefinedConversionError
raise UnsupportedEncoding
end
line_obj_index = 0 line_obj_index = 0
line_old = 1 line_old = 1
line_new = 1 line_new = 1
......
...@@ -43,7 +43,8 @@ feature 'Merge request conflict resolution', js: true, feature: true do ...@@ -43,7 +43,8 @@ feature 'Merge request conflict resolution', js: true, feature: true do
'conflict-too-large' => 'when the conflicts contain a large file', 'conflict-too-large' => 'when the conflicts contain a large file',
'conflict-binary-file' => 'when the conflicts contain a binary file', 'conflict-binary-file' => 'when the conflicts contain a binary file',
'conflict-contains-conflict-markers' => 'when the conflicts contain a file with ambiguous conflict markers', 'conflict-contains-conflict-markers' => 'when the conflicts contain a file with ambiguous conflict markers',
'conflict-missing-side' => 'when the conflicts contain a file edited in one branch and deleted in another' 'conflict-missing-side' => 'when the conflicts contain a file edited in one branch and deleted in another',
'conflict-non-utf8' => 'when the conflicts contain a non-UTF-8 file',
} }
UNRESOLVABLE_CONFLICTS.each do |source_branch, description| UNRESOLVABLE_CONFLICTS.each do |source_branch, description|
......
...@@ -183,6 +183,11 @@ CONFLICT ...@@ -183,6 +183,11 @@ CONFLICT
expect { parse_text('a' * 102401) }. expect { parse_text('a' * 102401) }.
to raise_error(Gitlab::Conflict::Parser::UnmergeableFile) to raise_error(Gitlab::Conflict::Parser::UnmergeableFile)
end end
it 'raises UnsupportedEncoding when the file contains non-UTF-8 characters' do
expect { parse_text("a\xC4\xFC".force_encoding(Encoding::ASCII_8BIT)) }.
to raise_error(Gitlab::Conflict::Parser::UnsupportedEncoding)
end
end end
end end
end end
...@@ -24,11 +24,12 @@ module TestEnv ...@@ -24,11 +24,12 @@ module TestEnv
'expand-collapse-lines' => '238e82d', 'expand-collapse-lines' => '238e82d',
'video' => '8879059', 'video' => '8879059',
'crlf-diff' => '5938907', 'crlf-diff' => '5938907',
'conflict-start' => '14fa46b', 'conflict-start' => '75284c7',
'conflict-resolvable' => '1450cd6', 'conflict-resolvable' => '1450cd6',
'conflict-binary-file' => '259a6fb', 'conflict-binary-file' => '259a6fb',
'conflict-contains-conflict-markers' => '5e0964c', 'conflict-contains-conflict-markers' => '5e0964c',
'conflict-missing-side' => 'eb227b3', 'conflict-missing-side' => 'eb227b3',
'conflict-non-utf8' => 'd0a293c',
'conflict-too-large' => '39fa04f', 'conflict-too-large' => '39fa04f',
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment