Add a timeout for git commands (#692)

jcouball · web-flow · commit 023017b1ee45 · 2024-02-21T18:25:18.000-08:00
* Implement the new timeout feature
Signed-off-by: James Couball &lt;jcouball@yahoo.com&gt;
diff --git a/README.md b/README.md
@@ -11,6 +11,18 @@
 [![Build Status](https://github.com/ruby-git/ruby-git/workflows/CI/badge.svg?branch=master)](https://github.com/ruby-git/ruby-git/actions?query=workflow%3ACI)
 [![Code Climate](https://codeclimate.com/github/ruby-git/ruby-git.png)](https://codeclimate.com/github/ruby-git/ruby-git)
 
+* [Summary](#summary)
+* [v2.0.0 pre-release](#v200-pre-release)
+* [Install](#install)
+* [Major Objects](#major-objects)
+* [Errors Raised By This Gem](#errors-raised-by-this-gem)
+* [Specifying And Handling Timeouts](#specifying-and-handling-timeouts)
+* [Examples](#examples)
+* [Ruby version support policy](#ruby-version-support-policy)
+* [License](#license)
+
+## Summary
+
 The [git gem](https://rubygems.org/gems/git) provides an API that can be used to
 create, read, and manipulate Git repositories by wrapping system calls to the `git`
 command line. The API can be used for working with Git in complex interactions
@@ -140,6 +152,60 @@ rescue Git::TimeoutError => e # Catch the more specific error first!
   puts "Git clone took too long and timed out #{e}"
 rescue Git::Error => e
   puts "Received the following error: #{e}"
+```
+
+## Specifying And Handling Timeouts
+
+The timeout feature was added in git gem version `2.0.0`.
+
+A timeout for git operations can be set either globally or for specific method calls
+that accept a `:timeout` parameter.
+
+The timeout value must be a real, non-negative `Numeric` value that specifies a
+number of seconds a `git` command will be given to complete before being sent a KILL
+signal. This library may hang if the `git` command does not terminate after receiving
+the KILL signal.
+
+When a command times out, a `Git::TimeoutError` is raised.
+
+If the timeout value is `0` or `nil`, no timeout will be enforced.
+
+If a method accepts a `:timeout` parameter and a receives a non-nil value, it will
+override the global timeout value. In this context, a value of `nil` (which is
+usually the default) will use the global timeout value and a value of `0` will turn
+off timeout enforcement for that method call no matter what the global value is.
+
+To set a global timeout, use the `Git.config` object:
+
+```ruby
+Git.config.timeout = nil # a value of nil or 0 means no timeout is enforced
+Git.config.timeout = 1.5 # can be any real, non-negative Numeric interpreted as number of seconds
+```
+
+The global timeout can be overridden for a specific method if the method accepts a
+`:timeout` parameter:
+
+```ruby
+repo_url = 'https://github.com/ruby-git/ruby-git.git'
+Git.clone(repo_url) # Use the global timeout value
+Git.clone(repo_url, timeout: nil) # Also uses the global timeout value
+Git.clone(repo_url, timeout: 0) # Do not enforce a timeout
+Git.clone(repo_url, timeout: 10.5)  # Timeout after 10.5 seconds raising Git::SignaledError
+```
+
+If the command takes too long, a `Git::SignaledError` will be raised:
+
+```ruby
+begin
+  Git.clone(repo_url, timeout: 10)
+rescue Git::TimeoutError => e
+  result = e.result
+  result.class #=> Git::CommandLineResult
+  result.status #=> #<Process::Status: pid 62173 SIGKILL (signal 9)>
+  result.status.timeout? #=> true
+  result.git_cmd # The git command ran as an array of strings
+  result.stdout # The command's output to stdout until it was terminated
+  result.stderr # The command's output to stderr until it was terminated
 end
 ```
 
diff --git a/bin/command_line_test b/bin/command_line_test
@@ -35,10 +35,11 @@ require 'optparse'
 class CommandLineParser
   def initialize
     @option_parser = OptionParser.new
+    @duration = 0
     define_options
   end
 
-  attr_reader :stdout, :stderr, :exitstatus, :signal
+  attr_reader :duration, :stdout, :stderr, :exitstatus, :signal
 
   # Parse the command line arguements returning the options
   #
@@ -84,7 +85,7 @@ class CommandLineParser
     option_parser.separator 'Options:'
     %i[
       define_help_option define_stdout_option define_stderr_option
-      define_exitstatus_option define_signal_option
+      define_exitstatus_option define_signal_option define_duration_option
     ].each { |m| send(m) }
   end
 
@@ -135,6 +136,15 @@ class CommandLineParser
     end
   end
 
+  # Define the duration option
+  # @return [void]
+  # @api private
+  def define_duration_option
+    option_parser.on('--duration=0', 'The number of seconds the command should take') do |duration|
+      @duration = Integer(duration)
+    end
+  end
+
   # Define the help option
   # @return [void]
   # @api private
@@ -176,5 +186,6 @@ options = CommandLineParser.new.parse(*ARGV)
 
 STDOUT.puts options.stdout if options.stdout
 STDERR.puts options.stderr if options.stderr
+sleep options.duration unless options.duration.zero?
 Process.kill(options.signal, Process.pid) if options.signal
 exit(options.exitstatus) if options.exitstatus
diff --git a/git.gemspec b/git.gemspec
@@ -28,7 +28,7 @@ Gem::Specification.new do |s|
   s.requirements = ['git 2.28.0 or greater']
 
   s.add_runtime_dependency 'addressable', '~> 2.8'
-  s.add_runtime_dependency 'process_executer', '~> 0.7'
+  s.add_runtime_dependency 'process_executer', '~> 1.1'
   s.add_runtime_dependency 'rchardet', '~> 1.8'
 
   s.add_development_dependency 'minitar', '~> 0.9'
diff --git a/lib/git.rb b/lib/git.rb
@@ -7,11 +7,13 @@
 require 'git/base'
 require 'git/branch'
 require 'git/branches'
+require 'git/command_line_error'
 require 'git/command_line_result'
 require 'git/command_line'
 require 'git/config'
 require 'git/diff'
 require 'git/encoding_utils'
+require 'git/error'
 require 'git/escaped_path'
 require 'git/failed_error'
 require 'git/git_execute_error'
@@ -24,7 +26,6 @@
 require 'git/repository'
 require 'git/signaled_error'
 require 'git/status'
-require 'git/signaled_error'
 require 'git/stash'
 require 'git/stashes'
 require 'git/timeout_error'
diff --git a/lib/git/command_line.rb b/lib/git/command_line.rb
@@ -166,21 +166,30 @@ def initialize(env, binary_path, global_opts, logger)
     # @param merge [Boolean] whether to merge stdout and stderr in the string returned
     # @param chdir [String] the directory to run the command in
     #
+    # @param timeout [Numeric, nil] the maximum seconds to wait for the command to complete
+    #
+    #   If timeout is zero or nil, the command will not time out. If the command
+    #   times out, it is killed via a SIGKILL signal and `Git::TimeoutError` is raised.
+    #
+    #   If the command does not respond to SIGKILL, it will hang this method.
+    #
     # @return [Git::CommandLineResult] the output of the command
     #
     #   This result of running the command.
     #
     # @raise [ArgumentError] if `args` is not an array of strings
     # @raise [Git::SignaledError] if the command was terminated because of an uncaught signal
     # @raise [Git::FailedError] if the command returned a non-zero exitstatus
+    # @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
+    # @raise [Git::TimeoutError] if the command times out
     #
-    def run(*args, out:, err:, normalize:, chomp:, merge:, chdir: nil)
+    def run(*args, out:, err:, normalize:, chomp:, merge:, chdir: nil, timeout: nil)
       git_cmd = build_git_cmd(args)
       out ||= StringIO.new
       err ||= (merge ? out : StringIO.new)
-      status = execute(git_cmd, out, err, chdir: (chdir || :not_set))
+      status = execute(git_cmd, out, err, chdir: (chdir || :not_set), timeout: timeout)
 
-      process_result(git_cmd, status, out, err, normalize, chomp)
+      process_result(git_cmd, status, out, err, normalize, chomp, timeout)
     end
 
     private
@@ -258,17 +267,24 @@ def raise_pipe_error(git_cmd, pipe_name, pipe)
     #
     # @param cmd [Array<String>] the git command to execute
     # @param chdir [String] the directory to run the command in
+    # @param timeout [Float, Integer, nil] the maximum seconds to wait for the command to complete
+    #
+    #   If timeout is zero of nil, the command will not time out. If the command
+    #   times out, it is killed via a SIGKILL signal and `Git::TimeoutError` is raised.
+    #
+    #   If the command does not respond to SIGKILL, it will hang this method.
     #
     # @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
+    # @raise [Git::TimeoutError] if the command times out
     #
-    # @return [Process::Status] the status of the completed subprocess
+    # @return [ProcessExecuter::Status] the status of the completed subprocess
     #
     # @api private
     #
-    def spawn(cmd, out_writers, err_writers, chdir:)
+    def spawn(cmd, out_writers, err_writers, chdir:, timeout:)
       out_pipe = ProcessExecuter::MonitoredPipe.new(*out_writers, chunk_size: 10_000)
       err_pipe = ProcessExecuter::MonitoredPipe.new(*err_writers, chunk_size: 10_000)
-      ProcessExecuter.spawn(env, *cmd, out: out_pipe, err: err_pipe, chdir: chdir)
+      ProcessExecuter.spawn(env, *cmd, out: out_pipe, err: err_pipe, chdir: chdir, timeout: timeout)
     ensure
       out_pipe.close
       err_pipe.close
@@ -313,11 +329,12 @@ def writers(out, err)
     #
     # @api private
     #
-    def process_result(git_cmd, status, out, err, normalize, chomp)
+    def process_result(git_cmd, status, out, err, normalize, chomp, timeout)
       out_str, err_str = post_process_all([out, err], normalize, chomp)
       logger.info { "#{git_cmd} exited with status #{status}" }
       logger.debug { "stdout:\n#{out_str.inspect}\nstderr:\n#{err_str.inspect}" }
       Git::CommandLineResult.new(git_cmd, status, out_str, err_str).tap do |result|
+        raise Git::TimeoutError.new(result, timeout) if status.timeout?
         raise Git::SignaledError.new(result) if status.signaled?
         raise Git::FailedError.new(result) unless status.success?
       end
@@ -329,14 +346,23 @@ def process_result(git_cmd, status, out, err, normalize, chomp)
     # @param out [#write] the object to write stdout to
     # @param err [#write] the object to write stderr to
     # @param chdir [String] the directory to run the command in
+    # @param timeout [Float, Integer, nil] the maximum seconds to wait for the command to complete
+    #
+    #   If timeout is zero of nil, the command will not time out. If the command
+    #   times out, it is killed via a SIGKILL signal and `Git::TimeoutError` is raised.
+    #
+    #   If the command does not respond to SIGKILL, it will hang this method.
+    #
+    # @raise [Git::GitExecuteError] if an exception was raised while collecting subprocess output
+    # @raise [Git::TimeoutError] if the command times out
     #
     # @return [Git::CommandLineResult] the result of the command to return to the caller
     #
     # @api private
     #
-    def execute(git_cmd, out, err, chdir:)
+    def execute(git_cmd, out, err, chdir:, timeout:)
       out_writers, err_writers = writers(out, err)
-      spawn(git_cmd, out_writers, err_writers, chdir: chdir)
+      spawn(git_cmd, out_writers, err_writers, chdir: chdir, timeout: timeout)
     end
   end
 end
diff --git a/lib/git/config.rb b/lib/git/config.rb
@@ -2,11 +2,12 @@ module Git
 
   class Config
 
-    attr_writer :binary_path, :git_ssh
+    attr_writer :binary_path, :git_ssh, :timeout
 
     def initialize
       @binary_path = nil
       @git_ssh = nil
+      @timeout = nil
     end
 
     def binary_path
@@ -17,6 +18,9 @@ def git_ssh
       @git_ssh || ENV['GIT_SSH']
     end
 
+    def timeout
+      @timeout || (ENV['GIT_TIMEOUT'] && ENV['GIT_TIMEOUT'].to_i)
+    end
   end
 
 end
diff --git a/lib/git/lib.rb b/lib/git/lib.rb
@@ -115,7 +115,7 @@ def clone(repository_url, directory, opts = {})
       arr_opts << repository_url
       arr_opts << clone_dir
 
-      command('clone', *arr_opts)
+      command('clone', *arr_opts, timeout: opts[:timeout])
 
       return_base_opts_from_clone(clone_dir, opts)
     end
@@ -1191,8 +1191,48 @@ def command_line
         Git::CommandLine.new(env_overrides, Git::Base.config.binary_path, global_opts, @logger)
     end
 
-    def command(*args, out: nil, err: nil, normalize: true, chomp: true, merge: false, chdir: nil)
-      result = command_line.run(*args, out: out, err: err, normalize: normalize, chomp: chomp, merge: merge, chdir: chdir)
+    # Runs a git command and returns the output
+    #
+    # @param args [Array] the git command to run and its arguments
+    #
+    #   This should exclude the 'git' command itself and global options.
+    #
+    #   For example, to run `git log --pretty=oneline`, you would pass `['log',
+    #   '--pretty=oneline']`
+    #
+    # @param out [String, nil] the path to a file or an IO to write the command's
+    #   stdout to
+    #
+    # @param err [String, nil] the path to a file or an IO to write the command's
+    #   stdout to
+    #
+    # @param normalize [Boolean] true to normalize the output encoding
+    #
+    # @param chomp [Boolean] true to remove trailing newlines from the output
+    #
+    # @param merge [Boolean] true to merge stdout and stderr
+    #
+    # @param chdir [String, nil] the directory to run the command in
+    #
+    # @param timeout [Numeric, nil] the maximum time to wait for the command to
+    # complete
+    #
+    # @see Git::CommandLine#run
+    #
+    # @return [String] the command's stdout (or merged stdout and stderr if `merge`
+    # is true)
+    #
+    # @raise [Git::GitExecuteError] if the command fails
+    #
+    #   The exception's `result` attribute is a {Git::CommandLineResult} which will
+    #   contain the result of the command including the exit status, stdout, and
+    #   stderr.
+    #
+    # @api private
+    #
+    def command(*args, out: nil, err: nil, normalize: true, chomp: true, merge: false, chdir: nil, timeout: nil)
+      timeout = timeout || Git.config.timeout
+      result = command_line.run(*args, out: out, err: err, normalize: normalize, chomp: chomp, merge: merge, chdir: chdir, timeout: timeout)
       result.stdout
     end
 
diff --git a/tests/units/test_command_line.rb b/tests/units/test_command_line.rb
diff --git a/tests/units/test_git_clone.rb b/tests/units/test_git_clone.rb
