Merge branch 'enforce-Ansi2html-output-encoding' into 'master'

Just enforce the output encoding for Ansi2html

See merge request !10758

4 files changed, 28 insertions, 12 deletions

diff --git a/changelogs/unreleased/enforce-Ansi2html-output-encoding.yml b/changelogs/unreleased/enforce-Ansi2html-output-encoding.yml
new file mode 100644
index 00000000000..b1200548518
--- /dev/null
+++ b/changelogs/unreleased/enforce-Ansi2html-output-encoding.yml
@@ -0,0 +1,4 @@
+---
+title: Fix trace cannot be written due to encoding
+merge_request: 10758
+author:
diff --git a/lib/ci/ansi2html.rb b/lib/ci/ansi2html.rb
index 1020452480a..b439b0ee29b 100644
--- a/lib/ci/ansi2html.rb
+++ b/lib/ci/ansi2html.rb
@@ -172,7 +172,7 @@ module Ci
         close_open_tags()
 
         OpenStruct.new(
-          html: @out,
+          html: @out.force_encoding(Encoding.default_external),
           state: state,
           append: append,
           truncated: truncated,
diff --git a/lib/gitlab/ci/trace/stream.rb b/lib/gitlab/ci/trace/stream.rb
index b929bdd55bc..68b14c7c04c 100644
--- a/lib/gitlab/ci/trace/stream.rb
+++ b/lib/gitlab/ci/trace/stream.rb
@@ -14,14 +14,7 @@ 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
+          @stream&.binmode
         end
 
         def valid?
@@ -68,8 +61,8 @@ module Gitlab
 
         def html(last_lines: nil)
           text = raw(last_lines: last_lines)
-          stream = StringIO.new(text)
-          ::Ci::Ansi2html.convert(stream).html
+          buffer = StringIO.new(text)
+          ::Ci::Ansi2html.convert(buffer).html
         end
 
         def extract_coverage(regex)
diff --git a/spec/lib/gitlab/ci/trace/stream_spec.rb b/spec/lib/gitlab/ci/trace/stream_spec.rb
index 03f040f4465..40ac5a3ed37 100644
--- a/spec/lib/gitlab/ci/trace/stream_spec.rb
+++ b/spec/lib/gitlab/ci/trace/stream_spec.rb
@@ -71,12 +71,20 @@ describe Gitlab::Ci::Trace::Stream do
   end
 
   describe '#append' do
+    let(:tempfile) { Tempfile.new }
+
     let(:stream) do
       described_class.new do
-        StringIO.new("12345678")
+        tempfile.write("12345678")
+        tempfile.rewind
+        tempfile
       end
     end
 
+    after do
+      tempfile.unlink
+    end
+
     it "truncates and append content" do
       stream.append("89", 4)
       stream.seek(0)
@@ -84,6 +92,17 @@ describe Gitlab::Ci::Trace::Stream do
       expect(stream.size).to eq(6)
       expect(stream.raw).to eq("123489")
     end
+
+    it 'appends in binary mode' do
+      '😺'.force_encoding('ASCII-8BIT').each_char.with_index do |byte, offset|
+        stream.append(byte, offset)
+      end
+
+      stream.seek(0)
+
+      expect(stream.size).to eq(4)
+      expect(stream.raw).to eq('😺')
+    end
   end
 
   describe '#set' do