unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
						download: 
* [PATCH] ensure body is closed during hijack
@ 2015-06-09 20:17  7% Eric Wong
  0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2015-06-09 20:17 UTC (permalink / raw)
  To: unicorn-public

Middlewares such as Rack::Lock (used by Rails) break badly unless
the response body is closed on hijack, so we will close it to follow
the lead of other popular Rack servers.

While it's unclear if there's anybody using rack.hijack with unicorn,
we'll try to emulate the behavior of other servers as much as
possible.

ref: https://github.com/ngauthier/tubesock/issues/10
---
 lib/unicorn/http_response.rb |  3 ---
 lib/unicorn/http_server.rb   | 20 +++++++++++++-------
 t/hijack.ru                  |  3 ++-
 t/t0200-rack-hijack.sh       |  7 +++++--
 test/unit/test_response.rb   | 11 -----------
 5 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/lib/unicorn/http_response.rb b/lib/unicorn/http_response.rb
index 801bf9a..016dac8 100644
--- a/lib/unicorn/http_response.rb
+++ b/lib/unicorn/http_response.rb
@@ -52,12 +52,9 @@ module Unicorn::HttpResponse
     end
 
     if hijack
-      body = nil # ensure we do not close body
       hijack.call(socket)
     else
       body.each { |chunk| socket.write(chunk) }
     end
-  ensure
-    body.respond_to?(:close) and body.close
   end
 end
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index 2657c29..9129ed8 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -559,16 +559,22 @@ class Unicorn::HttpServer
   # in 3 easy steps: read request, call app, write app response
   def process_client(client)
     status, headers, body = @app.call(env = @request.read(client))
-    return if @request.hijacked?
 
-    if 100 == status.to_i
-      e100_response_write(client, env)
-      status, headers, body = @app.call(env)
+    begin
       return if @request.hijacked?
+
+      if 100 == status.to_i
+        e100_response_write(client, env)
+        status, headers, body = @app.call(env)
+        return if @request.hijacked?
+      end
+      @request.headers? or headers = nil
+      http_response_write(client, status, headers, body,
+                          @request.response_start_sent)
+    ensure
+      body.respond_to?(:close) and body.close
     end
-    @request.headers? or headers = nil
-    http_response_write(client, status, headers, body,
-                        @request.response_start_sent)
+
     unless client.closed? # rack.hijack may've close this for us
       client.shutdown # in case of fork() in Rack app
       client.close # flush and uncork socket immediately, no keepalive
diff --git a/t/hijack.ru b/t/hijack.ru
index fcb0b6d..4adec61 100644
--- a/t/hijack.ru
+++ b/t/hijack.ru
@@ -2,12 +2,13 @@ use Rack::Lint
 use Rack::ContentLength
 use Rack::ContentType, "text/plain"
 class DieIfUsed
+  @@n = 0
   def each
     abort "body.each called after response hijack\n"
   end
 
   def close
-    abort "body.close called after response hijack\n"
+    warn "closed DieIfUsed #{@@n += 1}\n"
   end
 end
 run lambda { |env|
diff --git a/t/t0200-rack-hijack.sh b/t/t0200-rack-hijack.sh
index f772071..de3eb82 100755
--- a/t/t0200-rack-hijack.sh
+++ b/t/t0200-rack-hijack.sh
@@ -16,12 +16,15 @@ t_begin "check response hijack" && {
 	test "xresponse.hijacked" = x"$(curl -sSfv http://$listen/hijack_res)"
 }
 
-t_begin "killing succeeds" && {
+t_begin "killing succeeds after hijack" && {
 	kill $unicorn_pid
 }
 
-t_begin "check stderr" && {
+t_begin "check stderr for hijacked body close" && {
 	check_stderr
+	grep 'closed DieIfUsed 1\>' $r_err
+	grep 'closed DieIfUsed 2\>' $r_err
+	! grep 'closed DieIfUsed 3\>' $r_err
 }
 
 t_done
diff --git a/test/unit/test_response.rb b/test/unit/test_response.rb
index d0f0c79..3478288 100644
--- a/test/unit/test_response.rb
+++ b/test/unit/test_response.rb
@@ -72,17 +72,6 @@ class ResponseTest < Test::Unit::TestCase
     assert ! out.closed?
   end
 
-  def test_body_closed
-    expect_body = %w(1 2 3 4).join("\n")
-    body = StringIO.new(expect_body)
-    body.rewind
-    out = StringIO.new
-    http_response_write(out,200, {}, body)
-    assert ! out.closed?
-    assert body.closed?
-    assert_match(expect_body, out.string.split(/\r\n/).last)
-  end
-
   def test_unknown_status_pass_through
     out = StringIO.new
     http_response_write(out,"666 I AM THE BEAST", {}, [] )
-- 
EW


^ permalink raw reply	[relevance 7%]

* [ANN] unicorn 5.0.0.pre1 - incompatible changes!
@ 2015-06-15 22:56  5% Eric Wong
  0 siblings, 0 replies; 2+ results
From: Eric Wong @ 2015-06-15 22:56 UTC (permalink / raw)
  To: unicorn-public

This release finally drops Ruby 1.8 support and requires Ruby 1.9.3
or later.  The horrible "Status:" header in our HTTP response is
finally gone, saving at least 16 precious bytes in every single HTTP
response.

Under Ruby 2.1 and later, the monotonic clock is used for timeout
handling for better accuracy.

Several experimental, unused and undocumented features are removed.

There's also tiny, minor performance and memory improvements from
dropping 1.8 compatibility, but probably nothing noticeable on a
typical real-life (bloated) app.

The biggest performance improvement we made was to our website by
switching to olddoc.  Depending on connection speed, latency, and
renderer performance, it typically loads two to four times faster.

Finally, for the billionth time: unicorn must never be exposed
to slow clients, as it will never ever use new-fangled things
like non-blocking socket I/O, threads, epoll or kqueue.  unicorn
must be used with a fully-buffering reverse proxy such as nginx
for slow clients.

I'll tag 5.0.0 final in a week or so if all goes well

= gem install --pre unicorn
= git clone git://bogomips.org/unicorn.git
= http://unicorn.bogomips.org/

* ISSUES: update with mailing list subscription
* GIT-VERSION-GEN: start 5.0.0 development
* http: remove xftrust options
* FAQ: add entry for Rails autoflush_log
* dev: remove isolate dependency
* unicorn.gemspec: depend on test-unit 3.0
* http_response: remove Status: header
* remove RubyForge and Freecode references
* remove mongrel.rubyforge.org references
* http: remove the keepalive requests limit
* http: reduce parser from 72 to 56 bytes on 64-bit
* examples: add run_once to before_fork hook example
* worker: remove old tmp accessor
* http_server: save 450+ bytes of memory on x86-64
* t/t0002-parser-error.sh: relax test for rack 1.6.0
* remove SSL support
* tmpio: drop the "size" method
* switch docs + website to olddoc
* README: clarify/reduce references to unicorn_rails
* gemspec: fixup olddoc migration
* use the monotonic clock under Ruby 2.1+
* http: -Wshorten-64-to-32 warnings on clang
* remove old inetd+git examples and exec_cgi
* http: standalone require + reduction in binary size
* GNUmakefile: fix clean gem build + reduce build cruft
* socket_helper: reduce constant lookups and caching
* remove 1.8, <= 1.9.1 fallback for missing IO#autoclose=
* favor IO#close_on_exec= over fcntl in 1.9+
* use require_relative to reduce syscalls at startup
* doc: update support status for Ruby versions
* fix uninstalled testing and reduce require paths
* test_socket_helper: do not depend on SO_REUSEPORT
* favor "a.b(&:c)" form over "a.b { |x| x.c }"
* ISSUES: add section for bugs in other projects
* http_server: favor ivars over constants
* explain 11 byte magic number for self-pipe
* const: drop constants used by Rainbows!
* reduce and localize constant string use
* Links: mark Rainbows! as historical, reference yahns
* save about 200 bytes of memory on x86-64
* http: remove deprecated reset method
* http: remove experimental dechunk! method
* socket_helper: update comments
* doc: document UNICORN_FD in manpage
* doc: document Etc.nprocessors for worker_processes
* favor more string literals for cold call sites
* tee_input: support for Rack::TempfileReaper middleware
* support TempfileReaper in deployment and development envs
* favor kgio_wait_readable for single FD over select
* Merge tag 'v4.9.0'
* http_request: support rack.hijack by default
* avoid extra allocation for hijack proc creation
* FAQ: add note about ECONNRESET errors from bodies
* process SIGWINCH unless stdin is a TTY
* ISSUES: discourage HTML mail strongly, welcome nyms
* http: use rb_hash_clear in Ruby 2.0+
* http_response: avoid special-casing for Rack < 1.5
* www: install NEWS.atom.xml properly
* http_server: remove a few more accessors and constants
* http_response: simplify regular expression
* move the socket into Rack env for hijacking
* http: move response_start_sent into the C ext
* FAQ: reorder bit on Rack 1.1.x and Rails 2.3.x
* ensure body is closed during hijack

-- 
EW

^ permalink raw reply	[relevance 5%]

Results 1-2 of 2 | reverse results
2015-06-09 20:17  7% [PATCH] ensure body is closed during hijack Eric Wong
2015-06-15 22:56  5% [ANN] unicorn 5.0.0.pre1 - incompatible changes! Eric Wong


unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://yhbt.net/unicorn-public
	git clone --mirror http://ou63pmih66umazou.onion/unicorn-public

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.unicorn
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.unicorn

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git