Change HttpParserError to be subclass of StandardError (puma#3590)

* CI: test/test_request_invalid.rb - uncomment two tests, add asserts * JRuby - Change HttpParserError to be subclass of StandardError * C Extension - Change HttpParserError to be subclass of StandardError
didww · Jan 1, 2025 · 07ef55d · 07ef55d
1 parent 59dd65e
commit 07ef55d
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 6 deletions.
diff --git a/ext/puma_http11/org/jruby/puma/Http11.java b/ext/puma_http11/org/jruby/puma/Http11.java
@@ -77,7 +77,7 @@ public IRubyObject allocate(Ruby runtime, RubyClass klass) {
 
     public static void createHttp11(Ruby runtime) {
         RubyModule mPuma = runtime.defineModule("Puma");
-        mPuma.defineClassUnder("HttpParserError",runtime.getClass("IOError"),runtime.getClass("IOError").getAllocator());
+        mPuma.defineClassUnder("HttpParserError",runtime.getClass("StandardError"),runtime.getClass("StandardError").getAllocator());
 
         RubyClass cHttpParser = mPuma.defineClassUnder("HttpParser",runtime.getObject(),ALLOCATOR);
         cHttpParser.defineAnnotatedMethods(Http11.class);

diff --git a/ext/puma_http11/puma_http11.c b/ext/puma_http11/puma_http11.c
@@ -475,7 +475,7 @@ void Init_puma_http11(void)
   DEF_GLOBAL(server_protocol, "SERVER_PROTOCOL");
   DEF_GLOBAL(request_path, "REQUEST_PATH");
 
-  eHttpParserError = rb_define_class_under(mPuma, "HttpParserError", rb_eIOError);
+  eHttpParserError = rb_define_class_under(mPuma, "HttpParserError", rb_eStandardError);
   rb_global_variable(&eHttpParserError);
 
   rb_define_alloc_func(cHttpParser, HttpParser_alloc);

diff --git a/test/test_request_invalid.rb b/test/test_request_invalid.rb
@@ -51,7 +51,19 @@ def setup
     @server = Puma::Server.new app, nil, options
     @bind_port = (@server.add_tcp_listener @host, 0).addr[1]
     @server.run
-    sleep 0.15 if Puma.jruby?
+    sleep 0.15 if Puma::IS_JRUBY
+
+    @error_on_closed = if Puma::IS_MRI
+      if Puma::IS_OSX
+        [Errno::ECONNRESET, EOFError]
+      elsif Puma::IS_WINDOWS
+        [Errno::ECONNABORTED]
+      else
+        [EOFError]
+      end
+    else
+      [EOFError]
+    end
   end
 
   def teardown
@@ -70,12 +82,22 @@ def assert_status(request, status = 400, socket: nil)
     response = if socket
       socket.req_write(request).read_response
     else
-      send_http_read_response request
+      socket = new_socket
+      socket.req_write(request).read_response
     end
 
     re = /\AHTTP\/1\.[01] #{status}/
 
     assert_match re, response, "'#{response[/[^\r]+/]}' should be #{status}"
+
+    if status >= 400
+      if @server.leak_stack_on_error
+        cl = response.headers_hash['Content-Length'].to_i
+        refute_equal 0, cl
+      end
+      socket.req_write GET_11
+      assert_raises(*@error_on_closed) { socket.read_response }
+    end
   end
 
   # ──────────────────────────────────── below are oversize path length
@@ -86,7 +108,7 @@ def test_oversize_path
     assert_status  "GET #{path} HTTP/1.1\r\n\r\n"
   end
 
-  def __test_oversize_path_keep_alive
+  def test_oversize_path_keep_alive
     path = "/#{'a' * 8_500}"
 
     socket = new_socket
@@ -125,7 +147,7 @@ def test_content_length_bad_characters_3
     assert_status "#{GET_PREFIX}#{te}\r\n\r\nHello\r\n\r\n"
   end
 
-  def __test_content_length_bad_characters_1_keep_alive
+  def test_content_length_bad_characters_1_keep_alive
     socket = new_socket
 
     assert_status "GET / HTTP/1.1\r\n\r\n", 200, socket: socket