diff options
-rw-r--r-- | app/models/abuse_report.rb | 2 | ||||
-rw-r--r-- | app/services/users/destroy_service.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/fix-trace-encoding.yml | 4 | ||||
-rw-r--r-- | lib/gitlab/ci/trace/stream.rb | 11 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/trace/stream_spec.rb | 20 | ||||
-rw-r--r-- | spec/models/abuse_report_spec.rb | 3 | ||||
-rw-r--r-- | spec/services/users/destroy_service_spec.rb | 6 |
7 files changed, 41 insertions, 7 deletions
diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index 2340453831e..0d7c2d20029 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -16,7 +16,7 @@ class AbuseReport < ActiveRecord::Base def remove_user(deleted_by:) user.block - DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true) + DeleteUserWorker.perform_async(deleted_by.id, user.id, delete_solo_owned_groups: true, hard_delete: true) end def notify diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index ba58b174cc0..9eb6a600f6b 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -26,7 +26,7 @@ module Users ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute end - MigrateToGhostUserService.new(user).execute + MigrateToGhostUserService.new(user).execute unless options[:hard_delete] # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing namespace = user.namespace diff --git a/changelogs/unreleased/fix-trace-encoding.yml b/changelogs/unreleased/fix-trace-encoding.yml new file mode 100644 index 00000000000..152610c43f5 --- /dev/null +++ b/changelogs/unreleased/fix-trace-encoding.yml @@ -0,0 +1,4 @@ +--- +title: Fix another case where trace does not have proper encoding set +merge_request: 10728 +author: diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb index 3b335cdfd01..b929bdd55bc 100644 --- a/lib/gitlab/ci/trace/stream.rb +++ b/lib/gitlab/ci/trace/stream.rb @@ -14,6 +14,14 @@ module Gitlab def initialize @stream = yield + if @stream + @stream.binmode + # Ci::Ansi2html::Converter would read from @stream directly, + # using @stream.each_line to be specific. It's safe to set + # the encoding here because IO#seek(bytes) and IO#read(bytes) + # are not characters based, so encoding doesn't matter to them. + @stream.set_encoding(Encoding.default_external) + end end def valid? @@ -51,7 +59,7 @@ module Gitlab read_last_lines(last_lines) else stream.read - end + end.force_encoding(Encoding.default_external) end def html_with_state(state = nil) @@ -113,7 +121,6 @@ module Gitlab end chunks.join.lines.last(last_lines).join - .force_encoding(Encoding.default_external) end end end diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb index 9e3bd6d662f..03f040f4465 100644 --- a/spec/lib/gitlab/ci/trace/stream_spec.rb +++ b/spec/lib/gitlab/ci/trace/stream_spec.rb @@ -43,13 +43,29 @@ describe Gitlab::Ci::Trace::Stream do it 'forwards to the next linefeed, case 1' do stream.limit(7) - expect(stream.raw).to eq('') + result = stream.raw + + expect(result).to eq('') + expect(result.encoding).to eq(Encoding.default_external) end it 'forwards to the next linefeed, case 2' do stream.limit(29) - expect(stream.raw).to eq("\e[01;32m許功蓋\e[0m\n") + result = stream.raw + + expect(result).to eq("\e[01;32m許功蓋\e[0m\n") + expect(result.encoding).to eq(Encoding.default_external) + end + + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/30796 + it 'reads in binary, output as Encoding.default_external' do + stream.limit(52) + + result = stream.html + + expect(result).to eq("ヾ(´༎ຶД༎ຶ`)ノ<br><span class=\"term-fg-green\">許功蓋</span><br>") + expect(result.encoding).to eq(Encoding.default_external) end end end diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 4e71597521d..ced93c8f762 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -29,7 +29,8 @@ RSpec.describe AbuseReport, type: :model do it 'lets a worker delete the user' do expect(DeleteUserWorker).to receive(:perform_async).with(user.id, subject.user.id, - delete_solo_owned_groups: true) + delete_solo_owned_groups: true, + hard_delete: true) subject.remove_user(deleted_by: user) end diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index 43c18992d1a..4bc30018ebd 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -152,6 +152,12 @@ describe Users::DestroyService, services: true do service.execute(user) end + + it 'does not run `MigrateToGhostUser` if hard_delete option is given' do + expect_any_instance_of(Users::MigrateToGhostUserService).not_to receive(:execute) + + service.execute(user, hard_delete: true) + end end end end |