From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@80x24.org>
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net
X-Spam-Level: 
X-Spam-ASN:  
X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00
	shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2
Received: from localhost (dcvr.yhbt.net [127.0.0.1])
	by dcvr.yhbt.net (Postfix) with ESMTP id 23441211B4;
	Sat,  5 Jan 2019 20:51:20 +0000 (UTC)
Date: Sat, 5 Jan 2019 20:51:20 +0000
From: Eric Wong <e@80x24.org>
To: yahns-public@yhbt.net
Subject: [RFC] exec_cgi: add timeout parameter
Message-ID: <20190105205120.GA11253@dcvr>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
List-Id: <yahns-public.yhbt.net>

This may be useful for longer responses which are CPU-intensive,
which makes RLIMIT_CPU inappropriate.
---
 I hate adding new parameters/APIs, and I don't think this API
 is sufficient, since I can imagine different timeouts for:

 1) initial header
 2) each body read
 3) overall response time
 ...

 Anyways, I doubt anybody else cares for this module, so it
 won't be in the next release.  But I've been taking time to
 improve cgit <https://80x24.org/cgit.git> (upstream:
 <https://git.zx2c4.com/cgit> and using ExecCGI to launch it.

 extras/exec_cgi.rb           | 31 ++++++++++++++++++++++---------
 test/helper.rb               |  3 ++-
 test/test_extras_exec_cgi.rb | 24 ++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/extras/exec_cgi.rb b/extras/exec_cgi.rb
index 8a1939d..848975e 100644
--- a/extras/exec_cgi.rb
+++ b/extras/exec_cgi.rb
@@ -23,12 +23,13 @@
 #
 class ExecCgi
   class MyIO
-    attr_writer :my_pid
+    attr_accessor :my_pid
     attr_writer :body_tip
     attr_reader :rd
 
-    def initialize(rd)
+    def initialize(rd, timeout)
       @rd = rd
+      @timeout = timeout
     end
 
     def each
@@ -37,7 +38,10 @@ def each
 
       case tmp = @rd.read_nonblock(8192, buf, exception: false)
       when :wait_readable
-        @rd.wait_readable
+        unless @rd.wait_readable(@timeout[1])
+          Process.kill(@timeout[0], @my_pid)
+          break
+        end
       when nil
         break
       else # String
@@ -93,6 +97,7 @@ def initialize(*args)
     File.executable?(args[0]) or
       raise ArgumentError, "#{args[0]} is not executable"
     @opts = Hash === args[-1] ? args.pop : {}
+    @timeout = @opts.delete(:timeout) || []
   end
 
   # Calls the app
@@ -103,19 +108,27 @@ def call(env)
     env.each { |key,val| cgi_env[key] = val if key =~ /\AHTTP_/ }
 
     rd, wr = IO.pipe
-    io = MyIO.new(rd)
+    io = MyIO.new(rd, @timeout)
     errbody = io
     errbody.my_pid = spawn(cgi_env.merge!(@env), *@args,
                            @opts.merge(out: wr, close_others: true))
     wr.close
 
     begin
-      head = rd.readpartial(8192)
-      until head =~ /\r?\n\r?\n/
-        tmp = rd.readpartial(8192)
-        head << tmp
+      head = ''.b
+      tmp = ''.b
+      case rd.read_nonblock(8192, tmp, exception: false)
+      when :wait_readable
+        unless rd.wait_readable(@timeout[1])
+          Process.kill(@timeout[0], errbody.my_pid)
+        end
+      when nil
         tmp.clear
-      end
+        raise EOFError, 'timed out or EOF reached', []
+        break
+      else
+        head << tmp
+      end until head =~ /\r?\n\r?\n/
       head, body = head.split(/\r?\n\r?\n/, 2)
       io.body_tip = body
 
diff --git a/test/helper.rb b/test/helper.rb
index 550a0f1..edca30f 100644
--- a/test/helper.rb
+++ b/test/helper.rb
@@ -130,7 +130,8 @@ def cloexec_pipe
 
 def require_exec(cmd)
   ENV["PATH"].split(/:/).each do |path|
-    return true if File.executable?("#{path}/#{cmd}")
+    bin = "#{path}/#{cmd}"
+    return bin if File.executable?(bin)
   end
   skip "#{cmd} not found in PATH"
   false
diff --git a/test/test_extras_exec_cgi.rb b/test/test_extras_exec_cgi.rb
index 426409d..f4c022c 100644
--- a/test/test_extras_exec_cgi.rb
+++ b/test/test_extras_exec_cgi.rb
@@ -202,4 +202,28 @@ def test_rlimit_options
   ensure
     quit_wait(pid)
   end
+
+  def test_timeout
+    err, cfg, host, port = @err, Yahns::Config.new, @srv.addr[3], @srv.addr[1]
+    tout = 1
+    opts = { timeout: [:TERM, 0.5 ] }
+    bin = require_exec('sleep')
+    cmd = [ bin, '10', opts ]
+    pid = mkserver(cfg) do
+      require './extras/exec_cgi'
+      cfg.instance_eval do
+        stack = Rack::ContentLength.new(Rack::Chunked.new(ExecCgi.new(*cmd)))
+        app(:rack, stack) { listen "#{host}:#{port}" }
+        stderr_path err.path
+        worker_processes 1
+      end
+    end
+    c = get_tcp_client(host, port)
+    c.write "GET / HTTP/1.0\r\n\r\n"
+    assert_same c, c.wait(tout + 1)
+    assert_match %r{ 500 Internal Server Error\b}, c.readpartial(4096)
+    c.close
+  ensure
+    quit_wait(pid)
+  end
 end
-- 
EW