summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/abuse_report.rb2
-rw-r--r--app/services/users/destroy_service.rb2
-rw-r--r--changelogs/unreleased/fix-trace-encoding.yml4
-rw-r--r--lib/gitlab/ci/trace/stream.rb11
-rw-r--r--spec/lib/gitlab/ci/trace/stream_spec.rb20
-rw-r--r--spec/models/abuse_report_spec.rb3
-rw-r--r--spec/services/users/destroy_service_spec.rb6
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