From e6613a101de325caab78a1b392d0a149d02e270c Mon Sep 17 00:00:00 2001 From: Alex Slynko Date: Wed, 20 Aug 2014 22:47:18 +0100 Subject: Fix setting Path for Windows --- lib/chef/provider/env/windows.rb | 14 +++++++++++++- spec/unit/provider/env/windows_spec.rb | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/chef/provider/env/windows.rb b/lib/chef/provider/env/windows.rb index f73cb42f7e..990ab178af 100644 --- a/lib/chef/provider/env/windows.rb +++ b/lib/chef/provider/env/windows.rb @@ -33,7 +33,10 @@ class Chef end obj.variablevalue = @new_resource.value obj.put_ - ENV[@new_resource.key_name] = @new_resource.value + value = @new_resource.value + value = expand_path(value) if @new_resource.key_name.upcase == 'PATH' + ENV[@new_resource.key_name] = value + broadcast_env_change end @@ -71,6 +74,15 @@ class Chef flags = SMTO_BLOCK | SMTO_ABORTIFHUNG | SMTO_NOTIMEOUTIFNOTHUNG SendMessageTimeoutA(HWND_BROADCAST, WM_SETTINGCHANGE, 0, FFI::MemoryPointer.from_string('Environment').address, flags, 5000, nil) end + + private + + def expand_path(path) + system_vars = %w(HomeDrive HomePath ProgramFiles SystemDirectory SystemDrive SystemRoot Temp Tmp UserProfile WinDir) + system_vars.each_with_object(path) do |variable, new_path| + new_path.gsub!(/%#{variable}%/i, ENV[variable]) if ENV[variable] + end + end end end end diff --git a/spec/unit/provider/env/windows_spec.rb b/spec/unit/provider/env/windows_spec.rb index 329448ac05..8f72d19b29 100644 --- a/spec/unit/provider/env/windows_spec.rb +++ b/spec/unit/provider/env/windows_spec.rb @@ -52,6 +52,23 @@ describe Chef::Provider::Env::Windows, :windows_only do @provider.action_modify expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foobar') end + + describe "for PATH" do + before do + stub_const('ENV', {'SystemRoot' => 'D:\\Windows', 'Path' => 'D:\\Path'}) + @new_resource = Chef::Resource::Env.new('Path') + @new_resource.value("%SystemRoot%") + + @provider = Chef::Provider::Env::Windows.new(@new_resource, @run_context) + @provider.stub(:env_obj).and_return(double('null object').as_null_object) + end + + it "replaces Windows system variables" do + @provider.should_receive(:compare_value).and_return(true) + @provider.action_modify + expect(ENV['Path']).to eql('D:\\Windows') + end + end end describe "action_delete" do -- cgit v1.2.1 From 3f01d0a3ad058e3b21244d205ac1a7ad793875dc Mon Sep 17 00:00:00 2001 From: Jay Mundrawala Date: Tue, 9 Sep 2014 17:44:36 -0700 Subject: Use windows api to expand path. Cleaned up tests --- lib/chef/provider/env/windows.rb | 10 +-- lib/chef/win32/api/system.rb | 9 +++ spec/functional/resource/env_spec.rb | 46 ++++++++++++++ spec/unit/provider/env/windows_spec.rb | 111 +++++++++++++++++++-------------- 4 files changed, 125 insertions(+), 51 deletions(-) diff --git a/lib/chef/provider/env/windows.rb b/lib/chef/provider/env/windows.rb index 990ab178af..79abda8708 100644 --- a/lib/chef/provider/env/windows.rb +++ b/lib/chef/provider/env/windows.rb @@ -33,10 +33,9 @@ class Chef end obj.variablevalue = @new_resource.value obj.put_ - value = @new_resource.value + value = @new_resource.value value = expand_path(value) if @new_resource.key_name.upcase == 'PATH' ENV[@new_resource.key_name] = value - broadcast_env_change end @@ -78,10 +77,11 @@ class Chef private def expand_path(path) - system_vars = %w(HomeDrive HomePath ProgramFiles SystemDirectory SystemDrive SystemRoot Temp Tmp UserProfile WinDir) - system_vars.each_with_object(path) do |variable, new_path| - new_path.gsub!(/%#{variable}%/i, ENV[variable]) if ENV[variable] + buf = 0.chr * 32 * (1 << 10) # http://msdn.microsoft.com/en-us/library/windows/desktop/ms724265%28v=vs.85%29.aspx + if ExpandEnvironmentStringsA(path, buf, buf.length) == 0 + Chef::ReservedNames::Win32::Error.raise! end + buf.strip end end end diff --git a/lib/chef/win32/api/system.rb b/lib/chef/win32/api/system.rb index a58c0f38f4..d57699acb4 100644 --- a/lib/chef/win32/api/system.rb +++ b/lib/chef/win32/api/system.rb @@ -200,6 +200,15 @@ LRESULT WINAPI SendMessageTimeout( safe_attach_function :SendMessageTimeoutW, [:HWND, :UINT, :WPARAM, :LPARAM, :UINT, :UINT, :PDWORD_PTR], :LRESULT safe_attach_function :SendMessageTimeoutA, [:HWND, :UINT, :WPARAM, :LPARAM, :UINT, :UINT, :PDWORD_PTR], :LRESULT +=begin +DWORD WINAPI ExpandEnvironmentStrings( + _In_ LPCTSTR lpSrc, + _Out_opt_ LPTSTR lpDst, + _In_ DWORD nSize +); +=end + safe_attach_function :ExpandEnvironmentStringsW, [:pointer, :pointer, :DWORD], :DWORD + safe_attach_function :ExpandEnvironmentStringsA, [:pointer, :pointer, :DWORD], :DWORD end end end diff --git a/spec/functional/resource/env_spec.rb b/spec/functional/resource/env_spec.rb index 9002d4b727..24fe5e1dff 100755 --- a/spec/functional/resource/env_spec.rb +++ b/spec/functional/resource/env_spec.rb @@ -24,6 +24,8 @@ describe Chef::Resource::Env, :windows_only do let(:chef_env_test_mixed_case) { 'chefENVtest' } let(:env_value1) { 'value1' } let(:env_value2) { 'value2' } + + let(:env_value_expandable) { '%SystemRoot%' } let(:test_run_context) { node = Chef::Node.new node.default['platform'] = 'windows' @@ -71,6 +73,14 @@ describe Chef::Resource::Env, :windows_only do test_resource.run_action(:create) expect(ENV[chef_env_test_lower_case]).to eq(env_value2) end + + it 'should not expand environment variables if the variable is not PATH' do + expect(ENV[chef_env_test_lower_case]).to eq(nil) + test_resource.key_name(chef_env_test_lower_case) + test_resource.value(env_value_expandable) + test_resource.run_action(:create) + expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable) + end end context "when the modify action is invoked" do @@ -102,6 +112,42 @@ describe Chef::Resource::Env, :windows_only do test_resource.run_action(:modify) expect(ENV[chef_env_test_lower_case]).to eq(env_value2) end + + it 'should not expand environment variables if the variable is not PATH' do + test_resource.key_name(chef_env_test_lower_case) + test_resource.value(env_value1) + test_resource.run_action(:create) + expect(ENV[chef_env_test_lower_case]).to eq(env_value1) + test_resource.value(env_value_expandable) + test_resource.run_action(:modify) + expect(ENV[chef_env_test_lower_case]).to eq(env_value_expandable) + end + + context 'when using PATH' do + let(:random_name) { Time.now.to_i } + let(:env_val) { "#{env_value_expandable}_#{random_name}"} + let(:path_before) { test_resource.provider_for_action(test_resource.action).env_value('PATH') } + + it 'should expand PATH' do + path_before.should_not include(env_val) + test_resource.key_name('PATH') + test_resource.value("#{path_before};#{env_val}") + test_resource.run_action(:create) + ENV['PATH'].should_not include(env_val) + ENV['PATH'].should include("#{random_name}") + end + + after(:each) do + # cleanup so we don't flood the path + test_resource.key_name('PATH') + test_resource.value(path_before) + test_resource.run_action(:create) + if test_resource.provider_for_action(test_resource.action).env_value('PATH') != path_before + raise 'Failed to cleanup after ourselves' + end + end + end + end context "when the delete action is invoked" do diff --git a/spec/unit/provider/env/windows_spec.rb b/spec/unit/provider/env/windows_spec.rb index 8f72d19b29..2ea137c1d9 100644 --- a/spec/unit/provider/env/windows_spec.rb +++ b/spec/unit/provider/env/windows_spec.rb @@ -19,66 +19,85 @@ require 'spec_helper' describe Chef::Provider::Env::Windows, :windows_only do - before do - @node = Chef::Node.new - @events = Chef::EventDispatch::Dispatcher.new - @run_context = Chef::RunContext.new(@node, {}, @events) - @new_resource = Chef::Resource::Env.new("CHEF_WINDOWS_ENV_TEST") - @new_resource.value("foo") - @provider = Chef::Provider::Env::Windows.new(@new_resource, @run_context) - @provider.stub(:env_obj).and_return(double('null object').as_null_object) - end - - describe "action_create" do - before do - ENV.delete('CHEF_WINDOWS_ENV_TEST') - @provider.key_exists = false - end + let(:node) { Chef::Node.new } + let(:events) {Chef::EventDispatch::Dispatcher.new } + let(:run_context) { Chef::RunContext.new(node, {}, events) } - it "should update the ruby ENV object when it creates the key" do - @provider.action_create - expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foo') - end - end + context 'when environment variable is not PATH' do + let(:new_resource) { + new_resource = Chef::Resource::Env.new("CHEF_WINDOWS_ENV_TEST") + new_resource.value("foo") + new_resource + } + let(:provider) { + provider = Chef::Provider::Env::Windows.new(new_resource, run_context) + provider.stub(:env_obj).and_return(double('null object').as_null_object) + provider + } - describe "action_modify" do - before do - ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo' - end + describe "action_create" do + before do + ENV.delete('CHEF_WINDOWS_ENV_TEST') + provider.key_exists = false + end - it "should update the ruby ENV object when it updates the value" do - @provider.should_receive(:compare_value).and_return(true) - @new_resource.value("foobar") - @provider.action_modify - expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foobar') + it "should update the ruby ENV object when it creates the key" do + provider.action_create + expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foo') + end end - describe "for PATH" do + describe "action_modify" do before do - stub_const('ENV', {'SystemRoot' => 'D:\\Windows', 'Path' => 'D:\\Path'}) - @new_resource = Chef::Resource::Env.new('Path') - @new_resource.value("%SystemRoot%") + ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo' + end - @provider = Chef::Provider::Env::Windows.new(@new_resource, @run_context) - @provider.stub(:env_obj).and_return(double('null object').as_null_object) + it "should update the ruby ENV object when it updates the value" do + provider.should_receive(:compare_value).and_return(true) + new_resource.value("foobar") + provider.action_modify + expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql('foobar') end - it "replaces Windows system variables" do - @provider.should_receive(:compare_value).and_return(true) - @provider.action_modify - expect(ENV['Path']).to eql('D:\\Windows') + describe "action_delete" do + before do + ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo' + end + + it "should update the ruby ENV object when it deletes the key" do + provider.action_delete + expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql(nil) + end end end end - describe "action_delete" do - before do - ENV['CHEF_WINDOWS_ENV_TEST'] = 'foo' - end + context 'when environment is PATH' do + describe "for PATH" do + let(:system_root) {'%SystemRoot%'} + let(:system_root_value) { 'D:\Windows' } + let(:new_resource) { + new_resource = Chef::Resource::Env.new('PATH') + new_resource.value(system_root) + new_resource + } + let(:provider) { + provider = Chef::Provider::Env::Windows.new(new_resource, run_context) + provider.stub(:env_obj).and_return(double('null object').as_null_object) + provider + } + + before do + stub_const('ENV', {'PATH' => ''}) + end - it "should update the ruby ENV object when it deletes the key" do - @provider.action_delete - expect(ENV['CHEF_WINDOWS_ENV_TEST']).to eql(nil) + it "replaces Windows system variables" do + provider.should_receive(:compare_value).and_return(true) + provider.should_receive(:expand_path).with(system_root).and_return(system_root_value) + provider.action_modify + expect(ENV['PATH']).to eql(system_root_value) + end end + end end -- cgit v1.2.1 From c3189660fa38ccc3d0c3b475060bfae3e9c58e99 Mon Sep 17 00:00:00 2001 From: Jay Mundrawala Date: Thu, 11 Sep 2014 14:36:24 -0700 Subject: Moved windows environment helper functions to their own module --- lib/chef/mixin/windows_env_helper.rb | 56 ++++++++++++++++++++++++++++++++++++ lib/chef/provider/env/windows.rb | 25 ++-------------- 2 files changed, 58 insertions(+), 23 deletions(-) create mode 100644 lib/chef/mixin/windows_env_helper.rb diff --git a/lib/chef/mixin/windows_env_helper.rb b/lib/chef/mixin/windows_env_helper.rb new file mode 100644 index 0000000000..490b235065 --- /dev/null +++ b/lib/chef/mixin/windows_env_helper.rb @@ -0,0 +1,56 @@ +# +# Author:: Adam Edwards () +# Copyright:: Copyright (c) 2013 Opscode, Inc. +# License:: Apache License, Version 2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + + +require 'chef/exceptions' +require 'chef/platform/query_helpers' +require 'chef/win32/error' if Chef::Platform.windows? +require 'chef/win32/api/system' if Chef::Platform.windows? + +class Chef + module Mixin + module WindowsEnvHelper + + if Chef::Platform.windows? + include Chef::ReservedNames::Win32::API::System + end + + #see: http://msdn.microsoft.com/en-us/library/ms682653%28VS.85%29.aspx + HWND_BROADCAST = 0xffff + WM_SETTINGCHANGE = 0x001A + SMTO_BLOCK = 0x0001 + SMTO_ABORTIFHUNG = 0x0002 + SMTO_NOTIMEOUTIFNOTHUNG = 0x0008 + + def broadcast_env_change + flags = SMTO_BLOCK | SMTO_ABORTIFHUNG | SMTO_NOTIMEOUTIFNOTHUNG + SendMessageTimeoutA(HWND_BROADCAST, WM_SETTINGCHANGE, 0, FFI::MemoryPointer.from_string('Environment').address, flags, 5000, nil) + end + + def expand_path(path) + # http://msdn.microsoft.com/en-us/library/windows/desktop/ms724265%28v=vs.85%29.aspx + # Max size of env block on windows is 32k + buf = 0.chr * 32 * 1024 + if ExpandEnvironmentStringsA(path, buf, buf.length) == 0 + Chef::ReservedNames::Win32::Error.raise! + end + buf.strip + end + end + end +end diff --git a/lib/chef/provider/env/windows.rb b/lib/chef/provider/env/windows.rb index 79abda8708..572ec5c633 100644 --- a/lib/chef/provider/env/windows.rb +++ b/lib/chef/provider/env/windows.rb @@ -16,13 +16,13 @@ # limitations under the License. # -require 'chef/win32/api/system' if RUBY_PLATFORM =~ /mswin|mingw32|windows/ +require 'chef/mixin/windows_env_helper' class Chef class Provider class Env class Windows < Chef::Provider::Env - include Chef::ReservedNames::Win32::API::System if RUBY_PLATFORM =~ /mswin|mingw32|windows/ + include Chef::Mixin::WindowsEnvHelper def create_env obj = env_obj(@new_resource.key_name) @@ -62,27 +62,6 @@ class Chef end end - #see: http://msdn.microsoft.com/en-us/library/ms682653%28VS.85%29.aspx - HWND_BROADCAST = 0xffff - WM_SETTINGCHANGE = 0x001A - SMTO_BLOCK = 0x0001 - SMTO_ABORTIFHUNG = 0x0002 - SMTO_NOTIMEOUTIFNOTHUNG = 0x0008 - - def broadcast_env_change - flags = SMTO_BLOCK | SMTO_ABORTIFHUNG | SMTO_NOTIMEOUTIFNOTHUNG - SendMessageTimeoutA(HWND_BROADCAST, WM_SETTINGCHANGE, 0, FFI::MemoryPointer.from_string('Environment').address, flags, 5000, nil) - end - - private - - def expand_path(path) - buf = 0.chr * 32 * (1 << 10) # http://msdn.microsoft.com/en-us/library/windows/desktop/ms724265%28v=vs.85%29.aspx - if ExpandEnvironmentStringsA(path, buf, buf.length) == 0 - Chef::ReservedNames::Win32::Error.raise! - end - buf.strip - end end end end -- cgit v1.2.1