Skip to content

Commit

Permalink
Merge pull request #221 from chef/safe
Browse files Browse the repository at this point in the history
Signed-off-by: Tim Smith <tsmith@chef.io>
  • Loading branch information
tas50 authored Sep 10, 2020
2 parents 50ae416 + e722fe8 commit 8db3037
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 23 deletions.
2 changes: 1 addition & 1 deletion lib/mixlib/shellout.rb
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def format_for_exception
# running or died without setting an exit status (e.g., terminated by
# `kill -9`).
def exitstatus
@status && @status.exitstatus
@status&.exitstatus
end

# Run the command, writing the command's standard out and standard error
Expand Down
4 changes: 2 additions & 2 deletions lib/mixlib/shellout/unix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,11 @@ def reap_errant_child
return if attempt_reap

@terminate_reason = "Command exceeded allowed execution time, process terminated"
logger.error("Command exceeded allowed execution time, sending TERM") if logger
logger&.error("Command exceeded allowed execution time, sending TERM")
Process.kill(:TERM, child_pgid)
sleep 3
attempt_reap
logger.error("Command exceeded allowed execution time, sending KILL") if logger
logger&.error("Command exceeded allowed execution time, sending KILL")
Process.kill(:KILL, child_pgid)
reap

Expand Down
12 changes: 4 additions & 8 deletions lib/mixlib/shellout/windows.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def run_command
# Start the process
#
process, profile, token = Process.create3(create_process_args)
logger.debug(format_process(process, app_name, command_line, timeout)) if logger
logger&.debug(format_process(process, app_name, command_line, timeout))
begin
# Start pushing data into input
stdin_write << input if input
Expand Down Expand Up @@ -124,7 +124,7 @@ def run_command
kill_process_tree(process.process_id, wmi, logger)
Process.kill(:KILL, process.process_id)
rescue SystemCallError
logger.warn("Failed to kill timed out process #{process.process_id}") if logger
logger&.warn("Failed to kill timed out process #{process.process_id}")
end

raise Mixlib::ShellOut::CommandTimeout, [
Expand Down Expand Up @@ -398,20 +398,16 @@ def kill_process_tree(pid, wmi, logger)

def kill_process(instance, logger)
child_pid = instance.wmi_ole_object.processid
if logger
logger.debug([
logger&.debug([
"killing child process #{child_pid}::",
"#{instance.wmi_ole_object.Name} of parent #{pid}",
].join)
end
Process.kill(:KILL, instance.wmi_ole_object.processid)
rescue SystemCallError
if logger
logger.debug([
logger&.debug([
"Failed to kill child process #{child_pid}::",
"#{instance.wmi_ole_object.Name} of parent #{pid}",
].join)
end
end

def format_process(process, app_name, command_line, timeout)
Expand Down
15 changes: 6 additions & 9 deletions lib/mixlib/shellout/windows/core_ext.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

# Add new constants for Logon
module Process::Constants
private

LOGON32_LOGON_INTERACTIVE = 0x00000002
LOGON32_LOGON_BATCH = 0x00000004
Expand Down Expand Up @@ -148,15 +147,13 @@ def create3(args)
si_hash = {}

# If the startup_info key is present, validate its subkeys
if hash["startup_info"]
hash["startup_info"].each do |key, val|
key = key.to_s.downcase
unless valid_si_keys.include?(key)
raise ArgumentError, "invalid startup_info key '#{key}'"
end

si_hash[key] = val
hash["startup_info"]&.each do |key, val|
key = key.to_s.downcase
unless valid_si_keys.include?(key)
raise ArgumentError, "invalid startup_info key '#{key}'"
end

si_hash[key] = val
end

# The +command_line+ key is mandatory unless the +app_name+ key
Expand Down
6 changes: 3 additions & 3 deletions spec/mixlib/shellout_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ def shell_out_cmd
end

after do
@test_file.close if @test_file
@test_file&.close
end

let(:ruby_code) { "fd = File.for_fd(#{@test_file.to_i}) rescue nil; if fd; fd.seek(0); puts fd.read(5); end" }
Expand Down Expand Up @@ -1380,7 +1380,7 @@ def ruby_wo_shell(code)
end

context "with subprocess writing lots of data to both stdout and stderr" do
let(:expected_output_with) { lambda { |chr| (chr * 20_000) + (LINE_ENDING).to_s + (chr * 20_000) + (LINE_ENDING).to_s } }
let(:expected_output_with) { lambda { |chr| (chr * 20_000) + LINE_ENDING.to_s + (chr * 20_000) + LINE_ENDING.to_s } }

context "when writing to STDOUT first" do
let(:ruby_code) { %q{puts "f" * 20_000; STDERR.puts "u" * 20_000; puts "f" * 20_000; STDERR.puts "u" * 20_000} }
Expand Down Expand Up @@ -1543,7 +1543,7 @@ def ruby_wo_shell(code)
let(:options) { { user: user } }

it "should run as specified user" do
expect(running_user).to eql((user).to_s)
expect(running_user).to eql(user.to_s)
end
end
end
Expand Down

0 comments on commit 8db3037

Please sign in to comment.