diff --git a/changelogs/unreleased/44564-error-500-while-attempting-to-resolve-conflicts-due-to-utf-8-conversion-error.yml b/changelogs/unreleased/44564-error-500-while-attempting-to-resolve-conflicts-due-to-utf-8-conversion-error.yml new file mode 100644 index 00000000000..3fb96153b9c --- /dev/null +++ b/changelogs/unreleased/44564-error-500-while-attempting-to-resolve-conflicts-due-to-utf-8-conversion-error.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 error when trying to resolve non-ASCII conflicts in the editor +merge_request: 17962 +author: +type: fixed diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb index 3ccfd9a739d..65a65b67975 100644 --- a/lib/gitlab/conflict/file_collection.rb +++ b/lib/gitlab/conflict/file_collection.rb @@ -40,7 +40,10 @@ module Gitlab # when there are no conflict files. files.each(&:lines) files.any? - rescue Gitlab::Git::CommandError, Gitlab::Git::Conflict::Parser::UnresolvableError, Gitlab::Git::Conflict::Resolver::ConflictSideMissing + rescue Gitlab::Git::CommandError, + Gitlab::Git::Conflict::Parser::UnresolvableError, + Gitlab::Git::Conflict::Resolver::ConflictSideMissing, + Gitlab::Git::Conflict::File::UnsupportedEncoding false end cache_method :can_be_resolved_in_ui? diff --git a/lib/gitlab/git/conflict/file.rb b/lib/gitlab/git/conflict/file.rb index 2a9cf10a068..f08dab59ce4 100644 --- a/lib/gitlab/git/conflict/file.rb +++ b/lib/gitlab/git/conflict/file.rb @@ -2,17 +2,19 @@ module Gitlab module Git module Conflict class File + UnsupportedEncoding = Class.new(StandardError) + attr_reader :their_path, :our_path, :our_mode, :repository, :commit_oid - attr_accessor :content + attr_accessor :raw_content - def initialize(repository, commit_oid, conflict, content) + def initialize(repository, commit_oid, conflict, raw_content) @repository = repository @commit_oid = commit_oid @their_path = conflict[:theirs][:path] @our_path = conflict[:ours][:path] @our_mode = conflict[:ours][:mode] - @content = content + @raw_content = raw_content end def lines @@ -29,6 +31,14 @@ module Gitlab end end + def content + @content ||= @raw_content.dup.force_encoding('UTF-8') + + raise UnsupportedEncoding unless @content.valid_encoding? + + @content + end + def type lines unless @type diff --git a/lib/gitlab/git/conflict/parser.rb b/lib/gitlab/git/conflict/parser.rb index 3effa9d2d31..fb5717dd556 100644 --- a/lib/gitlab/git/conflict/parser.rb +++ b/lib/gitlab/git/conflict/parser.rb @@ -4,7 +4,6 @@ module Gitlab class Parser UnresolvableError = Class.new(StandardError) UnmergeableFile = Class.new(UnresolvableError) - UnsupportedEncoding = Class.new(UnresolvableError) # Recoverable errors - the conflict can be resolved in an editor, but not with # sections. @@ -75,10 +74,6 @@ module Gitlab def validate_text!(text) raise UnmergeableFile if text.blank? # Typically a binary file raise UnmergeableFile if text.length > 200.kilobytes - - text.force_encoding('UTF-8') - - raise UnsupportedEncoding unless text.valid_encoding? end def validate_delimiter!(condition) diff --git a/lib/gitlab/gitaly_client/conflict_files_stitcher.rb b/lib/gitlab/gitaly_client/conflict_files_stitcher.rb index 97c13d1fdb0..c275a065bce 100644 --- a/lib/gitlab/gitaly_client/conflict_files_stitcher.rb +++ b/lib/gitlab/gitaly_client/conflict_files_stitcher.rb @@ -17,7 +17,7 @@ module Gitlab current_file = file_from_gitaly_header(gitaly_file.header) else - current_file.content << gitaly_file.content + current_file.raw_content << gitaly_file.content end end end diff --git a/spec/lib/gitlab/git/conflict/file_spec.rb b/spec/lib/gitlab/git/conflict/file_spec.rb new file mode 100644 index 00000000000..afed6c32af6 --- /dev/null +++ b/spec/lib/gitlab/git/conflict/file_spec.rb @@ -0,0 +1,50 @@ +# coding: utf-8 +require 'spec_helper' + +describe Gitlab::Git::Conflict::File do + let(:conflict) { { theirs: { path: 'foo', mode: 33188 }, ours: { path: 'foo', mode: 33188 } } } + let(:invalid_content) { described_class.new(nil, nil, conflict, "a\xC4\xFC".force_encoding(Encoding::ASCII_8BIT)) } + let(:valid_content) { described_class.new(nil, nil, conflict, "Espa\xC3\xB1a".force_encoding(Encoding::ASCII_8BIT)) } + + describe '#lines' do + context 'when the content contains non-UTF-8 characters' do + it 'raises UnsupportedEncoding' do + expect { invalid_content.lines } + .to raise_error(described_class::UnsupportedEncoding) + end + end + + context 'when the content can be converted to UTF-8' do + it 'sets lines to the lines' do + expect(valid_content.lines).to eq([{ + full_line: 'España', + type: nil, + line_obj_index: 0, + line_old: 1, + line_new: 1 + }]) + end + + it 'sets the type to text' do + expect(valid_content.type).to eq('text') + end + end + end + + describe '#content' do + context 'when the content contains non-UTF-8 characters' do + it 'raises UnsupportedEncoding' do + expect { invalid_content.content } + .to raise_error(described_class::UnsupportedEncoding) + end + end + + context 'when the content can be converted to UTF-8' do + it 'returns a valid UTF-8 string' do + expect(valid_content.content).to eq('España') + expect(valid_content.content).to be_valid_encoding + expect(valid_content.content.encoding).to eq(Encoding::UTF_8) + end + end + end +end diff --git a/spec/lib/gitlab/git/conflict/parser_spec.rb b/spec/lib/gitlab/git/conflict/parser_spec.rb index 7b035a381f1..29a1702a1c6 100644 --- a/spec/lib/gitlab/git/conflict/parser_spec.rb +++ b/spec/lib/gitlab/git/conflict/parser_spec.rb @@ -212,13 +212,6 @@ CONFLICT .not_to raise_error end end - - context 'when the file contains non-UTF-8 characters' do - it 'raises UnsupportedEncoding' do - expect { parse_text("a\xC4\xFC".force_encoding(Encoding::ASCII_8BIT)) } - .to raise_error(Gitlab::Git::Conflict::Parser::UnsupportedEncoding) - end - end end end end