Write 400 on HTTP parse error. Fixes #142

lib/puma/client.rb

@@ -229,5 +229,19 @@ module Puma
 
       false
     end
+
+    def write_400
+      begin
+        @io << ERROR_400_RESPONSE
+      rescue StandardError
+      end
+    end
+
+    def write_500
+      begin
+        @io << ERROR_500_RESPONSE
+      rescue StandardError
+      end
+    end
   end
 end

lib/puma/const.rb

@@ -47,11 +47,17 @@ module Puma
 
     PUMA_TMP_BASE = "puma".freeze
 
+    # Indicate that we couldn't parse the request
+    ERROR_400_RESPONSE = "HTTP/1.1 400 Bad Request\r\n\r\n"
+
     # The standard empty 404 response for bad requests.  Use Error4040Handler for custom stuff.
     ERROR_404_RESPONSE = "HTTP/1.1 404 Not Found\r\nConnection: close\r\nServer: Puma #{PUMA_VERSION}\r\n\r\nNOT FOUND".freeze
 
     CONTENT_LENGTH = "CONTENT_LENGTH".freeze
 
+    # Indicate that there was an internal error, obviously.
+    ERROR_500_RESPONSE = "HTTP/1.1 500 Internal Server Error\r\n\r\n"
+
     # A common header for indicating the server is too busy.  Not used yet.
     ERROR_503_RESPONSE = "HTTP/1.1 503 Service Unavailable\r\n\r\nBUSY".freeze
 

lib/puma/reactor.rb

@@ -59,13 +59,16 @@ module Puma
 
               # The client doesn't know HTTP well
               rescue HttpParserError => e
+                c.write_400
                 c.close
+
                 sockets.delete c
 
                 @events.parse_error @server, c.env, e
-
               rescue StandardError => e
+                c.write_500
                 c.close
+
                 sockets.delete c
               end
             end

lib/puma/server.rb

@@ -206,7 +206,9 @@ module Puma
         begin
           process_now = client.eagerly_finish
         rescue HttpParserError => e
+          client.write_400
           client.close
+
           @events.parse_error self, client.env, e
         rescue IOError
           client.close
@@ -325,10 +327,14 @@ module Puma
 
       # The client doesn't know HTTP well
       rescue HttpParserError => e
+        client.write_400
+
         @events.parse_error self, client.env, e
 
       # Server error
       rescue StandardError => e
+        client.write_500
+
         @events.unknown_error self, e, "Read"
 
       ensure

test/test_integration.rb

@@ -3,6 +3,7 @@ require 'test/unit'
 require 'socket'
 require 'timeout'
 require 'net/http'
+require 'tempfile'
 
 require 'puma/cli'
 require 'puma/control_cli'
@@ -15,6 +16,7 @@ class TestIntegration < Test::Unit::TestCase
     @tcp_port = 9998
 
     @server = nil
+    @script = nil
   end
 
   def teardown
@@ -27,14 +29,27 @@ class TestIntegration < Test::Unit::TestCase
       Process.wait @server.pid
       @server.close
     end
+
+    if @script
+      @script.close!
+    end
   end
 
   def server(opts)
     core = "#{Gem.ruby} -rubygems -Ilib bin/puma"
     cmd = "#{core} --restart-cmd '#{core}' -b tcp://127.0.0.1:#{@tcp_port} #{opts}"
-    @server = IO.popen(cmd, "r")
+    tf = Tempfile.new "puma-test"
+    tf.puts "exec #{cmd}"
+    tf.close
+
+    @script = tf
+
+    @server = IO.popen("sh #{tf.path}", "r")
+
+    true while @server.gets =~ /Ctrl-C/
 
     sleep 1
+
     @server
   end
 
@@ -80,7 +95,8 @@ class TestIntegration < Test::Unit::TestCase
     s.readpartial(20)
     signal :USR2
 
-    sleep 3
+    true while @server.gets =~ /Ctrl-C/
+    sleep 1
 
     s.write "GET / HTTP/1.1\r\n\r\n"
 
@@ -94,4 +110,13 @@ class TestIntegration < Test::Unit::TestCase
     s << "GET / HTTP/1.0\r\n\r\n"
     assert_equal "Hello World", s.read.split("\r\n").last
   end
+
+  def test_bad_query_string_outputs_400
+    server "-q test/hello.ru 2>&1"
+
+    s = TCPSocket.new "localhost", @tcp_port
+    s << "GET /?h=% HTTP/1.0\r\n\r\n"
+    data = s.read
+    assert_equal "HTTP/1.1 400 Bad Request\r\n\r\n", data
+  end
 end