Dan Kubb's conditional response patch.

git-svn-id: svn+ssh://rubyforge.org/var/svn/mongrel/trunk@299 19e92222-5c0b-0410-8929-a290d50e31e9

4 files changed, 62 insertions, 42 deletions

diff --git a/lib/mongrel.rb b/lib/mongrel.rb
index 6919c1a..e8f65e7 100644
--- a/lib/mongrel.rb
+++ b/lib/mongrel.rb
@@ -145,7 +145,7 @@ module Mongrel
     MAX_BODY=MAX_HEADER
 
     # A frozen format for this is about 15% faster
-    STATUS_FORMAT = "HTTP/1.1 %d %s\r\nContent-Length: %d\r\nConnection: close\r\n".freeze
+    STATUS_FORMAT = "HTTP/1.1 %d %s\r\nConnection: close\r\n".freeze
     CONTENT_TYPE = "Content-Type".freeze
     LAST_MODIFIED = "Last-Modified".freeze
     ETAG = "ETag".freeze
@@ -159,7 +159,7 @@ module Mongrel
     LINE_END="\r\n".freeze
     REMOTE_ADDR="REMOTE_ADDR".freeze
     HTTP_X_FORWARDED_FOR="HTTP_X_FORWARDED_FOR".freeze
-    HTTP_IF_UNMODIFIED_SINCE="HTTP_IF_UNMODIFIED_SINCE".freeze
+    HTTP_IF_MODIFIED_SINCE="HTTP_IF_MODIFIED_SINCE".freeze
     HTTP_IF_NONE_MATCH="HTTP_IF_NONE_MATCH".freeze
     REDIRECT = "HTTP/1.1 302 Found\r\nLocation: %s\r\nConnection: close\r\n\r\n".freeze
   end
@@ -397,10 +397,10 @@ module Mongrel
       end
     end
 
-    def send_status(content_length=nil)
+    def send_status(content_length=@body.length)
       if not @status_sent
-        content_length ||= @body.length
-        write(Const::STATUS_FORMAT % [status, HTTP_STATUS_CODES[@status], content_length])
+        @header['Content-Length'] = content_length unless @status == 304
+        write(Const::STATUS_FORMAT % [@status, HTTP_STATUS_CODES[@status]])
         @status_sent = true
       end
     end
@@ -643,16 +643,29 @@ module Mongrel
       end
     end
 
+    def configure_socket_options
+      if /linux/ === RUBY_PLATFORM
+        # 9 is currently TCP_DEFER_ACCEPT
+        $tcp_defer_accept_opts = [9,1]
+        $tcp_cork_opts = [3,1]
+      end
+    end
 
     # Runs the thing.  It returns the thread used so you can "join" it.  You can also
     # access the HttpServer::acceptor attribute to get the thread later.
     def run
       BasicSocket.do_not_reverse_lookup=true
 
+      configure_socket_options
+
+      @socket.setsockopt(Socket::SOL_TCP, $tcp_defer_accept_opts[0], $tcp_defer_accept_opts[1]) if $tcp_defer_accept_opts
+
       @acceptor = Thread.new do
         while true
           begin
             client = @socket.accept
+            client.setsockopt(Socket::SOL_TCP, $tcp_cork_opts[0], $tcp_cork_opts[1]) if $tcp_cork_opts
+
             worker_list = @workers.list
 
             if worker_list.length >= @num_processors
diff --git a/lib/mongrel/handlers.rb b/lib/mongrel/handlers.rb
index fede2db..dc18ba9 100644
--- a/lib/mongrel/handlers.rb
+++ b/lib/mongrel/handlers.rb
@@ -203,18 +203,18 @@ module Mongrel
       # Calculated the same as apache, not sure how well the works on win32
       etag = Const::ETAG_FORMAT % [mtime.to_i, stat.size, stat.ino]
 
-      unmodified_since = request.params[Const::HTTP_IF_UNMODIFIED_SINCE]
+      modified_since = request.params[Const::HTTP_IF_MODIFIED_SINCE]
       none_match = request.params[Const::HTTP_IF_NONE_MATCH]
 
       # test to see if this is a conditional request, and test if
       # the response would be identical to the last response
       same_response = case
-                      when unmodified_since && !last_response_time = Time.httpdate(unmodified_since) rescue nil : false
-                      when unmodified_since && last_response_time > Time.now                                    : false
-                      when unmodified_since && mtime > last_response_time                                       : false
-                      when none_match       && none_match == '*'                                                : false
-                      when none_match       && !none_match.strip.split(/\s*,\s*/).include?(etag)                : false
-                      else unmodified_since || none_match  # validation successful if we get this far and at least one of the header exists
+                      when modified_since && !last_response_time = Time.httpdate(modified_since) rescue nil : false
+                      when modified_since && last_response_time > Time.now                                  : false
+                      when modified_since && mtime > last_response_time                                     : false
+                      when none_match     && none_match == '*'                                              : false
+                      when none_match     && !none_match.strip.split(/\s*,\s*/).include?(etag)              : false
+                      else modified_since || none_match  # validation successful if we get this far and at least one of the header exists
                       end
 
       header = response.header
diff --git a/lib/mongrel/rails.rb b/lib/mongrel/rails.rb
index 1e4cb42..6b4c7c1 100644
--- a/lib/mongrel/rails.rb
+++ b/lib/mongrel/rails.rb
@@ -77,7 +77,7 @@ module Mongrel
           rescue Errno::EPIPE
             # ignored
           rescue Object => rails_error
-            STDERR.puts "#{Tim.now}: Error calling Dispatcher.dispatch #{rails_error.inspect}"
+            STDERR.puts "#{Time.now}: Error calling Dispatcher.dispatch #{rails_error.inspect}"
             STDERR.puts rails_error.backtrace.join("\n")
           ensure
             @guard.unlock unless ActionController::Base.allow_concurrency
diff --git a/test/test_conditional.rb b/test/test_conditional.rb
index 511570e..3b71ac9 100644
--- a/test/test_conditional.rb
+++ b/test/test_conditional.rb
@@ -23,6 +23,7 @@ class HttpParserTest < Test::Unit::TestCase
     res = @http.start { |http| http.get(@path) }
     assert_not_nil @etag = res['ETag']
     assert_not_nil @last_modified = res['Last-Modified']
+    assert_not_nil @content_length = res['Content-Length']
   end
 
   def teardown
@@ -44,68 +45,74 @@ class HttpParserTest < Test::Unit::TestCase
     assert_status_for_get_and_head Net::HTTPNotModified, 'If-None-Match' => @etag
   end
 
-  # status should be 304 Not Modified when If-Unmodified-Since is the matching Last-Modified date
-  def test_not_modified_via_if_unmodified_since
-    assert_status_for_get_and_head Net::HTTPNotModified, 'If-Unmodified-Since' => @last_modified
+  # status should be 304 Not Modified when If-Modified-Since is the matching Last-Modified date
+  def test_not_modified_via_if_modified_since
+    assert_status_for_get_and_head Net::HTTPNotModified, 'If-Modified-Since' => @last_modified
   end
 
   # status should be 304 Not Modified when If-None-Match is the matching ETag
-  # and If-Unmodified-Since is the matching Last-Modified date
-  def test_not_modified_via_if_none_match_and_if_unmodified_since
-    assert_status_for_get_and_head Net::HTTPNotModified, 'If-None-Match' => @etag, 'If-Unmodified-Since' => @last_modified
+  # and If-Modified-Since is the matching Last-Modified date
+  def test_not_modified_via_if_none_match_and_if_modified_since
+    assert_status_for_get_and_head Net::HTTPNotModified, 'If-None-Match' => @etag, 'If-Modified-Since' => @last_modified
   end
 
   # status should be 200 OK when If-None-Match is invalid
   def test_invalid_if_none_match
     assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => 'invalid'
-    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => 'invalid', 'If-Unmodified-Since' => @last_modified
+    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => 'invalid', 'If-Modified-Since' => @last_modified
   end
 
-  # status should be 200 OK when If-Unmodified-Since is invalid
-  def test_invalid_if_unmodified_since
-    assert_status_for_get_and_head Net::HTTPOK,                           'If-Unmodified-Since' => 'invalid'
-    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => @etag, 'If-Unmodified-Since' => 'invalid'
+  # status should be 200 OK when If-Modified-Since is invalid
+  def test_invalid_if_modified_since
+    assert_status_for_get_and_head Net::HTTPOK,                           'If-Modified-Since' => 'invalid'
+    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => @etag, 'If-Modified-Since' => 'invalid'
   end
 
-  # status should be 304 Not Modified when If-Unmodified-Since is greater than the Last-Modified header, but less than the system time
-  def test_if_unmodified_since_greater_than_last_modified
+  # status should be 304 Not Modified when If-Modified-Since is greater than the Last-Modified header, but less than the system time
+  def test_if_modified_since_greater_than_last_modified
     sleep 2
     last_modified_plus_1 = (Time.httpdate(@last_modified) + 1).httpdate
-    assert_status_for_get_and_head Net::HTTPNotModified,                           'If-Unmodified-Since' => last_modified_plus_1
-    assert_status_for_get_and_head Net::HTTPNotModified, 'If-None-Match' => @etag, 'If-Unmodified-Since' => last_modified_plus_1
+    assert_status_for_get_and_head Net::HTTPNotModified,                           'If-Modified-Since' => last_modified_plus_1
+    assert_status_for_get_and_head Net::HTTPNotModified, 'If-None-Match' => @etag, 'If-Modified-Since' => last_modified_plus_1
   end
 
-  # status should be 200 OK when If-Unmodified-Since is less than the Last-Modified header
-  def test_if_unmodified_since_less_than_last_modified
+  # status should be 200 OK when If-Modified-Since is less than the Last-Modified header
+  def test_if_modified_since_less_than_last_modified
     last_modified_minus_1 = (Time.httpdate(@last_modified) - 1).httpdate
-    assert_status_for_get_and_head Net::HTTPOK,                           'If-Unmodified-Since' => last_modified_minus_1
-    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => @etag, 'If-Unmodified-Since' => last_modified_minus_1
+    assert_status_for_get_and_head Net::HTTPOK,                           'If-Modified-Since' => last_modified_minus_1
+    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => @etag, 'If-Modified-Since' => last_modified_minus_1
   end
 
-  # status should be 200 OK when If-Unmodified-Since is a date in the future
-  def test_future_if_unmodified_since
+  # status should be 200 OK when If-Modified-Since is a date in the future
+  def test_future_if_modified_since
     the_future = Time.at(2**31-1).httpdate
-    assert_status_for_get_and_head Net::HTTPOK,                           'If-Unmodified-Since' => the_future
-    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => @etag, 'If-Unmodified-Since' => the_future
+    assert_status_for_get_and_head Net::HTTPOK,                           'If-Modified-Since' => the_future
+    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => @etag, 'If-Modified-Since' => the_future
   end
 
   # status should be 200 OK when If-None-Match is a wildcard
   def test_wildcard_match
     assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => '*'
-    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => '*', 'If-Unmodified-Since' => @last_modified
+    assert_status_for_get_and_head Net::HTTPOK, 'If-None-Match' => '*', 'If-Modified-Since' => @last_modified
   end
 
   private
 
     # assert the response status is correct for GET and HEAD
-    def assert_status_for_get_and_head(status_class, headers = {})
+    def assert_status_for_get_and_head(response_class, headers = {})
       %w{ get head }.each do |method|
         res = @http.send(method, @path, headers)
-        assert_kind_of status_class, res
+        assert_kind_of response_class, res
         assert_equal @etag, res['ETag']
-        case status_class
-          when Net::HTTPNotModified : assert_nil res['Last-Modified']
-          when Net::HTTPOK          : assert_equal @last_modified, res['Last-Modified']
+        case response_class.to_s
+          when 'Net::HTTPNotModified' then
+            assert_nil res['Last-Modified']
+            assert_nil res['Content-Length']
+          when 'Net::HTTPOK' then
+            assert_equal @last_modified, res['Last-Modified']
+            assert_equal @content_length, res['Content-Length']
+          else
+            fail "Incorrect response class: #{response_class}"
         end
       end
     end