From c83b5a903a076fda67c7d062da1ad6ff9337fdd1 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Fri, 27 Mar 2009 17:26:03 -0700
Subject: Always try to send a valid HTTP response back

This reworks error handling throughout the entire stack to be
more Ruby-ish.  Exceptions are raised instead of forcing the
us to check return values.

  If a client is sending us a bad request, we send a 400.

  If unicorn or app breaks in an unexpected way, we'll
  send a 500.

Both of these last-resort error responses are sent using
IO#write_nonblock to avoid tying Unicorn up longer than
necessary and all exceptions raised are ignored.

Sending a valid HTTP response back should reduce the chance of
us from being marked as down or broken by a load balancer.
Previously, some load balancers would mark us as down if we close
a socket without sending back a valid response; so make a best
effort to send one.  If for some reason we cannot write a valid
response, we're still susceptible to being marked as down.

A successful HttpResponse.write() call will now close the socket
immediately (instead of doing it higher up the stack).  This
ensures the errors will never get written to the socket on a
successful response.
---
 test/unit/test_response.rb |  7 ++++---
 test/unit/test_server.rb   | 29 +++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 3 deletions(-)

(limited to 'test')

diff --git a/test/unit/test_response.rb b/test/unit/test_response.rb
index 4c7423b..62cabdf 100644
--- a/test/unit/test_response.rb
+++ b/test/unit/test_response.rb
@@ -22,7 +22,7 @@ class ResponseTest < Test::Unit::TestCase
     $, = "\f\v"
     out = StringIO.new
     HttpResponse.write(out,[200, {"X-Whatever" => "stuff"}, ["cool"]])
-    resp = out.read
+    resp = out.string
     assert ! resp.include?("\f\v"), "output didn't use $, ($OFS)"
     ensure
       $, = old_ofs
@@ -38,8 +38,9 @@ class ResponseTest < Test::Unit::TestCase
     code = 400
     io = StringIO.new
     HttpResponse.write(io, [code, {}, []])
-    io.rewind
-    assert_match(/.* #{HTTP_STATUS_CODES[code]}$/, io.readline.chomp, "wrong default reason phrase")
+    lines = io.string.split(/\r\n/)
+    assert_match(/.* #{HTTP_STATUS_CODES[code]}$/, lines.first,
+                 "wrong default reason phrase")
   end
 
   def test_rack_multivalue_headers
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index 356f946..eadeba4 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -35,6 +35,25 @@ class WebServerTest < Test::Unit::TestCase
     end
   end
 
+  def test_broken_app
+    teardown
+    port = unused_port
+    app = lambda { |env| raise RuntimeError, "hello" }
+    # [200, {}, []] }
+    redirect_test_io do
+      @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#{port}"] )
+      @server.start
+    end
+    sock = nil
+    assert_nothing_raised do
+      sock = TCPSocket.new('127.0.0.1', port)
+      sock.syswrite("GET / HTTP/1.0\r\n\r\n")
+    end
+
+    assert_match %r{\AHTTP/1.[01] 500\b}, sock.sysread(4096)
+    assert_nothing_raised { sock.close }
+  end
+
   def test_simple_server
     results = hit(["http://localhost:#{@port}/test"])
     assert_equal 'hello!\n', results[0], "Handler didn't really run"
@@ -77,6 +96,16 @@ class WebServerTest < Test::Unit::TestCase
     end
   end
 
+  def test_bad_client_400
+    sock = nil
+    assert_nothing_raised do
+      sock = TCPSocket.new('127.0.0.1', @port)
+      sock.syswrite("GET / HTTP/1.0\r\nHost: foo\rbar\r\n\r\n")
+    end
+    assert_match %r{\AHTTP/1.[01] 400\b}, sock.sysread(4096)
+    assert_nothing_raised { sock.close }
+  end
+
   def test_header_is_too_long
     redirect_test_io do
       long = "GET /test HTTP/1.1\r\n" + ("X-Big: stuff\r\n" * 15000) + "\r\n"
-- 
cgit v1.2.3-24-ge0c7