Merge pull request puppetlabs#4665 from Iristyle/ticket/stable/PUP-58…

…79-read-settings-and-JSON-files-as-UTF8

(PUP-5879) Read settings and json files as utf8

lib/puppet/data_providers/json_data_provider_factory.rb

@@ -18,7 +18,7 @@ class JsonDataProvider < Puppet::Plugins::DataProviders::PathBasedDataProvider
     include HieraInterpolate
 
     def initialize_data(path, lookup_invocation)
-      JSON.parse(File.read(path))
+      JSON.parse(Puppet::FileSystem.read(path, :encoding => 'utf-8'))
     rescue JSON::ParserError => ex
       # Filename not included in message, so we add it here.
       raise Puppet::DataBinding::LookupError, "Unable to parse (#{path}): #{ex.message}"

lib/puppet/indirector/msgpack.rb

@@ -58,7 +58,7 @@ def load_msgpack_from_file(file, key)
     msgpack = nil
 
     begin
-      msgpack = File.read(file)
+      msgpack = Puppet::FileSystem.read(file, :encoding => 'utf-8')
     rescue Errno::ENOENT
       return nil
     rescue => detail

lib/puppet/module.rb

@@ -48,7 +48,7 @@ def self.is_module_directory_name?(name)
 
   def self.is_module_namespaced_name?(name)
     # it must match the full module name according to forge validator
-    return true if name =~ /^[a-zA-Z0-9]+[-][a-z][a-z0-9_]*$/   
+    return true if name =~ /^[a-zA-Z0-9]+[-][a-z][a-z0-9_]*$/
     return false
   end
 
@@ -90,7 +90,7 @@ def has_metadata?
     return false unless Puppet::FileSystem.exist?(metadata_file)
 
     begin
-      metadata =  JSON.parse(File.read(metadata_file))
+      metadata =  JSON.parse(File.read(metadata_file, :encoding => 'utf-8'))
     rescue JSON::JSONError => e
       Puppet.debug("#{name} has an invalid and unparsable metadata.json file.  The parse error: #{e.message}")
       return false
@@ -145,7 +145,7 @@ def license_file
   end
 
   def load_metadata
-    @metadata = data = JSON.parse(File.read(metadata_file))
+    @metadata = data = JSON.parse(File.read(metadata_file, :encoding => 'utf-8'))
     @forge_name = data['name'].gsub('-', '/') if data['name']
 
     [:source, :author, :version, :license, :dependencies].each do |attr|

lib/puppet/module_tool/applications/builder.rb

@@ -65,9 +65,9 @@ def ignored_files
           gitignore = File.join(@path, '.gitignore')
 
           if File.file? pmtignore
-            @ignored_files = PathSpec.new File.read(pmtignore)
+            @ignored_files = PathSpec.new Puppet::FileSystem.read(pmtignore, :encoding => 'utf-8')
           elsif File.file? gitignore
-            @ignored_files = PathSpec.new File.read(gitignore)
+            @ignored_files = PathSpec.new Puppet::FileSystem.read(gitignore, :encoding => 'utf-8')
           else
             @ignored_files = PathSpec.new
           end

lib/puppet/network/auth_config_parser.rb

@@ -4,7 +4,7 @@ module Puppet::Network
 class AuthConfigParser
 
   def self.new_from_file(file)
-    self.new(File.read(file))
+    self.new(Puppet::FileSystem.read(file, :encoding => 'utf-8'))
   end
 
   def initialize(string)

lib/puppet/settings.rb

@@ -1189,7 +1189,7 @@ def value_sets_for(environment, mode)
   # Read the file in.
   # @api private
   def read_file(file)
-    return Puppet::FileSystem.read(file)
+    return Puppet::FileSystem.read(file, :encoding => 'utf-8')
   end
 
   # Private method for internal test use only; allows to do a comprehensive clear of all settings between tests.

spec/fixtures/unit/data_providers/environments/hiera_env_config/data1/third_utf8.json

@@ -0,0 +1,3 @@
+{
+    "test::param_json_utf8": "env data param_json_utf8 is ᚠᛇᚻ"
+}

spec/fixtures/unit/data_providers/environments/hiera_env_config/data1/utf8.yaml

@@ -0,0 +1,3 @@
+---
+test::param_yaml_utf8: env data param_yaml_utf8 is ᛫ᛒᛦ
+

spec/fixtures/unit/data_providers/environments/hiera_env_config/hiera.yaml

@@ -6,11 +6,15 @@
   - :name: "one path"
     :backend: yaml
     :path: "single"
-  - :name: "two paths"
+  - :name: "utf8"
+    :backend: yaml
+    :path: "utf8"
+  - :name: "three paths"
     :backend: json
     :paths:
       - "first"
       - "second"
+      - "third_utf8"
   - :name:  'other datadir'
     :backend: yaml
     :datadir: "data2"

spec/fixtures/unit/data_providers/environments/hiera_env_config/manifests/site.pp

@@ -1,5 +1,5 @@
-class test($param_a = 1, $param_b = 2, $param_c = 3, $param_d = 4, $param_e = 5) {
-  notify { "$param_a, $param_b, $param_c, $param_d, $param_e": }
+class test($param_a = 1, $param_b = 2, $param_c = 3, $param_d = 4, $param_e = 5, $param_yaml_utf8 = 'hi', $param_json_utf8 = 'hi') {
+  notify { "$param_a, $param_b, $param_c, $param_d, $param_e, $param_yaml_utf8, $param_json_utf8": }
 }
 
 include test

spec/integration/util/settings_spec.rb

@@ -44,6 +44,32 @@ def define_settings(section, settings_hash)
     expect(Puppet::FileSystem.stat(settings[:maindir]).mode & 007777).to eq(0750)
   end
 
+  it "will properly parse a UTF-8 configuration file" do
+    rune_utf8 = "\u16A0\u16C7\u16BB" # ᚠᛇᚻ
+    config = tmpfile("config")
+    define_settings(:main,
+      :config => {
+        :type => :file,
+        :default => config,
+        :desc => "a"
+      },
+      :environment => {
+        :default => 'dingos',
+        :desc => 'test',
+      }
+    )
+
+    File.open(config, 'w') do |file|
+      file.puts <<-EOF
+[main]
+environment=#{rune_utf8}
+      EOF
+    end
+
+    settings.initialize_global_settings
+    expect(settings[:environment]).to eq(rune_utf8)
+  end
+
   it "reparses configuration if configuration file is touched", :if => !Puppet.features.microsoft_windows? do
     config = tmpfile("config")
     define_settings(:main,

spec/unit/data_providers/hiera_data_provider_spec.rb

@@ -45,7 +45,7 @@ def compile(environment, code = nil)
 
   it 'reads hiera.yaml in environment root and configures multiple json and yaml providers' do
     resources = compile_and_get_notifications('hiera_env_config')
-    expect(resources).to include('env data param_a is 10, env data param_b is 20, env data param_c is 30, env data param_d is 40, env data param_e is 50')
+    expect(resources).to include("env data param_a is 10, env data param_b is 20, env data param_c is 30, env data param_d is 40, env data param_e is 50, env data param_yaml_utf8 is \u16EB\u16D2\u16E6, env data param_json_utf8 is \u16A0\u16C7\u16BB")
   end
 
   it 'reads hiera.yaml in module root and configures multiple json and yaml providers' do

spec/unit/indirector/msgpack_spec.rb

@@ -76,12 +76,22 @@ def with_content(text)
         expect(subject.find(request)).to be_nil
       end
 
+      it "can load a UTF-8 file from disk" do
+        rune_utf8 = "\u16A0\u16C7\u16BB" # ᚠᛇᚻ
+
+        with_content(model.new(rune_utf8).to_msgpack) do
+          instance = subject.find(request)
+          expect(instance).to be_an_instance_of model
+          expect(instance.value).to eq(rune_utf8)
+        end
+      end
+
       it "raises a descriptive error when the file can't be read" do
         with_content(model.new('foo').to_msgpack) do
           # I don't like this, but there isn't a credible alternative that
           # also works on Windows, so a stub it is. At least the expectation
           # will fail if the implementation changes. Sorry to the next dev.
-          File.expects(:read).with(file).raises(Errno::EPERM)
+          Puppet::FileSystem.expects(:read).with(file, {:encoding => 'utf-8'}).raises(Errno::EPERM)
           expect { subject.find(request) }.
             to raise_error Puppet::Error, /Could not read MessagePack/
         end

spec/unit/module_spec.rb

@@ -609,21 +609,21 @@
 
   it "should have metadata if it has a metadata file and its data is not empty" do
     Puppet::FileSystem.expects(:exist?).with(@module.metadata_file).returns true
-    File.stubs(:read).with(@module.metadata_file).returns "{\"foo\" : \"bar\"}"
+    File.stubs(:read).with(@module.metadata_file, {:encoding => 'utf-8'}).returns "{\"foo\" : \"bar\"}"
 
     expect(@module).to be_has_metadata
   end
 
   it "should have metadata if it has a metadata file and its data is not empty" do
     Puppet::FileSystem.expects(:exist?).with(@module.metadata_file).returns true
-    File.stubs(:read).with(@module.metadata_file).returns "{\"foo\" : \"bar\"}"
+    File.stubs(:read).with(@module.metadata_file, {:encoding => 'utf-8'}).returns "{\"foo\" : \"bar\"}"
 
     expect(@module).to be_has_metadata
   end
 
   it "should not have metadata if has a metadata file and its data is empty" do
     Puppet::FileSystem.expects(:exist?).with(@module.metadata_file).returns true
-    File.stubs(:read).with(@module.metadata_file).returns "This is some invalid json.\n"
+    File.stubs(:read).with(@module.metadata_file, {:encoding => 'utf-8'}).returns "This is some invalid json.\n"
     expect(@module).not_to be_has_metadata
   end
 
@@ -646,7 +646,7 @@
 
   it "should tolerate failure to parse" do
     Puppet::FileSystem.expects(:exist?).with(@module.metadata_file).returns true
-    File.stubs(:read).with(@module.metadata_file).returns(my_fixture('trailing-comma.json'))
+    File.stubs(:read).with(@module.metadata_file, {:encoding => 'utf-8'}).returns(my_fixture('trailing-comma.json'))
 
     expect(@module.has_metadata?).to be_falsey
   end
@@ -656,7 +656,7 @@ def a_module_with_metadata(data)
 
     mod = Puppet::Module.new("foo", "/path", mock("env"))
     mod.stubs(:metadata_file).returns "/my/file"
-    File.stubs(:read).with("/my/file").returns text
+    File.stubs(:read).with("/my/file", {:encoding => 'utf-8'}).returns text
     mod
   end
 
@@ -681,7 +681,7 @@ def a_module_with_metadata(data)
       it "should fail if #{attr} is not present in the metadata file" do
         @data.delete(attr.to_sym)
         @text = @data.to_pson
-        File.stubs(:read).with("/my/file").returns @text
+        File.stubs(:read).with("/my/file", {:encoding => 'utf-8'}).returns @text
         expect { @module.load_metadata }.to raise_error(
           Puppet::Module::MissingMetadata,
           "No #{attr} module metadata provided for foo"
@@ -690,6 +690,30 @@ def a_module_with_metadata(data)
     end
   end
 
+  describe "when loading the metadata file from disk" do
+    it "should properly parse utf-8 contents" do
+      rune_utf8 = "\u16A0\u16C7\u16BB" # ᚠᛇᚻ
+      metadata_json = tmpfile('metadata.json')
+      File.open(metadata_json, 'w') do |file|
+        file.puts <<-EOF
+  {
+    "license" : "GPL2",
+    "author" : "#{rune_utf8}",
+    "version" : "1.0",
+    "source" : "http://foo/",
+    "dependencies" : []
+  }
+        EOF
+      end
+
+      mod = Puppet::Module.new('foo', '/path', mock('env'))
+      mod.stubs(:metadata_file).returns metadata_json
+
+      mod.load_metadata
+      expect(mod.author).to eq(rune_utf8)
+    end
+  end
+
   it "should be able to tell if there are local changes" do
     modpath = tmpdir('modpath')
     foo_checksum = 'acbd18db4cc2f85cedef654fccc4a4d8'

spec/unit/module_tool/applications/builder_spec.rb

@@ -56,37 +56,41 @@ def create_ignored_files
       Puppet::FileSystem.touch(File.join(path, 'gitdirectory/gitartifact'))
       Puppet::FileSystem.touch(File.join(path, 'gitdirectory/gitimportantfile'))
       Puppet::FileSystem.touch(File.join(path, 'gitdirectory/sub/artifact'))
+      Puppet::FileSystem.touch(File.join(path, "git\u16A0\u16C7\u16BB"))
       Puppet::FileSystem.touch(File.join(path, 'pmtignored.foo'))
       Puppet::FileSystem.mkpath(File.join(path, 'pmtdirectory/sub'))
       Puppet::FileSystem.touch(File.join(path, 'pmtdirectory/pmtimportantfile'))
       Puppet::FileSystem.touch(File.join(path, 'pmtdirectory/pmtartifact'))
       Puppet::FileSystem.touch(File.join(path, 'pmtdirectory/sub/artifact'))
+      Puppet::FileSystem.touch(File.join(path, "pmt\u16A0\u16C7\u16BB"))
     end
 
     def create_pmtignore_file
-      Puppet::FileSystem.open(File.join(path, '.pmtignore'), 0600, 'w') do |f|
+      File.open(File.join(path, '.pmtignore'), 'w', 0600, :encoding => 'utf-8') do |f|
         f << <<-PMTIGNORE
 pmtignored.*
 pmtdirectory/sub/**
 pmtdirectory/pmt*
 !pmtimportantfile
+pmt\u16A0\u16C7\u16BB
 PMTIGNORE
       end
     end
 
     def create_gitignore_file
-      Puppet::FileSystem.open(File.join(path, '.gitignore'), 0600, 'w') do |f|
+      File.open(File.join(path, '.gitignore'), 'w', 0600, :encoding => 'utf-8') do |f|
         f << <<-GITIGNORE
 gitignored.*
 gitdirectory/sub/**
 gitdirectory/git*
 !gitimportantfile
+git\u16A0\u16C7\u16BB
 GITIGNORE
       end
     end
 
     def create_symlink_gitignore_file
-      Puppet::FileSystem.open(File.join(path, '.gitignore'), 0600, 'w') do |f|
+      File.open(File.join(path, '.gitignore'), 'w', 0600, :encoding => 'utf-8') do |f|
         f << <<-GITIGNORE
 symlinkfile
     GITIGNORE
@@ -152,6 +156,10 @@ def create_symlink_gitignore_file
         expect(target_exists?('gitignored.foo')).to eq true
       end
 
+      it "leaves UTF-8 files" do
+        expect(target_exists?("git\u16A0\u16C7\u16BB")).to eq true
+      end
+
       it "leaves directories" do
         expect(target_exists?('gitdirectory')).to eq true
       end
@@ -178,6 +186,10 @@ def create_symlink_gitignore_file
         expect(target_exists?('gitignored.foo')).to eq false
       end
 
+      it "ignores UTF-8 files" do
+        expect(target_exists?("git\u16A0\u16C7\u16BB")).to eq false
+      end
+
       it "ignores directories" do
         expect(target_exists?('gitdirectory')).to eq true
       end
@@ -204,6 +216,10 @@ def create_symlink_gitignore_file
         expect(target_exists?('pmtignored.foo')).to eq true
       end
 
+      it "leaves UTF-8 files" do
+        expect(target_exists?("pmt\u16A0\u16C7\u16BB")).to eq true
+      end
+
       it "leaves directories" do
         expect(target_exists?('pmtdirectory')).to eq true
       end
@@ -230,6 +246,10 @@ def create_symlink_gitignore_file
         expect(target_exists?('pmtignored.foo')).to eq false
       end
 
+      it "ignores UTF-8 files" do
+        expect(target_exists?("pmt\u16A0\u16C7\u16BB")).to eq false
+      end
+
       it "ignores directories" do
         expect(target_exists?('pmtdirectory')).to eq true
       end

spec/unit/network/auth_config_parser_spec.rb

@@ -4,6 +4,7 @@
 require 'puppet/network/authconfig'
 
 describe Puppet::Network::AuthConfigParser do
+  include PuppetSpec::Files
 
   let(:fake_authconfig) do
     "path ~ ^/catalog/([^/])\nmethod find\nallow *\n"
@@ -98,4 +99,19 @@
       described_class.new("path /certificates\nauthenticated yes").parse_rights
     end
   end
+
+  describe "when parsing rights from files" do
+    it "can read UTF-8" do
+      rune_path = "/\u16A0\u16C7\u16BB" # ᚠᛇᚻ
+      config = tmpfile('config')
+
+      File.open(config, 'w', :encoding => 'utf-8') do |file|
+        file.puts <<-EOF
+path #{rune_path}
+      EOF
+    end
+
+      expect(described_class.new_from_file(config).parse_rights[rune_path]).to be
+    end
+  end
 end