[PATCH] wbuf_common: close body proxies on sendfile abort

[PATCH] wbuf_common: close body proxies on sendfile abort

[Eric Wong]

If we're streaming large files and sendfile fails (due to a
client aborting the connection), we need to ensure middleware
proxies are closed to ensure proper logging of a partial request.

This affects users of the "clogger" gem serving static files.

Unfortunately with clogger (or any Rack API-compliant middleware
using "to_path"), we still cannot log the amount of bytes
transferred for a static file.
---
 Pushed to master as commit cf171301f3f8bc030825db7107151f06db9a00d5

 lib/yahns/wbuf_common.rb  |  3 +++
 test/test_serve_static.rb | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/lib/yahns/wbuf_common.rb b/lib/yahns/wbuf_common.rb
index 01e20bb..69fd00d 100644
--- a/lib/yahns/wbuf_common.rb
+++ b/lib/yahns/wbuf_common.rb
@@ -32,6 +32,9 @@ module Yahns::WbufCommon # :nodoc:
             "sf_offset=#@sf_offset sf_count=#@sf_count"
     end while @sf_count > 0
     wbuf_close(client)
+  rescue
+    wbuf_close(client)
+    raise
   end
 
   def wbuf_close_common(client)
diff --git a/test/test_serve_static.rb b/test/test_serve_static.rb
index edd700c..b6875bd 100644
--- a/test/test_serve_static.rb
+++ b/test/test_serve_static.rb
@@ -85,6 +85,67 @@ class TestServeStatic < Testcase
     [ off + 1, sparse ]
   end
 
+  class ToPathClose
+    attr_reader :closed_p
+
+    def initialize(app, tmpdir)
+      @app = app
+      @tmpdir = tmpdir
+      @log = "#@tmpdir/to_path--close"
+      @body = nil
+      @closed_p = false
+    end
+
+    def call(env)
+      s, h, b = @app.call(env)
+      @body = b
+      [ s, h, self ]
+    end
+
+    def each
+      raise "ToPathClose#each should not be called"
+    end
+
+    def to_path
+      File.open(@log, "a") { |fp| fp.write("to_path\n") }
+      "#@tmpdir/sparse"
+    end
+
+    def close
+      File.open(@log, "a") { |fp| fp.write("close\n") }
+      raise "Double close" if @closed_p
+      @closed_p = true
+      nil
+    end
+  end
+
+  def test_aborted_sendfile_closes_opened_path
+    tmpdir = Dir.mktmpdir
+    mksparse(tmpdir)
+    fifo = "#{tmpdir}/to_path--close"
+    assert system("mkfifo", fifo), "mkfifo"
+    err, cfg, host, port = @err, Yahns::Config.new, @srv.addr[3], @srv.addr[1]
+    pid = mkserver(cfg) do
+      cfg.instance_eval do
+        app = Rack::Builder.new do
+          use ToPathClose, tmpdir
+          run Rack::File.new(tmpdir)
+        end
+        app(:rack, app) { listen "#{host}:#{port}" }
+        stderr_path err.path
+      end
+    end
+    c = get_tcp_client(host, port)
+    c.write "GET /sparse HTTP/1.1\r\nHost: example.com\r\n\r\n"
+    assert_equal "to_path\n", File.read(fifo)
+    wait_for_full(c)
+    assert_nil c.close
+    Timeout.timeout(30) { assert_equal "close\n", File.read(fifo) }
+  ensure
+    quit_wait(pid)
+    FileUtils.rm_rf(tmpdir)
+  end
+
   def test_truncated_sendfile
     tmpdir = Dir.mktmpdir
     size, sparse = mksparse(tmpdir)
-- 
EW