(PUP-11348) Remove Windows ENV monkeypatches

Ruby 3 no longer relies on the current code page when reading/writing env
vars[1]. Instead it uses wide Win32 APIs and transcodes them to UTF-8.

Ruby's implementation also excludes env var that start with `=`

ruby/ruby@ca76337

Author: joshcooper

lib/puppet/util.rb

@@ -39,103 +39,53 @@ def create_erb(content)
   end
   module_function :create_erb
 
-  # @param name [String] The name of the environment variable to retrieve
-  # @param mode [Symbol] Which operating system mode to use e.g. :posix or :windows.  Use nil to autodetect
-  # @return [String] Value of the specified environment variable.  nil if it does not exist
+  # @deprecated Use ENV instead
   # @api private
   def get_env(name, mode = default_env)
-    if mode == :windows
-      Puppet::Util::Windows::Process.get_environment_strings.each do |key, value |
-        if name.casecmp(key) == 0 then
-          return value
-        end
-      end
-      return nil
-    else
-      ENV[name]
-    end
+    ENV[name]
   end
   module_function :get_env
 
-  # @param mode [Symbol] Which operating system mode to use e.g. :posix or :windows.  Use nil to autodetect
-  # @return [Hash] A hashtable of all environment variables
+  # @deprecated Use ENV instead
   # @api private
   def get_environment(mode = default_env)
-    case mode
-      when :posix
-        ENV.to_hash
-      when :windows
-        Puppet::Util::Windows::Process.get_environment_strings
-      else
-        raise _("Unable to retrieve the environment for mode %{mode}") % { mode: mode }
-    end
+    ENV.to_hash
   end
   module_function :get_environment
 
-  # Removes all environment variables
-  # @param mode [Symbol] Which operating system mode to use e.g. :posix or :windows.  Use nil to autodetect
+  # @deprecated Use ENV instead
   # @api private
   def clear_environment(mode = default_env)
-    case mode
-      when :posix
-        ENV.clear
-      when :windows
-        Puppet::Util::Windows::Process.get_environment_strings.each do |key, _|
-          Puppet::Util::Windows::Process.set_environment_variable(key, nil)
-        end
-      else
-        raise _("Unable to clear the environment for mode %{mode}") % { mode: mode }
-    end
+    ENV.clear
   end
   module_function :clear_environment
 
-  # @param name [String] The name of the environment variable to set
-  # @param value [String] The value to set the variable to.  nil deletes the environment variable
-  # @param mode [Symbol] Which operating system mode to use e.g. :posix or :windows.  Use nil to autodetect
+  # @deprecated Use ENV instead
   # @api private
   def set_env(name, value = nil, mode = default_env)
-    case mode
-      when :posix
-        ENV[name] = value
-      when :windows
-        Puppet::Util::Windows::Process.set_environment_variable(name,value)
-      else
-        raise _("Unable to set the environment variable %{name} for mode %{mode}") % { name: name, mode: mode }
-    end
+    ENV[name] = value
   end
   module_function :set_env
 
-  # @param name [Hash] Environment variables to merge into the existing environment.  nil values will remove the variable
-  # @param mode [Symbol] Which operating system mode to use e.g. :posix or :windows.  Use nil to autodetect
+  # @deprecated Use ENV instead
   # @api private
   def merge_environment(env_hash, mode = default_env)
-    case mode
-      when :posix
-        env_hash.each { |name, val| ENV[name.to_s] = val }
-      when :windows
-        env_hash.each do |name, val|
-          Puppet::Util::Windows::Process.set_environment_variable(name.to_s, val)
-        end
-      else
-        raise _("Unable to merge given values into the current environment for mode %{mode}") % { mode: mode }
-    end
+    ENV.merge!(hash.transform_keys(&:to_s))
   end
   module_function :merge_environment
 
   # Run some code with a specific environment.  Resets the environment back to
   # what it was at the end of the code.
-  # Windows can store Unicode chars in the environment as keys or values, but
-  # Ruby's ENV tries to roundtrip them through the local codepage, which can
-  # cause encoding problems - underlying helpers use Windows APIs on Windows
-  # see https://bugs.ruby-lang.org/issues/8822
+  #
+  # @param hash [Hash{String,Symbol => String}] Environment variables to override the current environment.
+  # @param mode [Symbol] ignored
   def withenv(hash, mode = :posix)
-    saved = get_environment(mode)
-    merge_environment(hash, mode)
-    yield
-  ensure
-    if saved
-      clear_environment(mode)
-      merge_environment(saved, mode)
+    saved = ENV.to_hash
+    begin
+      ENV.merge!(hash.transform_keys(&:to_s))
+      yield
+    ensure
+      ENV.replace(saved)
     end
   end
   module_function :withenv

spec/unit/util_spec.rb

@@ -65,6 +65,31 @@ def get_mode(file)
       end
       expect(ENV["FOO"]).to eq(nil)
     end
+
+    it "accepts symbolic keys" do
+      Puppet::Util.withenv(:FOO => "bar") do
+        expect(ENV["FOO"]).to eq("bar")
+      end
+    end
+
+    it "coerces invalid keys to strings" do
+      Puppet::Util.withenv(12345678 => "bar") do
+        expect(ENV["12345678"]).to eq("bar")
+      end
+    end
+
+    it "rejects keys with leading equals" do
+      expect {
+        Puppet::Util.withenv("=foo" => "bar") {}
+      }.to raise_error(Errno::EINVAL, /Invalid argument/)
+    end
+
+    it "includes keys with unicode replacement characters" do
+      Puppet::Util.withenv("foo\uFFFD" => "bar") do
+        expect(ENV).to be_include("foo\uFFFD")
+      end
+    end
+
     it "accepts a unicode key" do
       key = "\u16A0\u16C7\u16BB\u16EB\u16D2\u16E6\u16A6\u16EB\u16A0\u16B1\u16A9\u16A0\u16A2\u16B1\u16EB\u16A0\u16C1\u16B1\u16AA\u16EB\u16B7\u16D6\u16BB\u16B9\u16E6\u16DA\u16B3\u16A2\u16D7"
 
@@ -81,16 +106,21 @@ def get_mode(file)
       end
     end
 
+    it "rejects a non-string value" do
+      expect {
+        Puppet::Util.withenv("reject" => 123) {}
+      }.to raise_error(TypeError, /no implicit conversion of Integer into String/)
+    end
+
     it "accepts a nil value" do
       Puppet::Util.withenv("foo" => nil) do
         expect(ENV["foo"]).to eq(nil)
       end
     end
-
   end
 
   describe "#withenv on POSIX", :unless => Puppet::Util::Platform.windows? do
-    it "should preserve case" do
+    it "compares keys case sensitively" do
       # start with lower case key,
       env_key = SecureRandom.uuid.downcase
 
@@ -115,7 +145,7 @@ def get_mode(file)
   describe "#withenv on Windows", :if => Puppet::Util::Platform.windows? do
     let(:process) { Puppet::Util::Windows::Process }
 
-    it "should ignore case" do
+    it "compares keys case-insensitively" do
       # start with lower case key, ensuring string is not entirely numeric
       env_key = SecureRandom.uuid.downcase + 'a'