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