Merge pull request #319 from github/encode-shell-to-utf8

Encode shell output into UTF8

lib/licensed/shell.rb

@@ -9,11 +9,12 @@ module Shell
     def self.execute(cmd, *args, allow_failure: false, env: {})
       stdout, stderr, status = Open3.capture3(env, cmd, *args)
 
-      if status.success? || allow_failure
-        stdout.strip
-      else
-        raise Error.new([cmd, *args], status.exitstatus, stderr)
+      if !status.success? && !allow_failure
+        raise Error.new([cmd, *args], status.exitstatus, encode_content(stderr))
       end
+
+      # ensure that returned data is properly encoded
+      encode_content(stdout.strip)
     end
 
     # Executes a command and returns a boolean value indicating if the command
@@ -55,5 +56,21 @@ def escape_cmd
         end.join(" ")
       end
     end
+
+    private
+
+    ENCODING = Encoding::UTF_8
+    ENCODING_OPTIONS = {
+      invalid: :replace,
+      undef:   :replace,
+      replace: "",
+      univeral_newline: true
+    }.freeze
+
+    # Ensure that content that is returned from shell commands is in a usable
+    # encoding for the rest of the application
+    def self.encode_content(content)
+      content.encode(ENCODING, **ENCODING_OPTIONS)
+    end
   end
 end

test/shell_test.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+require "test_helper"
+require "tmpdir"
+
+describe Licensed::Shell do
+  let(:root) { File.expand_path("../..", __FILE__) }
+
+  describe "#execute" do
+    let(:content) { "��test".dup.force_encoding("ASCII-8BIT") }
+    let(:expected) { "test" }
+
+    it "encodes non-utf8 content in stdout" do
+      Open3.expects(:capture3).returns([content, "", stub(success?: true)])
+      assert_equal expected, Licensed::Shell.execute("test")
+    end
+
+    it "encodes non-utf8 content in stderr" do
+      Open3.expects(:capture3).returns(["", content, stub(success?: false, exitstatus: 1)])
+      err = assert_raises Licensed::Shell::Error do
+        Licensed::Shell.execute("test")
+      end
+
+      assert_equal "'test' exited with status 1\n        #{expected}", err.message
+    end
+  end
+end