(FACT-3497) Facter should not fail when binary data is detected

This commit updates LegacyFacter::Util::Normalization.normalize_string to
accept fact values with ASCII 8-BIT without immediately failing. When a fact
value with ASCII 8-BIT is detected but can be force encoded to UTF-8, Facter
will log a debug message with the fact name and value. Facter will only error
(with the fact name & value) when the fact value cannot be successfully force
encoded to UTF-8.

This commit also updates all the normalize methods in
LegacyFacter::Util::Normalization to take in 2 parameters now: the fact name &
value so the debug and error messages can include the fact name. This helps
users more easily figure out which fact is causing issues and has binary.

Author: AriaXLi

lib/facter/custom_facts/core/resolvable.rb

@@ -79,7 +79,7 @@ def value
           end
         end
 
-        LegacyFacter::Util::Normalization.normalize(result)
+        LegacyFacter::Util::Normalization.normalize(@fact.name, result)
       rescue Timeout::Error => e
         Facter.log_exception(e, "Timed out after #{limit} seconds while resolving #{qualified_name}")
 

lib/facter/custom_facts/util/fact.rb

@@ -35,7 +35,7 @@ class Fact
       #
       # @api private
       def initialize(name, options = {})
-        @name = LegacyFacter::Util::Normalization.normalize(name.to_s.downcase).intern
+        @name = LegacyFacter::Util::Normalization.normalize(name.to_s.downcase, name.to_s.downcase).intern
 
         @options = options.dup
         extract_ldapname_option!(options)

lib/facter/custom_facts/util/normalization.rb

@@ -17,18 +17,18 @@ class NormalizationError < StandardError; end
       # @api public
       # @raise [NormalizationError] If the data structure contained an invalid element.
       # @return [void]
-      def normalize(value)
+      def normalize(fact_name, value)
         case value
         when Integer, Float, TrueClass, FalseClass, NilClass, Symbol
           value
         when Date, Time
           value.iso8601
         when String
-          normalize_string(value)
+          normalize_string(fact_name, value)
         when Array
-          normalize_array(value)
+          normalize_array(fact_name, value)
         when Hash
-          normalize_hash(value)
+          normalize_hash(fact_name, value)
         else
           raise NormalizationError, "Expected #{value} to be one of #{VALID_TYPES.inspect}, but was #{value.class}"
         end
@@ -47,12 +47,17 @@ def normalize(value)
       # @param value [String]
       # @return [void]
 
-      def normalize_string(value)
+      def normalize_string(fact_name, value)
         if value.encoding == Encoding::ASCII_8BIT
-          raise NormalizationError, "String #{value.inspect} contains binary data"
-        end
+          value_copy_encoding = value.dup.force_encoding(Encoding::UTF_8)
+          raise NormalizationError, "custom fact \"#{fact_name}\" with value: #{value_copy_encoding} contains binary data and could not be force encoded to UTF-8" unless value_copy_encoding.valid_encoding?
+
+          log.debug("custom fact \"#{fact_name}\" with value: #{value} contained binary data and was force encoded to UTF-8 and now has the value: #{value_copy_encoding}")
+          value = value_copy_encoding
 
-        value = value.encode(Encoding::UTF_8)
+        else
+          value = value.encode(Encoding::UTF_8)
+        end
 
         unless value.valid_encoding?
           raise NormalizationError, "String #{value.inspect} doesn't match the reported encoding #{value.encoding}"
@@ -73,9 +78,9 @@ def normalize_string(value)
       # @raise [NormalizationError] If one of the elements failed validation
       # @param value [Array]
       # @return [void]
-      def normalize_array(value)
+      def normalize_array(fact_name, value)
         value.collect do |elem|
-          normalize(elem)
+          normalize(fact_name, elem)
         end
       end
 
@@ -85,8 +90,12 @@ def normalize_array(value)
       # @raise [NormalizationError] If one of the keys or values failed normalization
       # @param value [Hash]
       # @return [void]
-      def normalize_hash(value)
-        Hash[value.collect { |k, v| [normalize(k), normalize(v)] }]
+      def normalize_hash(fact_name, value)
+        Hash[value.collect { |k, v| [normalize(fact_name, k), normalize(fact_name, v)] }]
+      end
+
+      def log
+        Facter::Log.new(self)
       end
     end
   end

lib/facter/framework/core/fact_manager.rb

@@ -80,7 +80,7 @@ def resolve_core(user_query = [], options = {})
 
     def validate_resolved_facts(resolved_facts)
       resolved_facts.each do |fact|
-        normalize(fact.value)
+        normalize(fact.name, fact.value)
       rescue LegacyFacter::Util::Normalization::NormalizationError => e
         detail = e.message
         detail += "\n#{e.backtrace.join("\n")}" if @options[:trace]

spec/custom_facts/util/normalization_spec.rb

@@ -23,48 +23,65 @@ def utf8(str)
     describe 'and string encoding is supported', if: String.instance_methods.include?(:encoding) do
       it 'accepts strings that are ASCII and match their encoding and converts them to UTF-8' do
         str = 'ASCII'.encode(Encoding::ASCII)
-        normalized_str = normalization.normalize(str)
+        normalized_str = normalization.normalize('', str)
         expect(normalized_str.encoding).to eq(Encoding::UTF_8)
       end
 
+      it 'accepts ASCII 8-BIT (binary) string that can be successfully force encoded to UTF-8 and logs a debug message with the fact name and value and returns UTF-8 encoded version of the passed in string' do
+        str = 'test'.encode(Encoding::ASCII_8BIT)
+        logger = Facter::Log.new(self)
+        allow(normalization).to receive(:log).and_return logger # rubocop:disable RSpec/SubjectStub
+        expect(logger).to receive(:debug).with(/custom fact "" with value: #{str} contained binary data and was force encoded to UTF-8 and now has the value: #{str}/)
+        value = normalization.normalize('', str)
+        expect(value).to eq('test')
+        expect(value.encoding).to eq(Encoding::UTF_8)
+      end
+
+      it 'errors when given binary string that cannot be successfully force encoded to UTF-8' do
+        str = "\xc0".dup.force_encoding(Encoding::ASCII_8BIT) # rubocop:disable Performance/UnfreezeString
+        expect do
+          normalization.normalize('', str)
+        end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError)
+      end
+
       it 'accepts strings that are UTF-8 and match their encoding' do
         str = "let's make a ☃!".encode(Encoding::UTF_8)
-        expect(normalization.normalize(str)).to eq(str)
+        expect(normalization.normalize('', str)).to eq(str)
       end
 
       it 'converts valid non UTF-8 strings to UTF-8' do
         str = "let's make a ☃!".encode(Encoding::UTF_16LE)
-        enc = normalization.normalize(str).encoding
+        enc = normalization.normalize('', str).encoding
         expect(enc).to eq(Encoding::UTF_8)
       end
 
       it 'normalizes a frozen string returning a non-frozen string' do
         str = 'factvalue'.encode(Encoding::UTF_16LE).freeze
 
-        normalized_str = normalization.normalize(str)
+        normalized_str = normalization.normalize('', str)
         expect(normalized_str).not_to be_frozen
       end
 
       it 'rejects strings that are not UTF-8 and do not match their claimed encoding' do
         invalid_shift_jis = (+"\xFF\x5C!").force_encoding(Encoding::SHIFT_JIS)
         expect do
-          normalization.normalize(invalid_shift_jis)
+          normalization.normalize('', invalid_shift_jis)
         end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError,
                            /String encoding Shift_JIS is not UTF-8 and could not be converted to UTF-8/)
       end
 
       it "rejects strings that claim to be UTF-8 encoded but aren't" do
         str = (+"\255ay!").force_encoding(Encoding::UTF_8)
         expect do
-          normalization.normalize(str)
+          normalization.normalize('', str)
         end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError,
                            /String "\\xADay!" doesn't match the reported encoding UTF-8/)
       end
 
       it 'rejects strings that only have the start of a valid UTF-8 sequence' do
         str = "\xc3\x28"
         expect do
-          normalization.normalize(str)
+          normalization.normalize('', str)
         end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError,
                            /String "\\xC3\(" doesn't match the reported encoding UTF-8/)
       end
@@ -73,7 +90,7 @@ def utf8(str)
         str = 'Mitteleuropäische Zeit'.encode(Encoding::UTF_16LE)
         str.force_encoding(Encoding::UTF_8)
         expect do
-          normalization.normalize(str)
+          normalization.normalize('', str)
         end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError,
                            /String.*doesn't match the reported encoding UTF-8/)
       end
@@ -85,7 +102,7 @@ def utf8(str)
                         "null byte \x00 in the middle"]
         test_strings.each do |str|
           expect do
-            normalization.normalize(str)
+            normalization.normalize('', str)
           end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError,
                              /String.*contains a null byte reference/)
         end
@@ -95,13 +112,13 @@ def utf8(str)
     describe 'and string encoding is not supported', unless: String.instance_methods.include?(:encoding) do
       it 'accepts strings that are UTF-8 and match their encoding' do
         str = "let's make a ☃!"
-        expect(normalization.normalize(str)).to eq(str)
+        expect(normalization.normalize('', str)).to eq(str)
       end
 
       it 'rejects strings that are not UTF-8' do
         str = "let's make a \255\255\255!"
         expect do
-          normalization.normalize(str)
+          normalization.normalize('', str)
         end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError, /String .* is not valid UTF-8/)
       end
     end
@@ -112,7 +129,7 @@ def utf8(str)
       arr = [utf16('first'), utf16('second'), [utf16('third'), utf16('fourth')]]
       expected_arr = [utf8('first'), utf8('second'), [utf8('third'), utf8('fourth')]]
 
-      expect(normalization.normalize_array(arr)).to eq(expected_arr)
+      expect(normalization.normalize_array('', arr)).to eq(expected_arr)
     end
   end
 
@@ -121,7 +138,7 @@ def utf8(str)
       hsh = { utf16('first') => utf16('second'), utf16('third') => [utf16('fourth'), utf16('fifth')] }
       expected_hsh = { utf8('first') => utf8('second'), utf8('third') => [utf8('fourth'), utf8('fifth')] }
 
-      expect(normalization.normalize_hash(hsh)).to eq(expected_hsh)
+      expect(normalization.normalize_hash('', hsh)).to eq(expected_hsh)
     end
   end
 
@@ -131,7 +148,7 @@ def utf8(str)
         value = Time.utc(2020)
         expected = '2020-01-01T00:00:00Z'
 
-        expect(normalization.normalize(value)).to eq(expected)
+        expect(normalization.normalize('', value)).to eq(expected)
       end
     end
 
@@ -140,20 +157,20 @@ def utf8(str)
         value = Date.new(2020)
         expected = '2020-01-01'
 
-        expect(normalization.normalize(value)).to eq(expected)
+        expect(normalization.normalize('', value)).to eq(expected)
       end
     end
 
     [1, 1.0, true, false, nil, :symbol].each do |val|
       it "accepts #{val.inspect}:#{val.class}" do
-        expect(normalization.normalize(val)).to eq(val)
+        expect(normalization.normalize('', val)).to eq(val)
       end
     end
 
     [Object.new, Set.new].each do |val|
       it "rejects #{val.inspect}:#{val.class}" do
         expect do
-          normalization.normalize(val)
+          normalization.normalize('', val)
         end.to raise_error(LegacyFacter::Util::Normalization::NormalizationError, /Expected .*but was #{val.class}/)
       end
     end