From b079c1d346cba2d169006227cee8e9fa7fdab213 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 27 Apr 2016 00:27:00 +0000 Subject: test_proxy_pass: test for auto chunking on 1.0 backends These are followups to the following two commits: * commit d16326723d ("proxy_http_response: fix non-terminated fast responses, too") * commit 8c9f33a539 ("proxy_http_response: workaround non-terminated backends") --- test/test_proxy_pass.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/test_proxy_pass.rb b/test/test_proxy_pass.rb index 448e480..c938976 100644 --- a/test/test_proxy_pass.rb +++ b/test/test_proxy_pass.rb @@ -586,6 +586,21 @@ class TestProxyPass < Testcase assert_match %r{\AHTTP/1\.1 200 OK\r\n}, res assert_match %r{\r\n\r\neof-body-slow\z}, res s.close + + # we auto-chunk on 1.1 requests and 1.0 backends + %w(eof-body-slow eof-body-fast).each do |x| + s = TCPSocket.new(host, port) + s.write("GET /#{x} HTTP/1.1\r\nHost: example.com\r\n\r\n") + res = ''.dup + res << s.readpartial(512) until res =~ /0\r\n\r\n\z/ + s.close + head, body = res.split("\r\n\r\n", 2) + head = head.split("\r\n") + assert_equal 'HTTP/1.1 200 OK', head[0] + assert head.include?('Connection: keep-alive') + assert head.include?('Transfer-Encoding: chunked') + assert_match %r{\Ad\r\n#{x}\r\n0\r\n\r\n\z}, body + end end end -- cgit v1.2.3-24-ge0c7 From 395ed76a9916c850749a65d2a9040e39b803414d Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 27 Apr 2016 00:27:01 +0000 Subject: wbuf: drop persistence if writing to client fails We cannot maintain a persistent connection to a client if writing to the client fails; so we can't proceed to let the app hijack the response. This may happen in the unlikely case where a response header needs to be buffered with a Wbuf (and the app uses response hijacking). --- lib/yahns/wbuf_common.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/yahns/wbuf_common.rb b/lib/yahns/wbuf_common.rb index c51050b..ee18218 100644 --- a/lib/yahns/wbuf_common.rb +++ b/lib/yahns/wbuf_common.rb @@ -38,6 +38,7 @@ module Yahns::WbufCommon # :nodoc: end while @sf_count > 0 wbuf_close(client) rescue + @wbuf_persist = false # ensure a hijack response is not called wbuf_close(client) raise end -- cgit v1.2.3-24-ge0c7 From 10aa29fa2c8fc4fdff8554eab24be07e6885aa50 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 27 Apr 2016 00:27:02 +0000 Subject: proxy_http_response: cleanup: avoid redundant setting of "alive" We already check for the truthiness of "alive" in the "if" statement, so re-setting is pointless. --- lib/yahns/proxy_http_response.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/yahns/proxy_http_response.rb b/lib/yahns/proxy_http_response.rb index c8a2a42..e551323 100644 --- a/lib/yahns/proxy_http_response.rb +++ b/lib/yahns/proxy_http_response.rb @@ -101,7 +101,6 @@ module Yahns::HttpResponse # :nodoc: # but the backend does not terminate properly if alive && ! term && (env['HTTP_VERSION'] == 'HTTP/1.1'.freeze) res << "Transfer-Encoding: chunked\r\n".freeze - alive = true end res << (alive ? "Connection: keep-alive\r\n\r\n".freeze : "Connection: close\r\n\r\n".freeze) -- cgit v1.2.3-24-ge0c7 From 40873caf4a71a56332d32e7f4086bf49ca6e3916 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 27 Apr 2016 00:27:03 +0000 Subject: proxy_http_response: do not persist upstream on slow clients For slow clients, we want to be able to drop the connection to the upstream as soon as we are done buffering and not waste resources by leaving it in an :ignore state. We also need to remember the client for the fdmap to prevent shutdowns. Ugh, this is really hard to test locally. --- lib/yahns/proxy_http_response.rb | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/yahns/proxy_http_response.rb b/lib/yahns/proxy_http_response.rb index e551323..3462e40 100644 --- a/lib/yahns/proxy_http_response.rb +++ b/lib/yahns/proxy_http_response.rb @@ -56,6 +56,7 @@ module Yahns::HttpResponse # :nodoc: :wait_readable # self remains in :ignore, wait on upstream end + # start streaming the response once upstream is done sending headers to us. # returns :wait_readable if we need to read more from req_res # returns :ignore if we yield control to the client(self) # returns nil if completely done @@ -177,9 +178,9 @@ module Yahns::HttpResponse # :nodoc: end end - return proxy_busy_mod_done(alive) unless wbuf - req_res.resbuf = wbuf - proxy_busy_mod_blocked(wbuf, wbuf.busy) + # all done reading response from upstream, req_res will be discarded + # when we return nil: + wbuf ? proxy_busy_mod_blocked(wbuf, wbuf.busy) : proxy_busy_mod_done(alive) rescue => e proxy_err_response(502, req_res, e, wbuf) end @@ -247,7 +248,7 @@ module Yahns::HttpResponse # :nodoc: end busy = wbuf.busy and return proxy_busy_mod_blocked(wbuf, busy) - proxy_busy_mod_done(wbuf.wbuf_persist) # returns nil + proxy_busy_mod_done(wbuf.wbuf_persist) # returns nil to close req_res end def proxy_wait_next(qflags) @@ -288,23 +289,23 @@ module Yahns::HttpResponse # :nodoc: when :close then close end - nil # close the req_res, too + nil # signal close for ReqRes#yahns_step end def proxy_busy_mod_blocked(wbuf, busy) - q = Thread.current[:yahns_queue] # we are completely done reading and buffering the upstream response, # but have not completely written the response to the client, # yield control to the client socket: @state = wbuf - case busy - when :wait_readable then q.queue_mod(self, Yahns::Queue::QEV_RD) - when :wait_writable then q.queue_mod(self, Yahns::Queue::QEV_WR) - else - abort "BUG: invalid wbuf.busy: #{busy.inspect}" - end - # no touching self after queue_mod - :ignore + proxy_wait_next(case busy + when :wait_readable then Yahns::Queue::QEV_RD + when :wait_writable then Yahns::Queue::QEV_WR + else + abort "BUG: invalid wbuf.busy: #{busy.inspect}" + end) + # no touching self after proxy_wait_next, we may be running + # HttpClient#yahns_step in a different thread at this point + nil # signal close for ReqRes#yahns_step end # n.b.: we can use String#size for optimized dispatch under YARV instead -- cgit v1.2.3-24-ge0c7 From 5a74026b580cefeb95d75db40e1a55cd0be52751 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 27 Apr 2016 00:27:04 +0000 Subject: proxy_pass: drop resources immediately on errors We don't want to wait on GC to reap sockets on errors, generational GC in Ruby is less aggressive about reaping long-lived objects such as long-lived HTTP connections. --- lib/yahns/proxy_http_response.rb | 7 ++++++- lib/yahns/proxy_pass.rb | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/yahns/proxy_http_response.rb b/lib/yahns/proxy_http_response.rb index 3462e40..c5e7be5 100644 --- a/lib/yahns/proxy_http_response.rb +++ b/lib/yahns/proxy_http_response.rb @@ -43,7 +43,12 @@ module Yahns::HttpResponse # :nodoc: Rack::Utils::HTTP_STATUS_CODES[code]}\r\n\r\n") rescue nil shutdown rescue nil - req_res.shutdown rescue nil + @input = @input.close if @input + + # this is safe ONLY because we are in an :ignore state after + # Fdmap#forget when we got hijacked: + close + nil # signal close of req_res from yahns_step in yahns/proxy_pass.rb ensure wbuf.wbuf_abort if wbuf diff --git a/lib/yahns/proxy_pass.rb b/lib/yahns/proxy_pass.rb index 511db02..148957b 100644 --- a/lib/yahns/proxy_pass.rb +++ b/lib/yahns/proxy_pass.rb @@ -63,7 +63,8 @@ class Yahns::ProxyPass # :nodoc: when Yahns::WbufCommon # streaming/buffering the response body - return c.proxy_response_finish(req, resbuf, self) + # we assign wbuf for rescue below: + return c.proxy_response_finish(req, wbuf = resbuf, self) end while true # case @resbuf @@ -79,7 +80,7 @@ class Yahns::ProxyPass # :nodoc: when Errno::ECONNREFUSED, Errno::ECONNRESET, Errno::EPIPE e.set_backtrace([]) end - c.proxy_err_response(502, self, e, nil) + c.proxy_err_response(502, self, e, wbuf) end # returns :wait_readable if complete, :wait_writable if not -- cgit v1.2.3-24-ge0c7