From a80ba923659a4287f05a8c69fe2c5ad8b65d28f7 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 15 Nov 2009 12:09:23 -0800
Subject: tee_input: expand client error handling

First move it to a separate method, this allows subclasses to
reuse our error handler.  Additionally, capture HttpParserError
as well since backtraces are worthless when a client sends us
a bad request, too.
---
 lib/unicorn/tee_input.rb | 24 +++++++++++++++++-------
 test/unit/test_server.rb | 29 ++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb
index 01717ce..18ce44b 100644
--- a/lib/unicorn/tee_input.rb
+++ b/lib/unicorn/tee_input.rb
@@ -123,6 +123,21 @@ module Unicorn
 
   private
 
+    def client_error(e)
+      case e
+      when EOFError
+        # in case client only did a premature shutdown(SHUT_WR)
+        # we do support clients that shutdown(SHUT_WR) after the
+        # _entire_ request has been sent, and those will not have
+        # raised EOFError on us.
+        socket.close if socket
+        raise ClientShutdown, "bytes_read=#{@tmp.size}", []
+      when HttpParserError
+        e.set_backtrace([])
+        raise e
+      end
+    end
+
     # tees off a +length+ chunk of data from the input into the IO
     # backing store as well as returning it.  +dst+ must be specified.
     # returns nil if reading from the input returns nil
@@ -135,13 +150,8 @@ module Unicorn
         end
       end
       finalize_input
-      rescue EOFError
-        # in case client only did a premature shutdown(SHUT_WR)
-        # we do support clients that shutdown(SHUT_WR) after the
-        # _entire_ request has been sent, and those will not have
-        # raised EOFError on us.
-        socket.close if socket
-        raise ClientShutdown, "bytes_read=#{@tmp.size}", []
+      rescue => e
+        client_error(e)
     end
 
     def finalize_input
diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb
index 7f0c443..55147b7 100644
--- a/test/unit/test_server.rb
+++ b/test/unit/test_server.rb
@@ -16,8 +16,9 @@ class TestHandler
     while env['rack.input'].read(4096)
     end
     [200, { 'Content-Type' => 'text/plain' }, ['hello!\n']]
-    rescue Unicorn::ClientShutdown => e
+    rescue Unicorn::ClientShutdown, Unicorn::HttpParserError => e
       $stderr.syswrite("#{e.class}: #{e.message} #{e.backtrace.empty?}\n")
+      raise e
    end
 end
 
@@ -165,6 +166,32 @@ class WebServerTest < Test::Unit::TestCase
     assert_nothing_raised { sock.close }
   end
 
+  def test_client_malformed_body
+    sock = nil
+    buf = nil
+    bs = 15653984
+    assert_nothing_raised do
+      sock = TCPSocket.new('127.0.0.1', @port)
+      sock.syswrite("PUT /hello HTTP/1.1\r\n")
+      sock.syswrite("Host: example.com\r\n")
+      sock.syswrite("Transfer-Encoding: chunked\r\n")
+      sock.syswrite("Trailer: X-Foo\r\n")
+      sock.syswrite("\r\n")
+      sock.syswrite("%x\r\n" % [ bs ])
+      sock.syswrite("F" * bs)
+    end
+    begin
+      File.open("/dev/urandom", "rb") { |fp| sock.syswrite(fp.sysread(16384)) }
+    rescue
+    end
+    assert_nothing_raised { sock.close }
+    next_client = Net::HTTP.get(URI.parse("http://127.0.0.1:#@port/"))
+    assert_equal 'hello!\n', next_client
+    lines = File.readlines("test_stderr.#$$.log")
+    lines = lines.grep(/^Unicorn::HttpParserError: .* true$/)
+    assert_equal 1, lines.size
+  end
+
   def do_test(string, chunk, close_after=nil, shutdown_delay=0)
     # Do not use instance variables here, because it needs to be thread safe
     socket = TCPSocket.new("127.0.0.1", @port);
-- 
cgit v1.2.3-24-ge0c7