From 591c1e179ba950cdfb96523db016acfb57ce5b05 Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Tue, 23 Oct 2012 16:13:20 -0700 Subject: [PATCH 1/2] (#10278) Add Facter::Util::Resolution.locale_wrapper Without this patch, Facter does not have a clean way to reset the locale when executing system commands. This is a problem because we cannot depend on the output of commands like `ifconfig` unless we can also depend on the localization being used. This patch addresses the problem by adding the method locale_wrapper method. This method sets the LANG, LC_ALL, and LC_CTYPE environment variables to the value of C before evaluating the given block. --- lib/facter/util/resolution.rb | 21 ++++++++++++++++ spec/unit/util/resolution_spec.rb | 41 +++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb index 45dac53302..0355742dcb 100644 --- a/lib/facter/util/resolution.rb +++ b/lib/facter/util/resolution.rb @@ -255,4 +255,25 @@ def value return nil if result == "" return result end + + ## + # locale_wrapper accepts a block, sets the desired enviornment locale, then + # yields to the block. All of this happens in a with a thread local + # exclusive lock. + def self.locale_wrapper(env=ENV, &block) + rval = nil + Thread.exclusive do + current_lang = env['LANG'] + current_lc_all = env['LC_ALL'] + current_lc_ctype = env['LC_CTYPE'] + env['LANG'] = 'C' + env['LC_ALL'] = 'C' + env['LC_CTYPE'] = 'C' + rval = block.call + env['LANG'] = current_lang + env['LC_ALL'] = current_lc_all + env['LC_CTYPE'] = current_lc_ctype + end + rval + end end diff --git a/spec/unit/util/resolution_spec.rb b/spec/unit/util/resolution_spec.rb index ca41648d8c..d6b33304ad 100755 --- a/spec/unit/util/resolution_spec.rb +++ b/spec/unit/util/resolution_spec.rb @@ -565,4 +565,45 @@ end end end + + describe ".locale_wrapper" do + let(:env) do + env = ENV.to_hash + env['LANG'] = 'en_US.UTF8' + env['LC_ALL'] = 'en_US.UTF8' + env['LC_CTYPE'] = 'en_US.UTF8' + env + end + + it "accepts an environment object" do + Facter::Util::Resolution.locale_wrapper(env) do + "nothing to see here" + end + end + + it "returns the value of the block" do + Facter::Util::Resolution.locale_wrapper(env) do + "nothing to see here" + end.should == "nothing to see here" + end + + it "preserves the environment" do + env_start = env.dup + Facter::Util::Resolution.locale_wrapper(env) do + "nothing to see here" + end + + env.should == env_start + end + + %w{ LANG LC_CTYPE LC_ALL }.each do |var| + it "sets #{var} to 'C'" do + env[var].should == 'en_US.UTF8' + Facter::Util::Resolution.locale_wrapper(env) do + env[var].should == 'C' + end + env[var].should == 'en_US.UTF8' + end + end + end end From 150b4b781a5e241ac279f3911a5250399ac273a6 Mon Sep 17 00:00:00 2001 From: Jeff McCune Date: Tue, 23 Oct 2012 16:15:45 -0700 Subject: [PATCH 2/2] (#10278) Force ifconfig fact to use Facter default locale Without this patch Facter executes the ifconfig command without controlling the localization environment. This is a problem because the output of ifconfig causes the parser to fail when in the `de_DE.UTF8` locale. This patch fixes the problem by switching the ipaddress fact to use the locale_wrapper method. --- lib/facter/ipaddress.rb | 58 +++++++++++++++++++----------------- lib/facter/util/ipaddress.rb | 14 +++++++++ spec/unit/ipaddress_spec.rb | 2 ++ 3 files changed, 46 insertions(+), 28 deletions(-) create mode 100644 lib/facter/util/ipaddress.rb diff --git a/lib/facter/ipaddress.rb b/lib/facter/ipaddress.rb index a7d7c4d6be..363046edbf 100644 --- a/lib/facter/ipaddress.rb +++ b/lib/facter/ipaddress.rb @@ -1,3 +1,6 @@ +require 'facter/util/ipaddress' +require 'facter/util/resolution' + # Fact: ipaddress # # Purpose: Return the main IP address for a host. @@ -41,17 +44,17 @@ confine :kernel => %w{FreeBSD OpenBSD Darwin DragonFly} setcode do ip = nil - output = %x{/sbin/ifconfig} - - output.split(/^\S/).each { |str| - if str =~ /inet ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/ - tmp = $1 - unless tmp =~ /^127\./ - ip = tmp - break + if output = Facter::Util::Resolution.locale_wrapper { Facter::Util::IPAddress.ifconfig("/sbin/ifconfig") } + output.split(/^\S/).each do |str| + if match = str.match(/inet ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) + tmp = match[1] + unless tmp =~ /^127\./ + ip = tmp + break + end end end - } + end ip end @@ -61,17 +64,17 @@ confine :kernel => %w{NetBSD SunOS} setcode do ip = nil - output = %x{/sbin/ifconfig -a} - - output.split(/^\S/).each { |str| - if str =~ /inet ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/ - tmp = $1 - unless tmp =~ /^127\./ or tmp == "0.0.0.0" - ip = tmp - break + if output = Facter::Util::Resolution.locale_wrapper { Facter::Util::IPAddress.ifconfig("/sbin/ifconfig -a") } + output.split(/^\S/).each do |str| + if match = str.match(/inet ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) + tmp = match[1] + unless tmp =~ /^127\./ or tmp == "0.0.0.0" + ip = tmp + break + end end end - } + end ip end @@ -81,18 +84,17 @@ confine :kernel => %w{AIX} setcode do ip = nil - output = %x{/usr/sbin/ifconfig -a} - - output.split(/^\S/).each { |str| - if str =~ /inet ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/ - tmp = $1 - unless tmp =~ /^127\./ - ip = tmp - break + if output = Facter::Util::Resolution.locale_wrapper { Facter::Util::IPAddress.ifconfig("/usr/sbin/ifconfig -a") } + output.split(/^\S/).each do |str| + if match = str.match(/inet ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)/) + tmp = match[1] + unless tmp =~ /^127\./ + ip = tmp + break + end end end - } - + end ip end end diff --git a/lib/facter/util/ipaddress.rb b/lib/facter/util/ipaddress.rb new file mode 100644 index 0000000000..3de2920d32 --- /dev/null +++ b/lib/facter/util/ipaddress.rb @@ -0,0 +1,14 @@ +require 'facter/util/resolution' + +module Facter +module Util +module IPAddress + ## + # ifconfig provides a wrapper around Facter::Util::Resolution.exec intended + # to be stubbed in the tests. + def self.ifconfig(command="ifconfig 2>/dev/null") + Facter::Util::Resolution.exec(command) + end +end +end +end diff --git a/spec/unit/ipaddress_spec.rb b/spec/unit/ipaddress_spec.rb index f746e1012e..33ebe899bf 100755 --- a/spec/unit/ipaddress_spec.rb +++ b/spec/unit/ipaddress_spec.rb @@ -1,5 +1,7 @@ #! /usr/bin/env ruby +require 'facter/util/resolution' +require 'facter/util/ipaddress' require 'spec_helper' require 'facter/util/ip'