From a90b6b37f6ded7e9cded7b52044ddb34305f9446 Mon Sep 17 00:00:00 2001 From: Tran Anh Vu Date: Sun, 12 Mar 2023 23:29:37 +0700 Subject: [PATCH] Closes #2341 - Make sure fallback Rack response are sent to the client Currently, when Client I/O operations raise any exceptions, we build a fallback rack response (either by lowlevel_error_handler or by default puma's hard-coded response), but doesn't do anything with it. This commit ensures that this response is sent back to client. Use `bundle exec puma -C test/config/lowlevel_error_handler.rb test/rackup/hello.ru` to see the behavior. --- lib/puma/server.rb | 14 +++++++++++--- test/config/lowlevel_error_handler.rb | 7 +++++++ test/test_config.rb | 2 +- test/test_puma_server.rb | 15 +++++++++++++++ 4 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 test/config/lowlevel_error_handler.rb diff --git a/lib/puma/server.rb b/lib/puma/server.rb index ca9cd8b805..0898fc75ea 100644 --- a/lib/puma/server.rb +++ b/lib/puma/server.rb @@ -464,7 +464,7 @@ def process_client(client) end true rescue StandardError => e - client_error(e, client) + client_error(e, client, requests) # The ensure tries to close +client+ down requests > 0 ensure @@ -492,21 +492,29 @@ def with_force_shutdown(client, &block) # :nocov: # Handle various error types thrown by Client I/O operations. - def client_error(e, client) + def client_error(e, client, requests = 1) # Swallow, do not log return if [ConnectionError, EOFError].include?(e.class) - lowlevel_error(e, client.env) case e when MiniSSL::SSLError @log_writer.ssl_error e, client.io when HttpParserError + status, headers, res_body = lowlevel_error(e, client.env, 400) + prepare_response(status, headers, res_body, requests, client) + client.write_error(400) @log_writer.parse_error e, client when HttpParserError501 + status, headers, res_body = lowlevel_error(e, client.env, 501) + prepare_response(status, headers, res_body, requests, client) + client.write_error(501) @log_writer.parse_error e, client else + status, headers, res_body = lowlevel_error(e, client.env, 500) + prepare_response(status, headers, res_body, requests, client) + client.write_error(500) @log_writer.unknown_error e, nil, "Read" end diff --git a/test/config/lowlevel_error_handler.rb b/test/config/lowlevel_error_handler.rb new file mode 100644 index 0000000000..f4d8d6ac97 --- /dev/null +++ b/test/config/lowlevel_error_handler.rb @@ -0,0 +1,7 @@ +app do |env| + [200, nil, []] +end + +lowlevel_error_handler do |err| + [500, {}, ["something wrong happened"]] +end diff --git a/test/test_config.rb b/test/test_config.rb index df5d386714..7f1a75b442 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -586,7 +586,7 @@ def assert_run_hooks(hook_name, options = {}) def assert_warning_for_hooks_defined_in_single_mode(hook_name) out, _ = capture_io do - conf = Puma::Configuration.new do |c| + Puma::Configuration.new do |c| c.send(hook_name) end end diff --git a/test/test_puma_server.rb b/test/test_puma_server.rb index aba7ece84f..dd1ea863c5 100644 --- a/test/test_puma_server.rb +++ b/test/test_puma_server.rb @@ -1545,4 +1545,19 @@ def test_streaming_enum_body_2 assert_equal str * loops, resp_body assert_operator times.last - times.first, :>, 1.0 end + + def test_lowlevel_error_handler_response + options = { + lowlevel_error_handler: ->(_error) do + [500, {}, ["something wrong happened"]] + end + } + broken_app = ->(_env) { [200, nil, []] } + + server_run(**options, &broken_app) + + data = send_http_and_read "GET / HTTP/1.1\r\n\r\n" + + assert_match(/something wrong happened/, data) + end end